Re: [PATCH v2 2/2] builtins: kill: Do not signal current process group when pid is empty

[Chris Down <chris@chrisdown.name>]

Daniel Shahaf writes:
>Chris Down wrote on Sat, Feb 15, 2020 at 20:21:09 -0400:
>> 1. Add special handling to `kill` to avoid this case. See this patch[0]
>>    for a version that does that.
>
>> 0: https://www.zsh.org/mla/workers/2020/msg00251.html
>
>Suggest to state "workers/45426" next to the link: that's how we customarily
>refer to mailing lists posts.  The number comes from the X-Seq header and is
>shown in the archives too.

Sure thing. :-)

>> +++ b/Test/B11kill.ztst
>> @@ -50,3 +50,15 @@
>>    kill -INT
>>  1:kill with sigspec only
>>  ?(eval):kill:1: not enough arguments
>> +
>> +# Regression tests: `kill ''` should not result in `kill 0`.
>> +
>> +  trap 'exit 19' TERM
>> +  kill ''
>> +1:Plain kill with empty pid should not send signal to current process group
>> +?(eval):kill:2: illegal pid:
>
>Should this test use a subshell so as not to kill the test suite process in
>case the bug regresses?
>
>Personally, I'm partial to first adding commits in "expected to fail" mode
>(using the «f» flag after the expected exit code) and then in a separate commit
>fix the bug, since doing so in effect validates the test.  However, I see this
>as a nice-to-hvae, not a blocker.

For this particular one, the subshell won't help, since we're part of the same 
process group as make and the test runner regardless:

     make: *** [Makefile:263: check] User defined signal 1
     make[1]: *** [Makefile:190: check] User defined signal 1
     make[1]: Leaving directory '/home/cdown/git/zsh/Test'

One possibility to avoid that is to run these few tests in a shell that's a new 
session leader, like in W02jobs, I suppose. What do you think about that?
>
>> +  trap 'exit 11' INT
>> +  kill -INT ''
>> +1:kill with empty pid and sigspec should not send signal to current process group
>> +?(eval):kill:2: illegal pid:
>
>Could you add a comment explaining the magic numbers 11 and 19?

Sure thing, will do in v3.

Thanks!

Chris