[PATCH v3] Cygwin: Change pthread_sigqueue() to accept thread id

[Mark Geisert]

Hi Corinna,

On 11/7/2024 3:13 AM, Corinna Vinschen wrote:
> Hi Mark,
> 
> On Nov  6 23:29, Mark Geisert wrote:
>> Change the first parameter of pthread_sigqueue() to be a thread id rather
>> than a thread pointer. The change is to match the Linux implementation of
>> this function.
>>
>> The user-visible function prototype is changed in include/pthread.h.
>> The pthread_sigqueue() function is modified to work with a passed-in thread
>> id rather than an indirect thread pointer as before.  (It was
>> "pthread_t *thread", i.e., class pthread **.)  The release note for Cygwin
>> 3.5.5 is updated.  CYGWIN_VERSION_API_MINOR is bumped to 351.
> 
> This patch is against cygwin-3_5-branch.  All patches should go to the
> main branch in the first place and only then backported to the 3.5
> branch if it's a bugfix.  So the patch sent to cygwin-patches should
> always be against main.

Oops & Sheesh. Consider me reminded of this basic rule :-()

> That leads to a problem with your patch.  CYGWIN_VERSION_API_MINOR is
> already at 355 in the main branch, while it's at 350 in the 3.5 branch.
> That means, if your patch bumps CYGWIN_VERSION_API_MINOR, it would have
> to bump to 356.  However, 356 doesn't make sense in the 3.5 branch,
> and setting it to 351 in the 3.5 branch would collide with the 351
> bump in main having a different meaning.
> 
> The way out is, your patch should not bump CYGWIN_VERSION_API_MINOR
> at all.  Rather, just add a comment to version.h. There's already the
> note preceding the definition of CYGWIN_VERSION_API_MAJOR:
> 
>    Note that we forgot to bump the api for ualarm, strtoll, strtoull,
>    sigaltstack, sethostname. */
> 
> Just add the pthread_sigqueue bugfix here if you like, along the lines of
> 
>    Note that fixing the first pthread_sigqueue argument did not bump the api.
> 
> Alternatively we could just go ahead without touching version.h.

I like the latter alternative best.  v4 incoming shortly...

>> Reported-by: Christian Franke <Christian.Franke@t-online.de>
>> Addresses: https://cygwin.com/pipermail/cygwin/2024-September/256439.html
>> Signed-off-by: Mark Geisert <mark@maxrnd.com>
>> Fixes: 2041af1a535a (cygwin.din (pthread_sigqueue): Export.)
> 
>    $ git show 2041af1a535a
>    fatal: ambiguous argument '2041af1a535a': unknown revision or path not in the working tree.
>    Use '--' to separate paths from revisions, like this:
>    'git <command> [<revision>...] -- [<file>...]'
> 
> Shouldn't that be 50350cafb375?  The Fixes commit message text is
> supposed to be set in quotes,btw.:
> 
>    Fixes: 50350cafb375 ("* cygwin.din (pthread_sigqueue): Export.")

Of course, on the formatting.  Re incorrect hash: I must have flubbed 
the copy/paste from cgit by pointing at the wrong thing.  I'll verify 
where I went wrong there.

> Other than the above points, the patch itself is fine.

Thank you for the review.

..mark