Re: CVS commit: src/sys/miscfs/genfs

[Mindaugas Rasiukevicius <rmind%netbsd.org@localhost>]

Taylor R Campbell <campbell+netbsd-source-changes-d%mumble.net@localhost> wrote:
>    Date: Wed, 12 Mar 2014 16:16:32 +0200
>    From: Jukka Ruohonen <jruohonen%iki.fi@localhost>
> 
>    On Wed, Mar 12, 2014 at 09:39:23AM +0000, Juergen Hannken-Illjes wrote:
>    > Restructure layer_lock() to always lock before testing for dead node.
>    > Use ISSET() to test flags, add assertions.
> 
>    As I wrote in the manual page, I'd rather see ISSET(3) et. al.
> disappear, i.e. these obscure rather than clarify...
> 
> I disagree.  Phrases like `(vp->v_iflag & (VI_XLOCK | VI_CLEAN)) == 0'
> make my head's parser stumble -- there are just enough complements to
> juggle that it overwhelms my brain registers for the fast path.  I'd
> rather read `!ISSET(vp->v_iflag, (VI_XLOCK | VI_CLEAN))'.

I disagree.  For kernel developers, that kind of bitwise arithmetics and
masking ought to be intuitive.  If there is more logic and it gets long,
then separate it:

const bool foobar = (mask & (FOO | BAR)) == 0;
const bool baz = (mask & BAZ) != 0;

if (foobar && baz) ...

ISSET() is somewhat okay (although I do not use it), but I particularly
dislike __BIT() as I forget whether the 1st bit is n = 0 or whether this
API tries to be fancy and it is n = 1.  1U << n is just straigtforward.

-- 
Mindaugas