Date: Sat, 9 Jan 2010 07:56:10 +1100 (EST) From: Bruce Evans <brde@optusnet.com.au> To: Edward Tomasz Napierala <trasz@FreeBSD.org> Cc: svn-src-head@FreeBSD.org, svn-src-all@FreeBSD.org, src-committers@FreeBSD.org Subject: Re: svn commit: r201794 - in head/sys: ddb dev/ep dev/ex netinet6 Message-ID: <20100109073716.E57804@delplex.bde.org> In-Reply-To: <201001081544.o08FinVh015359@svn.freebsd.org> References: <201001081544.o08FinVh015359@svn.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Fri, 8 Jan 2010, Edward Tomasz Napierala wrote:
> Log:
> Replace several instances of 'if (!a & b)' with 'if (!(a &b))' in order
> to silence newer GCC versions.
I don't like compilers warning about simple logical expressions (especially
&& vs || and & vs | precedence :-(), but as stefanf pointed out, the reason
to change this (or almost anything) is not to silence a broken compiler,
but to fix a bug.
> Modified: head/sys/ddb/db_ps.c
> ==============================================================================
> --- head/sys/ddb/db_ps.c Fri Jan 8 15:41:24 2010 (r201793)
> +++ head/sys/ddb/db_ps.c Fri Jan 8 15:44:49 2010 (r201794)
> @@ -136,7 +136,7 @@ db_ps(db_expr_t addr, boolean_t hasaddr,
> if (TD_ON_LOCK(td))
> lflag++;
> if (TD_IS_SLEEPING(td)) {
> - if (!td->td_flags & TDF_SINTR)
> + if (!(td->td_flags & TDF_SINTR))
> dflag++;
> else
> sflag++;
All of these bugs should have been avoided by using normal style. KNF
doesn't even use the "!" operator for testing simple booleans! Older
KNF code in kern uses comparisions with 0 fairly consistently for testing
flags in a bitmap being 0. Even the not-so-old KNF code in kern that
tests the not-so-old flag TDF_SINTR does this (there is only one instance,
in kern_sig.c, where the above is spelled correctly as
((td->td_flags & TDF_SINTR) == 0). I like to use ! for testing even sets
of flags so I prefer the above, but this is not KNF.
Spelling for testing the opposite sense is more mixed. Omitting the
!= 0 seems to be most common.
> @@ -171,7 +171,7 @@ db_ps(db_expr_t addr, boolean_t hasaddr,
> state[1] = '\0';
>
> /* Additional process state flags. */
> - if (!p->p_flag & P_INMEM)
> + if (!(p->p_flag & P_INMEM))
> strlcat(state, "W", sizeof(state));
> if (p->p_flag & P_TRACED)
> strlcat(state, "X", sizeof(state));
Also spelled correctly in kern (1 instance).
Other tests of P_INMEM in kern show the mixed style for putting parentheses
around the test for the opposite sense. Then the parentheses are optional
and are sometimes omitted in expressions like
(p->p_flag & P_INMEM && p->p_pid == pid).
Bruce
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20100109073716.E57804>