On 25/01/2024 19:52, Grisha Levit wrote:
>> On Thu, Jan 25, 2024, 09:50 Pádraig Brady <P@draigbrady.com> wrote:
>> This mostly looks good, except:
>>
>> - No need to clear the errno before kill(3).
>> - Better to use SIG%d rather than the bare %d for signal _names_, as we already parse this format
> 
> Makes sense, done below.
> 
> * src/operand2sig.c (operand2sig): Drop signame argument, accept all
> signal numbers <= SIGNUM_BOUND.  All callers updated.
> * src/env.c (parse_signal_action_params, reset_signal_handlers)
> (parse_block_signal_params, set_signal_proc_mask)
> (list_signal_handling): Accept all signal numbers <= SIGNUM_BOUND,
> use SIG%d for printing if necessary.
> * src/kill.c (list_signals, main): Likewise.
> (send_signals): Check errno from kill(3) for bad signo.
> * src/timeout.c (main): Update operand2sig call.
> * tests/misc/kill.sh: Test listing all signal numbers.

I've made a few adjustments:

- Clarified the commit message.

- I've gone back to have kill -l produce a bare number for unnamed signals, because:
this is consistent with util-linux,
SIG%d is only parseable by coreutils (not util-linux or bash),
easier to programatically determine if a name is defined.
I've left as SIG%d for -t output as that's a coreutils specific option,
and not really programatically consumable anyway.

- I've added a validation check for `env --block=32` so that it fails
like `env --default=32` and `env --ignore=32`.
I.e. exits with EXIT_CANCELED.

- Added a NEWS entry.

I'll apply this later.

thanks!
Pádraig