Re: setfacl Recursive Functionality

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