Hi Ondrej, Still some formatting issues. But now, with formatting nearly right, I'm able to spot other things too.
> +# define ACE4_WHO_OWNER "OWNER@" > +# define ACE4_WHO_GROUP "GROUP@" > +# define ACE4_WHO_EVERYONE "EVERYONE@" > + > +# define ACE4_ACCESS_ALLOWED_ACE_TYPE 0 > +# define ACE4_ACCESS_DENIED_ACE_TYPE 1 > + > +/* ACE flag values */ > +# define ACE4_IDENTIFIER_GROUP 0x00000040 > +# define ROUNDUP(x, y) (((x) + (y) - 1) & - (y)) These are all inside a '# if' / '# endif' pair, so the preprocessor statement indentation level is 2. Please add one more space between '#' and 'define' here. > +int > +acl_nfs4_nontrivial (char *xattr, int len) > +{ > + int ret = 0, > + wholen, > + bufs = len; > + uint32_t flag, > + type, > + num_aces = ntohl (*((uint32_t*)(xattr))); /* Grab the number of > aces in the acl */ > + char *bufp = xattr; > + int num_a_aces = 0, > + num_d_aces = 0; > + > + ret = 0; 'ret' is already initialized to 0 above. One of the two initializations is redundant. Hmm... is the variable 'ret' used at all? You should routinely compile with "gcc -Wall"; this catches unused variables, usually. > + bufp += 4; /* sizeof(uint32_t); */ > + bufs -= 4; > + > + for (uint32_t ace_n = 0; num_aces > ace_n ; ace_n++) > + { > + int d_ptr; There are more variables which can be moved into the 'for' loop. Generally, moving variables into the smallest scope that they need provides for a better readability of the code. If I'm not mistaken, here, it applies to the variables wholen, flag, type. > + > + /* Get the acl type */ > + if (bufs <= 0) > + return -1; > + > + type = ntohl (*((uint32_t*)bufp)); > + > + bufp += 4; > + bufs -= 4; > + if (bufs <= 0) > + return -1; > + > + flag = ntohl (*((uint32_t*)bufp)); > + /* As per RFC 7530, the flag should be 0, but we are just generous to > Netapp > + * and also accept the Group flag > + */ > + if (flag & ~ACE4_IDENTIFIER_GROUP) > + return 1; > + > + /* we skip mask - > + * it's too risky to test it and it does not seem to be actually > needed */ > + bufp += 2*4; > + bufs -= 2*4; > + > + if (bufs <= 0) > + return -1; > + > + wholen = ntohl (*((uint32_t*)bufp)); > + > + bufp += 4; > + bufs -= 4; > + > + /* Get the who string */ > + if (bufs <= 0) > + return -1; > + > + /* for trivial ACL, we expect max 5 (typically 3) ACES, 3 Allow, 2 > deny */ > + if (((strncmp (bufp,ACE4_WHO_OWNER,wholen) == 0) || > + (strncmp (bufp,ACE4_WHO_GROUP,wholen) == 0)) && > + wholen == 6) Please move the || and && operators to the next line: if (((strncmp (bufp,ACE4_WHO_OWNER,wholen) == 0) || (strncmp (bufp,ACE4_WHO_GROUP,wholen) == 0)) && wholen == 6) Add add a space after each comma in an argument list. > + { > + if (type == ACE4_ACCESS_ALLOWED_ACE_TYPE) > + num_a_aces++; > + if (type == ACE4_ACCESS_DENIED_ACE_TYPE) > + num_d_aces++; > + } else Please put the closing brace and the 'else' on separate lines. > + if ((strncmp (bufp,ACE4_WHO_EVERYONE,wholen) == 0) > + && (type == ACE4_ACCESS_ALLOWED_ACE_TYPE) > + && (wholen == 9)) > + num_a_aces++; > + else > + return 1; > + > + d_ptr = ROUNDUP (wholen,4); > + bufp += d_ptr; > + bufs -= d_ptr; > + > + /* Make sure we aren't outside our domain */ > + if (bufs < 0) > + return -1; > + > + } > + return (num_a_aces <= 3) && (num_d_aces <= 2) > + && (num_a_aces + num_d_aces == num_aces) ? 0 : 1; You can simplify this: Knowing that boolean expressions have the value 1 for true and 0 for false in C, you can write it as return !((num_a_aces <= 3) && (num_d_aces <= 2) && (num_a_aces + num_d_aces == num_aces)); which is smaller and simpler. > diff --git a/lib/acl-internal.h b/lib/acl-internal.h > index 94553fab2..b674c7702 100644 > --- a/lib/acl-internal.h > +++ b/lib/acl-internal.h > @@ -146,6 +146,9 @@ rpl_acl_set_fd (int fd, acl_t acl) > # define acl_entries rpl_acl_entries > extern int acl_entries (acl_t); > # endif > +/* Return 1 if given ACL in XDR format is non-trivial > + * Return 0 if it is trivial */ > +extern int acl_nfs4_nontrivial (char *,int); Please add a space after the comma. > diff --git a/lib/file-has-acl.c b/lib/file-has-acl.c > index e02f0626a..82830ba4a 100644 > --- a/lib/file-has-acl.c > +++ b/lib/file-has-acl.c > @@ -32,6 +32,11 @@ > #if GETXATTR_WITH_POSIX_ACLS > # include <sys/xattr.h> > # include <linux/xattr.h> > +# include <arpa/inet.h> > +#ifndef XATTR_NAME_NFSV4_ACL > +# define XATTR_NAME_NFSV4_ACL "system.nfs4_acl" > +#endif Please indent the last three of these lines by one more space, after the '#'. Bruno