Re: Code review request: small optimization to localtime.c

Bruce Evans <brde@xxxxxxxxxxxxxxx> writes:
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.

This is not "opposite" - as I said, we have a rule about when we
*should* use an explicit comparison, but we lack a rule about when we
*should not*. I think we *should not* use an explicit comparison when
the expression being tested is obviously a predicate, for instance when
it is a variable or a call to a function whose name begins with "is",
"can" or similar. A corollary is that variables and functions *should
not* have names that begin with "is", "can" or similar unless they can
be used correctly without an explicit comparison. You should never have
to write something like "if (__isthreaded == 5)".

BTW, (flags & MASK) is a poor example; depending on the value of MASK,
there may actually be several distinct non-zero values, so an explicit
comparison is justified.

Anyway, there is too much existing code with bad style to change. I
draw the line (for non-booleans) between !error and !strcmp().

I loathe !strcmp(), but I also generally try to avoid !error.

DES
--
Dag-Erling Smørgrav - des@xxxxxx
_______________________________________________
freebsd-arch@xxxxxxxxxxx mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-arch
To unsubscribe, send any mail to "freebsd-arch-unsubscribe@xxxxxxxxxxx"

Relevant Pages

  • [PATCH 03/18] flag parameters: paccept
    ... This syscall differs from accept in that it adds ... The flags parameter can be used to set flag like SOCK_CLOEXEC. ... we really want the signal mask ... handler (int s) ...
    (Linux-Kernel)
  • [PATCH 02/18] flag parameters: paccept
    ... This syscall differs from accept in that it adds ... The flags parameter can be used to set flag like SOCK_CLOEXEC. ... we really want the signal mask ... handler (int s) ...
    (Linux-Kernel)
  • [patch 30/44] parisc: use generic bitops
    ... _atomic_spin_unlock_irqrestore(addr, flags);} ... {unsigned long mask = 1UL << CHOP_SHIFTCOUNT; ... * @addr: The address to start the search at ... * Returns the bit-number of the first set bit, not the number of the byte ...
    (Linux-Kernel)
  • Re: [PATCH 1/4] x86 paravirt_ops: create no_paravirt.h for native ops
    ... * Set IOPL bits in EFLAGS from given mask ... +static inline void set_iopl_mask(unsigned mask) ... all non IF manipulations of flags are separated that would help. ... in our case, this is not even a hypercall, merely a VMI call). ...
    (Linux-Kernel)
  • [PATCH 6/9] define first set of BIT* macros
    ... unsigned long flags; ... static inline void clear_bit(int nr, volatile unsigned long *addr) ...
    (Linux-Kernel)