Code readability (Was: [PATCH] More smb_filename cleanups.)

[Jeremy Allison]

On Fri, Mar 11, 2016 at 10:49:40AM +0100, Ralph Boehme wrote:
> On Fri, Mar 11, 2016 at 07:59:05AM +0100, Andreas Schneider wrote:
> > On Thursday 10 March 2016 19:15:52 Jeremy Allison wrote:
> > > Here is a short, easy to review patchset
> > > that removes one lp_posix_pathname, fixes
> > > on VFS module that got missed in the get_nt_acl_fn
> > > changes, and finally fixes up some of the functions that
> > > call into utility functions that call
> > > lp_posix_pathnames() to take a const
> > > struct smb_filename * instead of a
> > > const char *.
> > 
> > General question, not really patchset related.
> > 
> > Seeing this:
> > 
> > +		    !(fsp->posix_flags & FSP_POSIX_FLAGS_PATHNAMES) &&
> > 
> > would it makes sense for code readability to add a macro in a prominent place:
> > 
> > #define isflagset(flagfield, flag) (((flagfield) & (flag)) == (flag))
> > 
> > then use:
> > 
> > !isflagset(fsp->posix_flags, FSP_POSIX_FLAGS_PATHNAMES)
> 
> I'd prefer the direct binary operator in front of me instead of adding
> a layer.

Yes. Adding that macro doesn't make things clearer for me.

Now, back to reviewing the code.... :-) :-).