Re: exit vs. kill on ARM

[Shaun Jackman <sjackman at gmail dot com>]

On 7/19/05, Dave Korn <dave.korn@artimi.com> wrote:
>   I don't know ARM, so this is totally off the top of my head, but ..... is
> there a SIGKILL that might be appropriate instead?  If you're running under
> GDB or another debugger, wouldn't using it make the debugger DTRT?

There's a breakpoint instruction, asm(".word 0xe7ffdefe") or
asm(".word 0xdfffdfff"), that the debugger should deal with properly.
The best behaviour is to call the syscall and let the kernel DTRT,
which may include signalling the debugger with a breakpoint.

> > This patch also removes _raise.
> 
>   Generally patches should serve one purpose, and something like this should
> be submitted separately.  Suppose you were wrong about _raise and someone
> needed to revert your patch in CVS in a hurry; it'd be a lot better if it
> didn't un-do the _exit/_kill change at the same time.

_raise is closely enough related to _kill it seemed reasonable to
submit the changes as one patch. I suppose the important distinction
is that the two changes do not depend on each other, and so should
probably be in separate patches.

> >  My grepping of the code has shown that
> > newlib calls _raise_r, which is implemented in
> > newlib/libc/signal/signal.c, but never _raise.
[clip]
> still links, and we know your change shouldn't affect anything but ARM.  How
> many different ARM targets are there?  How many did you try building?  Is
> there a non-reentrant newlib build that might want _raise instead of
> _raise_r?

The best answer would be "many-many ARM targets", and I tried building
only one, arm-elf. Unfortunately, I don't have the resources (read:
time) to test more than this one target, but I do make my best effort
to convince myself the changes don't break other targets. In the end,
every patch I submit is burdened with a WFM, YMMV, IANAOB [1] warning.
I still hope they are useful to the community though, inherent danger
notwithstanding.

There is a macro REENTRANT_SYSCALLS_PROVIDED which, if not defined,
causes the _xxx_r functions to be implemented in terms of _xxx. _raise
is slightly different though, because _raise is not actually a
standard syscall. raise(3) is usually a library call that's
implemented in terms of the system calls getpid(2) and kill(2). In
this case, I believe _raise is a vestigial stub and warrants removal.

[1] I am not an omniscient being.

>   Oh, there's one other very minor point to bear in mind in future patches.
> In your patch you've moved the location of _exit from before _kill to after
> it.  Given that all the functions are prototyped at the top of the file,
[clip]

I generally place coding style at a higher priority than minimising
deltas, although both are important. I avoid top-prototyped functions
and try to define functions in order so that later functions call
earlier functions. I find this helps structure the source file, with
the added help of the compiler enforcing the rules. Which is more
important, making the diff easier to read, or making the resulting
source code easier to read? This is not a rhetorical question, and
I'll change my patch to suit the consensus.

Cheers,
Shaun