[PATCH] staging/wlags49_h2: Fix build error when CONFIG_SYSFS is not set

Henk de Groot henk.de.groot at hetnet.nl
Thu Jun 17 21:07:50 UTC 2010


Hello Javier,

Op 17-6-2010 19:46, Javier Martinez Canillas schreef:
> 2) #ifdefs are ugly
>
> Code cluttered with ifdefs is difficult to read and maintain.  Don't do
> it. Instead, put your ifdefs in a header, and conditionally define
> 'static inline'
>    

That's very true. The wlags49_h2 driver is full of them and it would 
improve readability  the code to clean it up.

> So I discard this option. Do you want me to do it anyway and send a
> new patch? I'm willing to solve it the right way.
>    

Please do not listen to me. Your patch is fine. I'm not a regular kernel 
hacker so I don't know the rules either. Its more likely you know more 
about it than me already.

Of course I have idea's about the code and how to write it but I believe 
it is more important to have a unified standard across the kernel. I'm 
not seasoned enough to comment what's right or wrong with respect to the 
established coding standard. For example my style is to always put {} in 
the body of an "if" but I learned that its against the kernel coding 
standard if the body is a single statement. And that has to take have 
preference over my own ideas to get uniform code.

I know this driver does not keep up with the standards (yet?), 
especially the HCF library part (the files that do not start with a wl_ 
prefix). It might even be generated by some design tool because Agere 
did some strange things with structures and its very hard to read an 
follow as it is.

Anyway, please keep the patch as is, unless somebody with much more 
kernel knowledge comes along and tells us how it was supposed to be 
done. And I think they already said its fine now.

Kind regards,

Henk.




More information about the devel mailing list