[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