Re: [Acl-devel] typo in acl_get_perm?

[Andreas Grünbacher]

2015-12-26 14:34 GMT+01:00 Corinna Vinschen <address@hidden>:
> On Dec 26 11:57, Andreas Grünbacher wrote:
>> Corinna,
>>
>> 2015-12-25 15:36 GMT+01:00 Corinna Vinschen <address@hidden>:
>> > I was wondering about a discrepancy between the man page for
>> > acl_get_perm and the actual implementation and I'm not sure if I just
>> > misunderstand the docs.
>> >
>> > The man page claims:
>> >
>> >   The acl_get_perm() function tests if the permission specified by the
>> >   argument perm is *contained* in the ACL permission set pointed to by
>> >   the argument permset_d.
>> >
>> > To me "contained" implies that all bits given in perm has to be set
>> > in the permissions permset_d refers to
>>
>> this function supports testing for one permission, not for a set of
>> permissions. The man page seems clear about that; I'm unsure what's
>> confusing you.
>
> 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?

> Also, the test for incoming permission bits is not testing for a single bit
> and failing if more than one is set.

The behavior when passing more than one permission is not defined.

> It only looks that no other bit than the allowd RWX bits is set.
>
> 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.

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

Thanks,
Andreas