svn commit: r201794 - in head/sys: ddb dev/ep dev/ex netinet6

Bruce Evans brde at optusnet.com.au
Fri Jan 8 20:56:13 UTC 2010


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


More information about the svn-src-all mailing list