[illumos-Developer] webrev for #948: chmod(1M) is not -R friendly when doing ACLs

[Yuri Pankov]

On Wed, Apr 27, 2011 at 09:46:23PM -0500, Jason King wrote:
> On Wed, Apr 27, 2011 at 8:52 PM, Yuri Pankov <yuri.pankov at gmail.com> wrote:
> 
> > Hi,
> >
> > This is webrev for #948: chmod(1M) is not -R friendly when doing ACLs.
> >
> > http://cr.illumos.org/view/jj0myccc/illumos-948-webrev/
> >
> > The idea is to make errors for all ACL-related operations non-fatal (as
> > they were supposed to be, it seems), while making two other cases fatal
> > (the "unknown ACL action" and error from acl_check - not sure how to
> > trigger both, but they really *do* seem to be fatal). I've also added
> > the full path to the files/dir in error messages, to make it more
> > user-friendly (there's no context in the error messages otherwise).
> >
> > The (really simple) testcase is:
> >
> > $ mkdir -p a/b/c/d; chmod A+user:yuri:full_set:allow a/b; \
> >  chmod A=user:root:full_set:allow a/b/c; /usr/bin/chmod -R \
> >  A-user:yuri:full_set:allow a; \
> >  ls -Vd a/b
> > chmod: ERROR: ACL entry doesn't exist
> > drwxr-xr-x+  3 yuri     staff          3 Apr 28 05:42 a/b
> >              user:yuri:rwxpdDaARWcCos:-------:allow
> >                 owner@:rwxp--aARWcCos:-------:allow
> >                 group@:r-x---a-R-c--s:-------:allow
> >              everyone@:r-x---a-R-c--s:-------:allow
> >
> > Here chmod fails on the first file not having specified ACE and just
> > exits.
> >
> >
> > $ mkdir -p a/b/c/d; chmod A+user:yuri:full_set:allow a/b; \
> >  chmod A=user:root:full_set:allow a/b/c; \
> >  ~/ws/illumos-gate-chmod-acl/usr/src/cmd/chmod/chmod -R \
> >  A-user:yuri:full_set:allow a; \
> >  ls -Vd a/b
> > chmod: WARNING: /home/yuri/a: ACL entry doesn't exist
> > chmod: WARNING: /home/yuri/a/b/c: ACL entry doesn't exist
> > chmod: WARNING: /home/yuri/a/b/c: Permission denied
> > drwxr-xr-x   3 yuri     staff          3 Apr 28 05:43 a/b
> >                 owner@:rwxp--aARWcCos:-------:allow
> >                 group@:r-x---a-R-c--s:-------:allow
> >              everyone@:r-x---a-R-c--s:-------:allow
> >
> > Here we have informative error (warning) messages, and chmod continues
> > to descend down the tree, removing the ACE where it does exist.
> >
> >
> > Not sure what are tests would be useful here as it's complete change in
> > chmod's behaviour for the -R case.
> >
> >
> chmod.c:730: This invokes getcwd on every file that has an acl on it.  Why
> not just pass in path from dochmod() to doacl() and avoid all the duplicate
> calls?  Otherwise looks good to me.

I've added some printfs to dochmod(), chmodr() and doacl() functions and
don't really see how I can improve it, hope the output will make it more
clear:

$ mkdir -p a/b/c/d; touch a/z a/b/y a/b/c/x a/b/c/d/w; \
  chmod A+user:yuri:full_set:allow a/b; \
  chmod A=user:root:full_set:allow a/b/c; \
  ~/ws/illumos-gate-chmod-acl/usr/src/cmd/chmod/chmod -R \
  A-user:yuri:full_set:allow a; ls -Vd a/b
dochmod: name=a path=a
chmodr: dir=a path=a
doacl: curdir=/home/yuri file=a
chmod: WARNING: /home/yuri/a: ACL entry doesn't exist
dochmod: name=b path=a/b
chmodr: dir=b path=a/b
doacl: curdir=/home/yuri/a file=b
dochmod: name=y path=a/b/y
doacl: curdir=/home/yuri/a/b file=y
chmod: WARNING: /home/yuri/a/b/y: ACL entry doesn't exist
dochmod: name=c path=a/b/c
chmodr: dir=c path=a/b/c
doacl: curdir=/home/yuri/a/b file=c
chmod: WARNING: /home/yuri/a/b/c: ACL entry doesn't exist
chmod: WARNING: /home/yuri/a/b/c: Permission denied
dochmod: name=z path=a/z
doacl: curdir=/home/yuri/a file=z
chmod: WARNING: /home/yuri/a/z: ACL entry doesn't exist
drwxr-xr-x   3 yuri     staff          4 Apr 28 09:17 a/b
                 owner@:rwxp--aARWcCos:-------:allow
                 group@:r-x---a-R-c--s:-------:allow
              everyone@:r-x---a-R-c--s:-------:allow


Yuri