[U-Boot] [PATCH 1/2] Fix OneNAND ipl to read CONFIG_SYS_MONITOR_LEN

Wolfgang Denk wd at denx.de
Mon Mar 23 23:46:23 CET 2009


Dear Scott,

In message <49C80B99.5010808 at freescale.com> you wrote:
>
> >>> +	if (page < 2 && (onenand_readw(ONENAND_SPARERAM) != 0xffff))
> >>> +		return 1;
> >> Unnecessary parens.
> > 
> > Where? I find them pretty useful.
> 
> Around the second comparison.  Why "if (a < b && (c != d))" and not "if 
> (a < b && c != d)", or if the parens are preferred, "if ((a < b) && (c 
> != d))"?  Is it because "c" is a function call?

Actually I'd probably write this as

	if ((page < 2) && (onenand_readw(ONENAND_SPARERAM) != 0xffff))

for consistency, but being lazy I guess I might use the same code as
the OP.

> OK -- I guess this is another of the unwritten points on which U-boot's 
> style deviates from that which is typical in Linux.

Actually this is not some formal style (at least no rule that I know
of), but personal taste :-)

When I parse something like

	if (page < 2 && onenand_readw(ONENAND_SPARERAM) != 0xffff)

I can eaisly see the "page < 2" expression because it  is  relatively
short. But I have to look twice for the "onenand_readw(ONENAND_SPARE-
RAM)  !=  0xffff" part because it is long and includes nested parens.
So I feel tempted to make this easier to read by surrounding it  with
parens - like the OP did.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
I'm frequently appalled by the low regard you Earthmen have for life.
	-- Spock, "The Galileo Seven", stardate 2822.3


More information about the U-Boot mailing list