[PATCH v4] Fix Linux fcntl OFD locks for non-LFS architectures (BZ#20251)

[Adhemerval Zanella]

On 26/06/2018 11:45, Florian Weimer wrote:
> On 06/22/2018 02:20 PM, Adhemerval Zanella wrote:
>>>> +Â Â Â Â Â  /* case F_OFD_GETLK:
>>>> +Â Â Â Â Â Â Â Â  case F_OFD_GETLK64:
>>>> +Â Â Â Â Â Â Â Â  case F_SETLK64:
>>>> +Â Â Â Â Â Â Â Â  case F_GETOWN:Â  */
>>>> +Â Â Â Â Â  default:
>>>> +Â Â Â Â Â Â Â  return __fcntl64_nocancel_adjusted (fd, cmd, arg);
>>>> +Â Â Â  }
>>>> Â Â  }
>>>
>>> The comment before the default case looks wrong to me.  F_OFD_GETLK is duplicated.  Maybe add comments for the cases where mapping is not needed, explaining why.
>>
>> I changed to:
>>
>> Â Â Â Â Â Â  /* Since only F_SETLKW{64}/F_OLD_SETLK are cancellation entrypoints and
>> Â Â Â Â Â Â Â Â  only OFD locks requires LFS handling, all others flags are handled
>>          unmodified by calling __NR_fcntl64.  */
> 
> Thanks.  I think it should read “only OFD locks require LFS handling”.

Fixed.

> 
>>>> +# include <shlib-compat.h>
>>>> +# if SHLIB_COMPAT(libc, GLIBC_2_0, GLIBC_2_28)
>>>> +int
>>>> +__old_libc_fcntl64 (int fd, int cmd, ...)
>>>> +{
>>>> +Â  va_list ap;
>>>> +Â  void *arg;
>>>> +
>>>> +Â  va_start (ap, cmd);
>>>> +Â  arg = va_arg (ap, void *);
>>>> +Â  va_end (ap);
>>>> +
>>>> +Â  return __libc_fcntl64 (fd, cmd, arg);
>>>> +} > +compat_symbol (libc, __old_libc_fcntl64, fcntl, GLIBC_2_0);
>>>
>>> This should have a comment why you call it __old_libc_fcntl64, when there never was a fcntl64 before.
>>
>> I added.
>>
>> Â Â  /* Previous versions called __NR_fcntl64 for fcntl (which do not handle
>>       OFD locks in LFS mode).  */
> 
> “which did not handle”?
> 

Indeed, fixed.

I will push it shortly.