This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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



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.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]