staging/wlags49_h2: Fix build error when CONFIG_SYSFS is

Hello,

Didn't get any feedback so resending the patch.

I got a wlags49_h2 driver build error in linux-next when CONFIG_SYSFS is not set.

CC [M] drivers/staging/wlags49_h2/wl_cs.o
In file included from drivers/staging/wlags49_h2/wl_cs.c:104:
drivers/staging/wlags49_h2/wl_sysfs.h: In function ‘register_wlags_sysfs’:
drivers/staging/wlags49_h2/wl_sysfs.h:5: error: parameter name omitted drivers/staging/wlags49_h2/wl_sysfs.h: In function ‘unregister_wlags_sysfs’:
drivers/staging/wlags49_h2/wl_sysfs.h:6: error: parameter name omitted make[1]: *** [drivers/staging/wlags49_h2/wl_cs.o] Error 1
make: *** [_module_drivers/staging/wlags49_h2] Error 2

This is due a wrong function definition (it does not include parameters names).

Current patch solves the issue.

Thanks a lot

---
 drivers/staging/wlags49_h2/wl_sysfs.h |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

b/drivers/staging/wlags49_h2/wl_sysfs.h
index 6d96d03..fa658c3 100644

@@ -2,6 +2,6 @@
 extern void register_wlags_sysfs(struct net_device *);
 extern void unregister_wlags_sysfs(struct net_device *);
 #else
-static void register_wlags_sysfs(struct net_device *) { return; };
-static void unregister_wlags_sysfs(struct net_device *) { return; };
+static inline void register_wlags_sysfs(struct net_device *net) { }
+static inline void unregister_wlags_sysfs(struct net_device *net) { }
 #endif

--
1.7.0.4

The patch is ok. I'm pretty sure that it will get merged eventually.

To be honest, it wouldn't surprise me if it didn't get merged until 2.6.36. It fixes a build error yes, but it's also a staging thing so there might not be a rush on it.

regards,
dan carpenter

Thank you very much for the feedback Dan.

Best regards,

-----------------------------------------
Javier Martínez Canillas
+595 981 88 66 58

Javier,

I assumed Greg KH already added this last version of your patch to the queue for the staging tree. Anyway I think it's fine. The only problem I see is a possible complained from the compiler because of an argument that's defined but never used. The only way around that is to avoid the call to the function in the first place by using compiler directives in the code (or use a #define but that solution was already rejected). Disabling the call to the function in the code may even save some more overhead and make the memory footprint smaller.

Kind regards,

Henk.

Op 17-6-2010 7:17, Javier Martinez Canillas schreef:

Hank,

Thank you very much for your feedback.

I thought about using #ifdefs in the code too but then read in Documentation/SubmittingPatches:

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'

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.

Thanks a lot.

Best regards,

-----------------------------------------
Javier Martínez Canillas
+595 981 88 66 58
--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Gcc never complains about arguments that aren't used. You often need to do this so that your function matches how a function pointer is defined.

I also tested this. To disable CONFIG_SYSFS do a "make allnoconfig" then enable CONFIG_EMBEDDED, then you can disable CONFIG_SYSFS. It's deliberately a pain in the arse I suppose.

It's not a huge deal to have a #define either, but Joe Perches is right that the current fix really is the better one.

regards,
dan carpenter

Hello Javier,

Op 17-6-2010 19:46, Javier Martinez Canillas schreef:

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

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.