Re: setfacl Recursive Functionality

From: Shawn Webb <lattera_at_gmail.com>
Date: Wed, 9 Feb 2011 14:10:42 -0700
Included in the attached patch is the refactor using fts(3) and with the -L
and -H options. I'm still looking for feedback and suggestions on how to
improve the patch. I'll also port these changes over to my getfacl patch.

If anyone's interested in following up-to-date development of the patch, the
link to it on github is
https://github.com/lattera/patches/blob/master/freebsd/setfacl_recursive.patch

I'd like to take the time to address why I created the
remove_invalid_inherit function since I got a private email asking why it
existed. Other than symbolic links, non-directory entries cannot have
inheritance set. That function prevents attempting to set inheritance flags
on non-directory entries when doing a recursive call. That way, you can run
`setfacl -R -m user:<user>:read_data:file_inherit/dir_inherit:allow
<directory>` and not run into errors.

Thanks,

Shawn

On Tue, Feb 8, 2011 at 7:51 PM, Shawn Webb <lattera_at_gmail.com> wrote:

> On Tue, Feb 8, 2011 at 7:35 PM, Tim Kientzle <tim_at_kientzle.com> wrote:
>
>> On Feb 8, 2011, at 9:58 AM, Shawn Webb wrote:
>> > I've just finished a patch to add recursive functionality to setfacl.
>> Before
>> > I officially submit it, I'd like a few suggestions on how to improve the
>> > patch.
>> >
>> > The part I'm worried about involves the #define directive at top. I'm
>> not
>> > sure what ramifications using that define might have. I needed it for my
>> > remove_invalid_inherit() function to work.
>>
>> You should certainly not need
>>   #define _ACL_PRIVATE
>> for any user-space utilities.  What exactly is the
>> problem without that?
>>
>> Your approach to directory walking here
>> is a little simplistic.  In particular, you're storing
>> every filename for the entire tree in memory,
>> which is a problem for large filesystems.
>>
>> It would be much better to refactor the code so that
>> the actual ACL update was in a function and then
>> recurse_directory should call that function for
>> each filename as it visited it.  That will reduce
>> the memory requirements significantly.
>>
>> You should also take a look at fts(3).  In particular,
>> you'll want to implement the BSD-standard
>> -L/-P/-H options, and fts(3) makes that much easier.
>> (-L always follows symlinks, -P never follows symlinks,
>> -H follows symlinks on the command line).
>>
>> Tim
>>
>>
> Great suggestions. I'll definitely look at implementing that functionality.
>
> As a side note, it looks like my setfacl patch segfaults on freebsd-current
> r218075 with the zpool v28 patchset applied. I wrote it on freebsd 8.2-RC3
> with zpool v15.
>
> Thanks,
>
> Shawn
>

Received on Wed Feb 09 2011 - 20:10:44 UTC

This archive was generated by hypermail 2.4.0 : Wed May 19 2021 - 11:40:11 UTC