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. -- To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
