[PATCH] kill: add '--' option to separate options from arguments

[Xabier Oneca -- xOneca]

Hello,

Shouldn't '--' be taken care by getopt/getopt32 so all applets benefit
from it? (I thought it already did that...)

2017-08-28 14:07 GMT+02:00 Denys Vlasenko <vda.linux at googlemail.com>:
> Applied, thanks.
>
> On Sun, Aug 27, 2017 at 9:25 AM, Ron Yorston <rmy at pobox.com> wrote:
>> Using a negative pid to send TERM to a process group results in an
>> obscure error:
>>
>>    $ ./busybox kill -12345
>>    kill: bad signal name '12345'
>>
>> There are a couple of workarounds:
>>
>> - specify the signal: 'kill -TERM -12345'
>>
>> - use the 'shell hack' and add a leading space to the argument:
>>   'kill " -12345"'.  This only works if the shell hack is enabled.
>>
>> The full versions of kill and killall provide a '--' argument to
>> separate options from arguments.  Add this capability to BusyBox.
>>
>> This doesn't avoid the need for the shell hack, which is only partly
>> about handling arguments that start with a dash:  it also packs multiple
>> pids into a single argument.
>>
>> bloat-o-meter gives the somewhat unbelievable result:
>>
>> function                                             old     new   delta
>> kill_main                                            993     999      +6
>> packed_usage                                       32073 32047     -26
>> .rodata                                           164190  164164     -26
>> ------------------------------------------------------------------------------
>> (add/remove: 0/0 grow/shrink: 1/2 up/down: 6/-52)             Total: -46 bytes
>>    text    data     bss     dec     hex filename
>>  872711    4110    1944  878765   d68ad busybox_old
>>  872691    4110    1944  878745   d6899 busybox_unstripped
>>
>> A few more changes like that and BusyBox will take a negative amount
>> of disk space.
>>
>> Signed-off-by: Ron Yorston <rmy at pobox.com>
>> ---
>>  procps/kill.c | 9 +++++++--
>>  1 file changed, 7 insertions(+), 2 deletions(-)
>>
>> diff --git a/procps/kill.c b/procps/kill.c
>> index 0ddae2f70..e085f4d56 100644
>> --- a/procps/kill.c
>> +++ b/procps/kill.c
>> @@ -42,7 +42,7 @@
>>  //kbuild:lib-$(CONFIG_KILLALL5) += kill.o
>>
>>  //usage:#define kill_trivial_usage
>> -//usage:       "[-l] [-SIG] PID..."
>> +//usage:       "[-l] [-SIG] [--] PID..."
>>  //usage:#define kill_full_usage "\n\n"
>>  //usage:       "Send a signal (default: TERM) to given PIDs\n"
>>  //usage:     "\n       -l      List all signal names and numbers"
>> @@ -59,7 +59,7 @@
>>  //usage:       "$ kill 252\n"
>>  //usage:
>>  //usage:#define killall_trivial_usage
>> -//usage:       "[-l] [-q] [-SIG] PROCESS_NAME..."
>> +//usage:       "[-l] [-q] [-SIG] [--] PROCESS_NAME..."
>>  //usage:#define killall_full_usage "\n\n"
>>  //usage:       "Send a signal (default: TERM) to given processes\n"
>>  //usage:     "\n       -l      List all signal names and numbers"
>> @@ -184,6 +184,10 @@ int kill_main(int argc UNUSED_PARAM, char **argv)
>>         if (is_killall5 && arg[0] == 'o')
>>                 goto do_it_now;
>>
>> +       /* -- separates options from arguments */
>> +       if (!is_killall5 && arg[0] == '-')
>> +               goto do_it_sooner;
>> +
>>         if (argv[1] && arg[0] == 's' && arg[1] == '\0') { /* -s SIG? */
>>                 arg = *++argv;
>>         } /* else it must be -SIG */
>> @@ -192,6 +196,7 @@ int kill_main(int argc UNUSED_PARAM, char **argv)
>>                 bb_error_msg("bad signal name '%s'", arg);
>>                 return EXIT_FAILURE;
>>         }
>> + do_it_sooner:
>>         arg = *++argv;
>>
>>   do_it_now:
>> --
>> 2.13.5