Staging: rtl8192e: Fix sparse warning

PATCH In review Joel Pelaez Jorge 3 participants 9 messages Apr 6, 2014 Staging

Share & tools

Joel Pelaez Jorge

author

2014-04-06 18:43 UTC #0 lore raw

PATCH v2 1/1 Staging: rtl8192e: Fix sparse warning

Fix a sparse warning, non static symbol is no declared as such. And fix a conflict with static function declared extern to include rtl_wx.h header.

drivers/staging/rtl8192e/rtl8192e/rtl_wx.c:1323:24: warning:
symbol 'r8192_wx_handlers_def' was not declared. Should it be static?

Signed-off-by Joel Pelaez Jorge <[email protected]>

---
 drivers/staging/rtl8192e/rtl8192e/rtl_wx.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

drivers/staging/rtl8192e/rtl8192e/rtl_wx.c +2 -1

index 498995d..f17584d 100644
@@ -19,6 +19,7 @@
 
 #include <linux/string.h>
 #include "rtl_core.h"
+#include "rtl_wx.h"
 
 #define RATE_COUNT 12
 static u32 rtl8192_rates[] = {

@@ -1293,7 +1294,7 @@ static iw_handler r8192_private_handler[] = {
 	(iw_handler)r8192_wx_get_PromiscuousMode,
 };
 
-static struct iw_statistics *r8192_get_wireless_stats(struct net_device *dev)
+struct iw_statistics *r8192_get_wireless_stats(struct net_device *dev)
 {
 	struct r8192_priv *priv = rtllib_priv(dev);
 	struct rtllib_device *ieee = priv->rtllib;

Signature · 1 line

1.7.10.4

Joe Perches

2014-04-06 18:49 UTC #1 lore raw

Quoted 34 lines · Joel Pelaez Jorge

Fix a sparse warning, non static symbol is no declared as such. And fix a conflict with static function declared extern to include rtl_wx.h header.
drivers/staging/rtl8192e/rtl8192e/rtl_wx.c:1323:24: warning:
symbol 'r8192_wx_handlers_def' was not declared. Should it be static?
Signed-off-by Joel Pelaez Jorge <[email protected]>
---
  drivers/staging/rtl8192e/rtl8192e/rtl_wx.c |    3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)
drivers/staging/rtl8192e/rtl8192e/rtl_wx.c +2 -1
index 498995d..f17584d 100644
@@ -19,6 +19,7 @@
 
  #include <linux/string.h>
  #include "rtl_core.h"
+#include "rtl_wx.h"
 
  #define RATE_COUNT 12
  static u32 rtl8192_rates[] = {
@@ -1293,7 +1294,7 @@ static iw_handler r8192_private_handler[] = {
  	(iw_handler)r8192_wx_get_PromiscuousMode,
  };
 
-static struct iw_statistics *r8192_get_wireless_stats(struct net_device 
 *dev)
+struct iw_statistics *r8192_get_wireless_stats(struct net_device *dev)
  {
  	struct r8192_priv *priv = rtllib_priv(dev);

Quoted 1 line

  	struct rtllib_device *ieee = priv->rtllib;

As far as I can tell, there's no reason this should be declared in the .h file and it should stay static and not be exposed at all.

Joel Pelaez Jorge

author

2014-04-06 19:00 UTC #2 lore raw

Quoted 30 lines · Joe Perches

Quoted 29 lines · Joel Pelaez Jorge
Fix a sparse warning, non static symbol is no declared as such. And fix a conflict with static function declared extern to include rtl_wx.h header.
drivers/staging/rtl8192e/rtl8192e/rtl_wx.c:1323:24: warning:
symbol 'r8192_wx_handlers_def' was not declared. Should it be static?
Signed-off-by Joel Pelaez Jorge <[email protected]>
---
   drivers/staging/rtl8192e/rtl8192e/rtl_wx.c |    3 ++-
   1 file changed, 2 insertions(+), 1 deletion(-)
drivers/staging/rtl8192e/rtl8192e/rtl_wx.c +1 -0
index 498995d..f17584d 100644
@@ -19,6 +19,7 @@
 
   #include <linux/string.h>
   #include "rtl_core.h"
+#include "rtl_wx.h"
 
   #define RATE_COUNT 12
   static u32 rtl8192_rates[] = {
@@ -1293,7 +1294,7 @@ static iw_handler r8192_private_handler[] = {
   	(iw_handler)r8192_wx_get_PromiscuousMode,
   };
 

Quoted 12 lines

Quoted 6 lines
drivers/staging/rtl8192e/rtl8192e/rtl_wx.c +1 -1
-static struct iw_statistics *r8192_get_wireless_stats(struct net_device
 *dev)
+struct iw_statistics *r8192_get_wireless_stats(struct net_device *dev)
   {
   	struct r8192_priv *priv = rtllib_priv(dev);
   	struct rtllib_device *ieee = priv->rtllib;
As far as I can tell, there's no reason this should be declared in the .h file and it should stay static and not be exposed at all.

In the source "rtl_core.c" is possible to use the symbol r8192_get_wireless_stats, if WIRELESS_EXT has specifics values.

Although this scenario is unlikely to happen, is better prepare this code for it.

Also, a better way will be delete 'extern' reference to symbol in header file and source file.

Joe Perches

2014-04-06 19:02 UTC #3 lore raw

Quoted 2 lines · Joel Pelaez Jorge

Quoted 1 line · Joe Perches
On Sun, 2014-04-06 at 13:43 -0500, Joel Pelaez Jorge wrote:

[]

Quoted 1 line

Quoted 1 line
Review
Quoted 1 line
(quoted diff) +0 -0

[]

Quoted 10 lines

Quoted 10 lines
Quoted 6 lines
-static struct iw_statistics *r8192_get_wireless_stats(struct net_device *dev)
+struct iw_statistics *r8192_get_wireless_stats(struct net_device *dev)
   {
   	struct r8192_priv *priv = rtllib_priv(dev);
   	struct rtllib_device *ieee = priv->rtllib;
As far as I can tell, there's no reason this should be declared in the .h file and it should stay static and not be exposed at all.

[]

Quoted 5 lines

In the source "rtl_core.c" is possible to use the symbol r8192_get_wireless_stats, if WIRELESS_EXT has specifics values.
Although this scenario is unlikely to happen, is better prepare this code for it.

Huh? This makes no sense to me.
Maybe you can expand a bit here.

Quoted 2 lines

Also, a better way will be delete 'extern' reference to symbol in header file and source file.

That's what I said.

Greg Kroah-Hartman

maintainer

2014-04-06 18:59 UTC #4 lore raw

In reply to Joel Pelaez Jorge

Quoted 18 lines · Joel Pelaez Jorge

Fix a sparse warning, non static symbol is no declared as such. And fix a conflict with static function declared extern to include rtl_wx.h header.
drivers/staging/rtl8192e/rtl8192e/rtl_wx.c:1323:24: warning:
symbol 'r8192_wx_handlers_def' was not declared. Should it be static?
Signed-off-by Joel Pelaez Jorge <[email protected]>
---
  drivers/staging/rtl8192e/rtl8192e/rtl_wx.c |    3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)
drivers/staging/rtl8192e/rtl8192e/rtl_wx.c +0 -0
index 498995d..f17584d 100644

drivers/staging/rtl8192e/rtl8192e/rtl_wx.c line 22

Quoted 4 lines

drivers/staging/rtl8192e/rtl8192e/rtl_wx.c +1 -0

@@ -19,6 +19,7 @@
 
  #include <linux/string.h>
  #include "rtl_core.h"
+#include "rtl_wx.h"

Why is this needed?

drivers/staging/rtl8192e/rtl8192e/rtl_wx.c static iw_handler r8192_private_handler[] = { · line 1298

Quoted 10 lines

drivers/staging/rtl8192e/rtl8192e/rtl_wx.c +1 -1

  #define RATE_COUNT 12
  static u32 rtl8192_rates[] = {

@@ -1293,7 +1294,7 @@ static iw_handler r8192_private_handler[] = {
  	(iw_handler)r8192_wx_get_PromiscuousMode,
  };
 
-static struct iw_statistics *r8192_get_wireless_stats(struct net_device 
 *dev)
+struct iw_statistics *r8192_get_wireless_stats(struct net_device *dev)

Why make this global? This doesn't have anything to do with the warning you are working on as described above.

totally confused,

greg k-h

Joel Pelaez Jorge

author

2014-04-06 19:12 UTC #5 lore raw

Quoted 40 lines · Greg Kroah-Hartman

Quoted 18 lines · Joel Pelaez Jorge
Fix a sparse warning, non static symbol is no declared as such. And fix a conflict with static function declared extern to include rtl_wx.h header.
drivers/staging/rtl8192e/rtl8192e/rtl_wx.c:1323:24: warning:
symbol 'r8192_wx_handlers_def' was not declared. Should it be static?
Signed-off-by Joel Pelaez Jorge <[email protected]>
---
   drivers/staging/rtl8192e/rtl8192e/rtl_wx.c |    3 ++-
   1 file changed, 2 insertions(+), 1 deletion(-)
drivers/staging/rtl8192e/rtl8192e/rtl_wx.c +0 -0
index 498995d..f17584d 100644
drivers/staging/rtl8192e/rtl8192e/rtl_wx.c line 22
Quoted 4 lines
drivers/staging/rtl8192e/rtl8192e/rtl_wx.c +1 -0
@@ -19,6 +19,7 @@
 
   #include <linux/string.h>
   #include "rtl_core.h"
+#include "rtl_wx.h"
Why is this needed?
drivers/staging/rtl8192e/rtl8192e/rtl_wx.c static iw_handler r8192_private_handler[] = { · line 1298
Quoted 10 lines
drivers/staging/rtl8192e/rtl8192e/rtl_wx.c +1 -1
   #define RATE_COUNT 12
   static u32 rtl8192_rates[] = {
@@ -1293,7 +1294,7 @@ static iw_handler r8192_private_handler[] = {
   	(iw_handler)r8192_wx_get_PromiscuousMode,
   };
 
-static struct iw_statistics *r8192_get_wireless_stats(struct net_device
 *dev)
+struct iw_statistics *r8192_get_wireless_stats(struct net_device *dev)
Why make this global? This doesn't have anything to do with the warning you are working on as described above.

Quoted 5 lines

totally confused,
greg k-h

In the "rtl_wx.h" header declare extern 'r8192_wx_handlers_def' but r8192_get_wireless_stats too. Because r8192_get_wireless_stats is used by rtl_core.h if WIRELESS_EXT is more that 12 but less that 17.

Greg Kroah-Hartman

maintainer

2014-04-06 20:24 UTC #6 lore raw

Quoted 41 lines · Joel Pelaez Jorge

Quoted 40 lines · Greg Kroah-Hartman
Quoted 18 lines · Joel Pelaez Jorge
Fix a sparse warning, non static symbol is no declared as such. And fix a conflict with static function declared extern to include rtl_wx.h header.
drivers/staging/rtl8192e/rtl8192e/rtl_wx.c:1323:24: warning:
symbol 'r8192_wx_handlers_def' was not declared. Should it be static?
Signed-off-by Joel Pelaez Jorge <[email protected]>
---
   drivers/staging/rtl8192e/rtl8192e/rtl_wx.c |    3 ++-
   1 file changed, 2 insertions(+), 1 deletion(-)
drivers/staging/rtl8192e/rtl8192e/rtl_wx.c +0 -0
index 498995d..f17584d 100644
drivers/staging/rtl8192e/rtl8192e/rtl_wx.c line 22
Quoted 4 lines
drivers/staging/rtl8192e/rtl8192e/rtl_wx.c +1 -0
@@ -19,6 +19,7 @@
 
   #include <linux/string.h>
   #include "rtl_core.h"
+#include "rtl_wx.h"
Why is this needed?
drivers/staging/rtl8192e/rtl8192e/rtl_wx.c static iw_handler r8192_private_handler[] = { · line 1298
Quoted 10 lines
drivers/staging/rtl8192e/rtl8192e/rtl_wx.c +1 -1
   #define RATE_COUNT 12
   static u32 rtl8192_rates[] = {
@@ -1293,7 +1294,7 @@ static iw_handler r8192_private_handler[] = {
   	(iw_handler)r8192_wx_get_PromiscuousMode,
   };
 
-static struct iw_statistics *r8192_get_wireless_stats(struct net_device
 *dev)
+struct iw_statistics *r8192_get_wireless_stats(struct net_device *dev)
Why make this global? This doesn't have anything to do with the warning you are working on as described above.

Quoted 7 lines

Quoted 5 lines
totally confused,
greg k-h
In the "rtl_wx.h" header declare extern 'r8192_wx_handlers_def' but r8192_get_wireless_stats too.

I don't understand what this sentance means :(

Quoted 2 lines

Because r8192_get_wireless_stats is used by rtl_core.h if WIRELESS_EXT is more that 12 but less that 17.

Will that value ever change?

thanks,

greg k-h

Joel Pelaez Jorge

author

2014-04-06 20:43 UTC #7 lore raw

Quoted 52 lines · Greg Kroah-Hartman

Quoted 41 lines · Joel Pelaez Jorge
Quoted 40 lines · Greg Kroah-Hartman
Quoted 18 lines · Joel Pelaez Jorge
Fix a sparse warning, non static symbol is no declared as such. And fix a conflict with static function declared extern to include rtl_wx.h header.
drivers/staging/rtl8192e/rtl8192e/rtl_wx.c:1323:24: warning:
symbol 'r8192_wx_handlers_def' was not declared. Should it be static?
Signed-off-by Joel Pelaez Jorge <[email protected]>
---
    drivers/staging/rtl8192e/rtl8192e/rtl_wx.c |    3 ++-
    1 file changed, 2 insertions(+), 1 deletion(-)
drivers/staging/rtl8192e/rtl8192e/rtl_wx.c +0 -0
index 498995d..f17584d 100644
drivers/staging/rtl8192e/rtl8192e/rtl_wx.c line 22
Quoted 4 lines
drivers/staging/rtl8192e/rtl8192e/rtl_wx.c +1 -0
@@ -19,6 +19,7 @@
 
    #include <linux/string.h>
    #include "rtl_core.h"
+#include "rtl_wx.h"
Why is this needed?
drivers/staging/rtl8192e/rtl8192e/rtl_wx.c static iw_handler r8192_private_handler[] = { · line 1298
Quoted 10 lines
drivers/staging/rtl8192e/rtl8192e/rtl_wx.c +1 -1
    #define RATE_COUNT 12
    static u32 rtl8192_rates[] = {
@@ -1293,7 +1294,7 @@ static iw_handler r8192_private_handler[] = {
    	(iw_handler)r8192_wx_get_PromiscuousMode,
    };
 
-static struct iw_statistics *r8192_get_wireless_stats(struct net_device
 *dev)
+struct iw_statistics *r8192_get_wireless_stats(struct net_device *dev)
Why make this global? This doesn't have anything to do with the warning you are working on as described above.
Quoted 7 lines
Quoted 5 lines
totally confused,
greg k-h
In the "rtl_wx.h" header declare extern 'r8192_wx_handlers_def' but r8192_get_wireless_stats too.
I don't understand what this sentance means :(

Quoted 9 lines

Quoted 2 lines
Because r8192_get_wireless_stats is used by rtl_core.h if WIRELESS_EXT is more that 12 but less that 17.
Will that value ever change?
thanks,
greg k-h

The file: 'rtl_wx.h' is a header that defines some symbols uses externally by 'rtl_core.c', it declare:

extern struct iw_handler_def r8192_wx_handlers_def;
struct iw_statistics *r8192_get_wireless_stats(struct net_device *dev);

If this header is included in rtl_wx.c, fix the sparse warning, but cause conflict with the function declaration, because in 'rtl_wx.c' r8192_get_wireless_stats is defined as 'static'.

In the header is defined as 'extern' because is called by rtl_core.c if the WIRELESS_EXT certain values.

By the way, WIRELESS_EXT is a define constant that indicate the wireless extension version supported by the kernel it only up. The better solution is remove the code that called the function, remove the declaration and only keep 'r8192_wx_handlers_def'.

Greg Kroah-Hartman

maintainer

2014-04-06 20:56 UTC #8 lore raw

Quoted 68 lines · Joel Pelaez Jorge

Quoted 52 lines · Greg Kroah-Hartman
Quoted 41 lines · Joel Pelaez Jorge
Quoted 40 lines · Greg Kroah-Hartman
Quoted 18 lines · Joel Pelaez Jorge
Fix a sparse warning, non static symbol is no declared as such. And fix a conflict with static function declared extern to include rtl_wx.h header.
drivers/staging/rtl8192e/rtl8192e/rtl_wx.c:1323:24: warning:
symbol 'r8192_wx_handlers_def' was not declared. Should it be static?
Signed-off-by Joel Pelaez Jorge <[email protected]>
---
    drivers/staging/rtl8192e/rtl8192e/rtl_wx.c |    3 ++-
    1 file changed, 2 insertions(+), 1 deletion(-)
drivers/staging/rtl8192e/rtl8192e/rtl_wx.c +0 -0
index 498995d..f17584d 100644
drivers/staging/rtl8192e/rtl8192e/rtl_wx.c line 22
Quoted 4 lines
drivers/staging/rtl8192e/rtl8192e/rtl_wx.c +1 -0
@@ -19,6 +19,7 @@
 
    #include <linux/string.h>
    #include "rtl_core.h"
+#include "rtl_wx.h"
Why is this needed?
drivers/staging/rtl8192e/rtl8192e/rtl_wx.c static iw_handler r8192_private_handler[] = { · line 1298
Quoted 10 lines
drivers/staging/rtl8192e/rtl8192e/rtl_wx.c +1 -1
    #define RATE_COUNT 12
    static u32 rtl8192_rates[] = {
@@ -1293,7 +1294,7 @@ static iw_handler r8192_private_handler[] = {
    	(iw_handler)r8192_wx_get_PromiscuousMode,
    };
 
-static struct iw_statistics *r8192_get_wireless_stats(struct net_device
 *dev)
+struct iw_statistics *r8192_get_wireless_stats(struct net_device *dev)
Why make this global? This doesn't have anything to do with the warning you are working on as described above.
Quoted 7 lines
Quoted 5 lines
totally confused,
greg k-h
In the "rtl_wx.h" header declare extern 'r8192_wx_handlers_def' but r8192_get_wireless_stats too.
I don't understand what this sentance means :(
Quoted 9 lines
Quoted 2 lines
Because r8192_get_wireless_stats is used by rtl_core.h if WIRELESS_EXT is more that 12 but less that 17.
Will that value ever change?
thanks,
greg k-h
The file: 'rtl_wx.h' is a header that defines some symbols uses externally by 'rtl_core.c', it declare:
extern struct iw_handler_def r8192_wx_handlers_def;
struct iw_statistics *r8192_get_wireless_stats(struct net_device *dev);

Quoted 6 lines

If this header is included in rtl_wx.c, fix the sparse warning, but cause conflict with the function declaration, because in 'rtl_wx.c' r8192_get_wireless_stats is defined as 'static'.
In the header is defined as 'extern' because is called by rtl_core.c if the WIRELESS_EXT certain values.

Ok, that makes more sense. Can you put all of that in the changelog entry and resend the patch?

Quoted 4 lines

By the way, WIRELESS_EXT is a define constant that indicate the wireless extension version supported by the kernel it only up. The better solution is remove the code that called the function, remove the declaration and only keep 'r8192_wx_handlers_def'.

That would be best...

thanks,

greg k-h