Re: [PATCH v3 2/4] compat/mingw: allow sigaction(SIGCHLD)

From: Phillip Wood <phillip.wood123@gmail.com>
Date: Jun 26, 2025, 13:56 UTC
Message-ID: <a1fb8c27-6ddf-42d5-a062-a9710f6cc1cd@gmail.com>
In-Reply-To: <qizh636elher65bsdzkiqohzyo23tmon7hxcl4jcuftculbtm6@nupmqjy3igja>

On 26/06/2025 14:15, Carlo Marcelo Arenas Belón wrote:

Show 20 quoted lines

> 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@gmail.com>
>>>
>>> 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.

Thanks

Phillip

Show 6 quoted lines

> so you can
> have the later be literally SIG_ERR, eventhough it will be ironically
> equivalent it casted into an int.
> 
> Csrlo
>