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

[Florian Weimer <fweimer at redhat dot com>]

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”.

> > > +# 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”?

Florian