[PATCH] gta03-pca9632.patch
matt_hsu
matt_hsu at openmoko.org
Tue Sep 16 05:52:43 CEST 2008
Werner Almesberger wrote:
> michael wrote:
>
>> Same problem :) *kzalloc*
>>
>
> Yup, assigments in expressions are generally a bad idea.
> GCC will warn you if you
> if (foo = bar())
> and it will also warn you if you
> foo == bar()
> but with
> if ((foo == bar()))
> all bets are off. Besides, such "hidden" assignments are easily
> overlooked.
>
> Also, a bit more confidence in operator precedence wouldn't be
> amiss :-) E.g,
> return ((idc*100)/256);
> could just be
> return idc*100/256;
>
> Extra parentheses where they're obviously not needed just confuse
> the reader, because they suggest that there is actually some subtle
> reason why they're here, and they also make things harder to read.
> E.g.,
> led_state = ((led_state >> (2 * ldrx)) & 0x03) ;
>
I see. Lesson learned. :)
Matt
> Besides this nitpicking, I didn't find anything to complain about :-)
> Nice work !
>
> Ah, one thing. It would be good to clearly mark code that's just
> for general review/comments and not for inclusion into our tree. How
> about using neither [PATCH] in the subject nor Signed-off-by unless
> it's ready for inclusion ?
>
> - Werner
>
More information about the openmoko-kernel
mailing list