Re: [PATCH v3 4/6] nptl: pthread_rwlock: Move timeout validation into _full functions

[Mike Crowe <mac at mcrowe dot com>]

On Thursday 06 June 2019 at 16:18:05 -0300, Adhemerval Zanella wrote:
> 
> 
> On 27/05/2019 17:03, Mike Crowe wrote:
> > As recommended by the comments in the implementations of
> > pthread_rwlock_timedrdlock and pthread_rwlock_timedwrlock, let's move the
> > timeout validity checks into the corresponding pthread_rwlock_rdlock_full
> > and pthread_rwlock_wrlock_full functions. Since these functions may be
> > called with abstime == NULL, an extra check for that is necessary too.
> 
> Is this really a gain here? It just moves some code around and add
> an extra non required check. Wouldn't be better just to remove the
> comments?

It looks like the comment was added by Torvald Riegel in
cc25c8b4c1196a8c29e9a45b1e096b99a87b7f8c back in 2017.

>From what I can tell from the x86_64 disassembly, the compiler throws away the extra check because
these functions are all inline and it knows that abstime will be NULL.

However, I'm perfectly happy not to make this change, but it means that
I'll need to add the same checks to pthread_rwlock_clockwrlock and
pthread_wlock_clockrdlock.

I think that you've suggested consolidating these checks into a single
function in the past, which would make that easier. How about adding
is_valid_timespec or similar? Where would it be declared?

Thanks.

Mike.