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. Carlo [1] https://learn.microsoft.com/en-us/cpp/c-runtime-library/reference/signal
