Hi Wolfgang, ----- Original Message ----- From: "Wolfgang Denk" wd@denx.de To: "Scott Wood" scottwood@freescale.com Cc: u-boot@lists.denx.de; "apgmoorthy" moorthy.apg@samsung.com Sent: Tuesday, March 24, 2009 4:16 AM Subject: Re: [U-Boot] [PATCH 1/2] Fix OneNAND ipl to read CONFIG_SYS_MONITOR_LEN
Dear Scott,
In message 49C80B99.5010808@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.
I agree with for better readabilty we will chnage and resend it today only. We will try to improve readability of code.
Thanks for suggestion. Amit
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@denx.de I'm frequently appalled by the low regard you Earthmen have for life. -- Spock, "The Galileo Seven", stardate 2822.3 _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot