Re: [Acl-devel] typo in acl_get_perm?

[Corinna Vinschen]

Hi Andreas,

On Dec 26 15:46, Andreas Grünbacher wrote:
> 2015-12-26 14:34 GMT+01:00 Corinna Vinschen <address@hidden>:
> > On Dec 26 11:57, Andreas Grünbacher wrote:
> >> 2015-12-25 15:36 GMT+01:00 Corinna Vinschen <address@hidden>:
> > [...]
> > The man page doesn't make it clear that only a single permission bit is
> > allowed as input.
> 
> "The [...] function tests if the permission [...] is contained in the
> [...] set."
> 
> So again, what's unclear about that?

Not trying to put too fine a point on it, but isn't the fact that I
misunderstood an indicator that, perhaps, more than a single person
might misunderstand it?  I'm not the only dumb person out there ;)

> > Come to think of it, the test is wrong anyway.  It's always 0 because it's
> > using the wrong "not"-operator (! vs ~).  The same problem exists at least
> > in two other files as well, acl_add_perm.c and acl_delete_perm.c.
> > Patch attached.
> 
> That's a real bug, indeed. I'm pushing your fix, thank you.

Thanks to you!

> > Actually, I think that testing if all permission bits given in the input
> > are contained in the permission set makes more sense and is more in line
> > with the behaviour of acl_add_perm and acl_delete_perm.  So I dare to
> > propose the second patch to test for any perms in the input being
> > contained in the patchset.
> 
> This function has behaved that way for the last 13 years. It is very
> unlikely but still possible that there is code out there that depends
> on the current behavior, so I am reluctant to change the behavior now.

I understand the sentiment, but the existing test is kind of borderline.
Even if you only want to test a single bit, the test I'm proposing will
still work as far as the man page was originally meant.  Additionally it
will also allow to check for a permission set being contained in
permset_d, while the old test would outright fail.  So, from my point of
view, it's an improvement.  But of course it's your choice, so I won't
insist further.  No worries :)


Thanks,
Corinna

signature.asc
Description: PGP signature