Re: [PATCH v2 2/4] manual: Add documentation for pthread_tryjoin_np and pthread_timedjoin_np

[Adhemerval Zanella <adhemerval dot zanella at linaro dot org>]

On 18/10/2019 13:28, Mike Crowe wrote:
> Since pthread_timedjoin_np is not new, we risk changing the behaviour of
> existing code that may have come to rely on the behaviour when passing
> NULL. Even if we use symbol versioning to ensure that code compiled against
> old glibc continues to behave as it always did, we still risk changing the
> behaviour when code is compiled against the newer glibc. If that code is
> compiled with -Wnonnull and the compiler is able to determine that NULL is
> being passed (which it may not if the code in question is using a wrapper
> function) then a warning would be emitted which may help, but does not
> guarantee that the change will be noticed.

At least for code built against a new symbol we can not only add the nonnull
but at result EINVAL for invalid abstime. It would be semantic change, but
it is what compat symbol are used mainly.

> 
> pthread_clockjoin_np is new, so it doesn't have any legacy behaviour. We
> can mark the timeout as __nonnull. We could check the argument against NULL
> and return EINVAL, but it seems that the compiler warns when checking
> __nonnull arguments and just throws the check away[1] so we're probably
> stuck with potentially faulting in that situation.
> 
> In my view, changing the behaviour of pthread_timedjoin_np now is of little
> benefit compared to the risk of breaking existing code. That ship has
> sailed. Having decided that, I thought that making pthread_clockjoin_np
> behave differently to pthread_timedjoin_np would be confusing, so I settled
> on it behaving the same way.
> 
> So, what are you proposing we should do? Change the behaviour of the
> existing pthread_timedjoin_np function and the new pthread_clockjoin_np
> function or only of the latter?

I agree with your rationale that we should not change pthread_timedjoin_np
and mimic its behaviour for pthread_clockjoin_np (in fact I acked the patches
already).

This discussion was mostly due the proposition Yann Droneaud brought to you
and you mentioned in the email.  I think the patchset is good to be pushed
upstream.


> 
> Thanks.
> 
> Mike.
> 
> [1] https://godbolt.org/z/Msw9vk
>