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

[Michael Adam]

On 2016-03-11 at 08:31 -0800, Jeremy Allison wrote:
> 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.

If you want to add a macro for clarity, it should
then read something like this:

   if (!posix_pathnames(fsp)) ...

Cheers - Michael
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20160311/e044291a/signature.sig>