On 26/06/2025 15:58, Carlo Marcelo Arenas Belón wrote:
On Thu, Jun 26, 2025 at 02:56:22PM -0800, Phillip Wood wrote:
On 26/06/2025 14:15, Carlo Marcelo Arenas Belón wrote:
On Thu, Jun 26, 2025 at 01:52:47PM -0800, Phillip Wood wrote:
On 26/06/2025 09:53, Carlo Marcelo Arenas Belón via GitGitGadget wrote:
From: =?UTF-8?q?Carlo=20Marcelo=20Arenas=20Bel=C3=B3n?= <carenas@xxxxxxxxx>
A future change will start using sigaction to setup a SIGCHLD signal
handler.
The current code uses signal() which returns SIG_ERR (but doesn't
seem to set errno) so instruct sigaction() to do the same.
Why are we returning -1 below instead of SIG_ERR if we want the behavior to
match?
By "match", I mean that in both cases we will get an error return value
and errno won't be set to EINVAL (which is what POSIX requires)
In our codebase since we ignore the return code anyway, it wouldn't make
a difference, either way.
signal() returns a pointer, and sigaction() returns and int,
Oh right, I'd forgotten they have different return types. I think we should
probably be setting errno = EINVAL before returning -1 to match what this
function does with other signals it does not support - just because our
current callers ignore the return value doesn't mean that future callers
will and they might want check errno if they see the function fail.
I agree, and indeed had to triple check and change my implementation after I
confirmed that signal(SIGCHLD) does not change errno on Windows (not our
version, neither of the windows libc or mingw, even if it is documented[1] to
do so.
It might be because the signal number itself is bogus (there is none for
SIGCHLD in their headers, and git uses their own numbers in compat), but
either way, I would rather be consistent with signal() at least originally.
I'm not sure I understand - don't we want the sigaction() wrapper to
behave like sigaction() would?
Thanks
Phillip
Carlo
[1] https://learn.microsoft.com/en-us/cpp/c-runtime-library/reference/signal
