Hi Wolfgang, ----- Original Message ----- From: "Wolfgang Denk" <w...@denx.de> To: "Scott Wood" <scottw...@freescale.com> Cc: <u-boot@lists.denx.de>; "apgmoorthy" <moorthy....@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.5010...@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: > w...@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 _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
