[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