Date: Fri, 30 Nov 2007 10:04:54 +1100 (EST) From: Bruce Evans <brde@optusnet.com.au> To: "M. Warner Losh" <imp@bsdimp.com> Cc: des@des.no, arch@freebsd.org Subject: Re: Code review request: small optimization to localtime.c Message-ID: <20071130094705.E6718@besplex.bde.org> In-Reply-To: <20071129.084108.-713549098.imp@bsdimp.com> References: <20071128.151021.709401576.imp@bsdimp.com> <86lk8hhzs0.fsf@ds4.des.no> <20071129.084108.-713549098.imp@bsdimp.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Thu, 29 Nov 2007, M. Warner Losh wrote:
> In message: <86lk8hhzs0.fsf@ds4.des.no>
> Dag-Erling_Smørgrav <des@des.no> writes:
> : "M. Warner Losh" <imp@bsdimp.com> writes:
> : > Please find enclosed some small optimizations. [...]
> :
> : almost completely unrelated, but while you're at it:
> :
> : > if (__isthreaded != 0) {
> :
> : __isthreaded is clearly (by its name) a predicate, comparing it
> : explicitly to 0 is redundant and disrupts my flow of thought when
> : reading the code. Instead of just reading "if is threaded", I have to
> : take a second to parse the expression and check which way the comparison
> : goes.
> :
> : We already have a policy (unwritten as far as I know) of using explicit
> : comparisons for variables which are not clearly predicates, can we also
> : have one of *not* using explicit comparisons for those that are? And
> : document both cases in style(9)?
>
> True, but very Brucian in the nature of the comment: I didn't change
> this in existing code. :-)
KNF rules are sort of the opposite in some respects --
-- "if ((flags & MASK) != 0)", which is like the above, is slightly
more normal than "if (flags & MASK)".
-- The unary "!" operator is rarely used. "if (!isfoo)" and
"if (!(flags & MASK))" are not normal.
Anyway, there is too much existing code with bad style to change. I
draw the line (for non-booleans) between !error and !strcmp().
Bruce
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20071130094705.E6718>