Re: [PATCH v11] posix: Deprecate group_member for Linux

[Joe Simmons-Talbott <josimmon@redhat.com>]

On Wed, Mar 27, 2024 at 4:57 PM Paul Eggert <eggert@cs.ucla.edu> wrote:
>
> On 3/27/24 12:43, Joe Simmons-Talbott wrote:
> > +  int granted;
> > +
> > +  if (uid == stats.st_uid)
> > +    granted = (unsigned int) (stats.st_mode & (mode << 6)) >> 6;
> > +  else
> > +    {
> > +      if (((flag & AT_EACCESS) && stats.st_gid == __getegid()) ||
> > +          (!(flag & AT_EACCESS) && stats.st_gid == __getgid()))
> > +        granted = (unsigned int) (stats.st_mode & (mode << 3)) >> 3;
> > +      else
> > +        {
> > +          int gm = __group_member2 (stats.st_gid);
> > +          if (gm == -1)
> > +            return -1;
> > +          else if (gm)
> > +            granted = (unsigned int) (stats.st_mode & (mode << 3)) >> 3;
> > +          else
> > +            granted = (stats.st_mode & mode);
> > +        }
> > +    }
> >
> >     if (granted == mode)
> >       return 0;
>
> This is still too complicated, with repetitions of AT_EACCESS that were
> not in the original. Come to think of it, it has unnecessary casts and
> repetitive shifting and masking, and that indentation of || is not GNU
> style. How about something like the following untested code instead?
>
>    int shift;
>
>    if (uid == stats.st_uid)
>      shift = 6;
>    else
>      {
>        int gm = (stats.st_gid
>                 == (flag & AT_EACCESS ? __getegid () : __getgid ()));
>        if (!gm)
>         {
>            gm = __group_member2 (stats.st_gid);
>            if (gm < 0)
>              return gm;
>          }
>        shift = gm ? 3 : 0;
>      }
>
>    if ((stats.st_mode >> shift & mode) == mode)
>      return 0;
>

I like that much better.  I've posted a v12[1] with your suggestion
that was tested via 'make check'.

Thanks,
Joe

[1] https://sourceware.org/pipermail/libc-alpha/2024-March/155666.html

-- 
Joe Simmons-Talbott