The following case was encountered in the wild:
% zsh; echo "$?"
% trap 'exit 5' TERM
% kill ''
5
This behaviour seems more likely to be the result of bugs in programs
(e.g. `kill -9 "$unsetvar") rather than being desirable
behaviour to me. It also seems unintentional judging by the code and
documentation, since it comes about as a result of the fact that:
- `isanum` returns true for empty strings (since an empty string
technically only consists of digits and minuses...);
- `atoi`, when passed a pointer to an invalid number, returns 0;
- `kill(0, signal)` sends the signal in question to all processes in the
current process group.
There are (at least) two ways to solve this issue:
1. Add special handling to `kill` to avoid this case. See this patch[0]
for a version that does that.
2. Change how isanum behaves. Since the only two call sites that use it
both seem like they should handle the case where the input char array
is empty, that seems like a reasonable overall change to me.[1]
After this patch:
% trap 'exit 5' TERM
% kill ''
kill: illegal pid:
The regression test for `kill` without a sigspec is also included in
this commit, as previously it's not possible to test it trivially as it
would still kill the test runner in expected-to-fail mode; see
discussion in workers/45449.
0: workers/45426: https://www.zsh.org/mla/workers/2020/msg00251.html
1: The other call site using isanum() is the fg builtin, but in that
case we just fail later since we can't find any job named '', so no
big deal either way. It's the kill case which is more concerning.
The version without a sigspec can't be added yet because it would still
kill the test runner even in expected-to-fail mode; see workers/45449
for discussion. For the same reason, we use a signal which is non-fatal
by default and unlikely to be sent by someone else, SIGURG, to do the
expected-to-fail case prior to the fix.
This is not totally comprehensive, but at least it's a start for the
core functionality. In the next commits, we'll also use this base to add
some regression tests.
- Fix retval handling in bin_setopt()
- Don't skip_setuid / skip_setgid. It's not our place to optimize away noops
(that might not even _be_ noops; they might change the saved uid…).
- Remove HAVE_* guard checks around functions that are used unguarded elsewhere.
- Use bsd-setres_id.c from OpenSSH to provide setresuid() / setresgid()
everywhere, and thus simplify the ifdef soup. Fix some preëxisting
bugs in the macro definitions of setuid() (do we still need that one?).
- Fix zwarning() format codes for variadic arguments type safety
- Restored a comment from HEAD
- Fix failure modes around initgroups()
- Compared privilege restoration code with OpenSSH's permanently_drop_uid() and
updated as needed
- Add E01 PRIVILEGED sanity checks
As an alternative, one can use:
autocmd BufRead,BufNewFile **/*.ztst setfiletype zsh
(But see Util/ztst-syntax.vim and Util/ztst-ftplugin.vim for an alternative.)
Having reviewed 20076, 20084, 21734, and 21735, my understanding is that
the original intention was:
- A 'return' in a function does run always-list
- An 'exit' outside a function does not run always-list
- A 'return' outside a function is treated as an 'exit'
All of which are the case today. The remaining case, of 'exit' used
inside a function, was not specified by the referenced -workers@ posts;
does, as implemented, run the always-list; and furthermore, based in
21734 it's fair to assume that the original documentation was assuming
that 'exit' would be used outside of any function, just like it assumed
'return' would be used inside a function.
Therefore, have the documentation specify only the behaviour of 'exit'
outside any function, and leave the behaviour of 'exit' inside
a function unspecified. Anyone who relied on the documentation of 'exit'
as documented until this commit would have run into the
documentation/implementation discrepancy described in 45075.
Pick number of leading or trailing path components to substitute.
Active in history, brace parameters, glob qualifiers.
Add tests for all three environments.
* Enable zparseopts to perform basic usage validation (aborting on an
unrecognised option-like parameter)
* Officially document the resolution of ambiguous option specs
