[PATCH v2] Staging: rtl8192e: Fix sparse warning

[PATCH v2] Staging: rtl8192e: Fix sparse warning

[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 <joelpelaez@gmail.com>
---
  drivers/staging/rtl8192e/rtl8192e/rtl_wx.c |    3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/rtl8192e/rtl8192e/rtl_wx.c 
b/drivers/staging/rtl8192e/rtl8192e/rtl_wx.c
index 498995d..f17584d 100644
--- a/drivers/staging/rtl8192e/rtl8192e/rtl_wx.c
+++ b/drivers/staging/rtl8192e/rtl8192e/rtl_wx.c
@@ -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;
-- 
1.7.10.4

Re: [PATCH v2] Staging: rtl8192e: Fix sparse warning

[Joe Perches]

On Sun, 2014-04-06 at 13:43 -0500, Joel Pelaez Jorge wrote:
> 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 <joelpelaez@gmail.com>
> ---
>   drivers/staging/rtl8192e/rtl8192e/rtl_wx.c |    3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/rtl8192e/rtl8192e/rtl_wx.c 
> b/drivers/staging/rtl8192e/rtl8192e/rtl_wx.c
> index 498995d..f17584d 100644
> --- a/drivers/staging/rtl8192e/rtl8192e/rtl_wx.c
> +++ b/drivers/staging/rtl8192e/rtl8192e/rtl_wx.c
> @@ -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;

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.

Re: [PATCH v2] Staging: rtl8192e: Fix sparse warning

[Joel Pelaez Jorge]

El 06/04/14 13:49, Joe Perches escribió:
> On Sun, 2014-04-06 at 13:43 -0500, Joel Pelaez Jorge wrote:
>> 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 <joelpelaez@gmail.com>
>> ---
>>    drivers/staging/rtl8192e/rtl8192e/rtl_wx.c |    3 ++-
>>    1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/staging/rtl8192e/rtl8192e/rtl_wx.c
>> b/drivers/staging/rtl8192e/rtl8192e/rtl_wx.c
>> index 498995d..f17584d 100644
>> --- a/drivers/staging/rtl8192e/rtl8192e/rtl_wx.c
>> +++ b/drivers/staging/rtl8192e/rtl8192e/rtl_wx.c
>> @@ -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;
>
> 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.

Re: [PATCH v2] Staging: rtl8192e: Fix sparse warning

[Greg Kroah-Hartman]

On Sun, Apr 06, 2014 at 01:43:38PM -0500, Joel Pelaez Jorge wrote:
> 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 <joelpelaez@gmail.com>
> ---
>   drivers/staging/rtl8192e/rtl8192e/rtl_wx.c |    3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/rtl8192e/rtl8192e/rtl_wx.c 
> b/drivers/staging/rtl8192e/rtl8192e/rtl_wx.c
> index 498995d..f17584d 100644
> --- a/drivers/staging/rtl8192e/rtl8192e/rtl_wx.c
> +++ b/drivers/staging/rtl8192e/rtl8192e/rtl_wx.c
> @@ -19,6 +19,7 @@
> 
>   #include <linux/string.h>
>   #include "rtl_core.h"
> +#include "rtl_wx.h"

Why is this needed?

> 
>   #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

Re: [PATCH v2] Staging: rtl8192e: Fix sparse warning

[Joe Perches]

On Sun, 2014-04-06 at 14:00 -0500, Joel Pelaez Jorge wrote:
> El 06/04/14 13:49, Joe Perches escribió:
> > On Sun, 2014-04-06 at 13:43 -0500, Joel Pelaez Jorge wrote:
[]
> >> diff --git a/drivers/staging/rtl8192e/rtl8192e/rtl_wx.c
[]
> >> -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.

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

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

That's what I said.

Re: [PATCH v2] Staging: rtl8192e: Fix sparse warning

[Joel Pelaez Jorge]

El 06/04/14 14:01, Greg Kroah-Hartman escribió:
> On Sun, Apr 06, 2014 at 01:43:38PM -0500, Joel Pelaez Jorge wrote:
>> 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 <joelpelaez@gmail.com>
>> ---
>>    drivers/staging/rtl8192e/rtl8192e/rtl_wx.c |    3 ++-
>>    1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/staging/rtl8192e/rtl8192e/rtl_wx.c
>> b/drivers/staging/rtl8192e/rtl8192e/rtl_wx.c
>> index 498995d..f17584d 100644
>> --- a/drivers/staging/rtl8192e/rtl8192e/rtl_wx.c
>> +++ b/drivers/staging/rtl8192e/rtl8192e/rtl_wx.c
>> @@ -19,6 +19,7 @@
>>
>>    #include <linux/string.h>
>>    #include "rtl_core.h"
>> +#include "rtl_wx.h"
>
> Why is this needed?
>
>>
>>    #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
>
 >
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.

Re: [PATCH v2] Staging: rtl8192e: Fix sparse warning

[Greg Kroah-Hartman]

On Sun, Apr 06, 2014 at 02:12:44PM -0500, Joel Pelaez Jorge wrote:
> El 06/04/14 14:01, Greg Kroah-Hartman escribió:
> > On Sun, Apr 06, 2014 at 01:43:38PM -0500, Joel Pelaez Jorge wrote:
> >> 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 <joelpelaez@gmail.com>
> >> ---
> >>    drivers/staging/rtl8192e/rtl8192e/rtl_wx.c |    3 ++-
> >>    1 file changed, 2 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/staging/rtl8192e/rtl8192e/rtl_wx.c
> >> b/drivers/staging/rtl8192e/rtl8192e/rtl_wx.c
> >> index 498995d..f17584d 100644
> >> --- a/drivers/staging/rtl8192e/rtl8192e/rtl_wx.c
> >> +++ b/drivers/staging/rtl8192e/rtl8192e/rtl_wx.c
> >> @@ -19,6 +19,7 @@
> >>
> >>    #include <linux/string.h>
> >>    #include "rtl_core.h"
> >> +#include "rtl_wx.h"
> >
> > Why is this needed?
> >
> >>
> >>    #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
> >
>  >
> 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 :(

> 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

Re: [PATCH v2] Staging: rtl8192e: Fix sparse warning

[Joel Pelaez Jorge]

El 06/04/14 15:26, Greg Kroah-Hartman escribió:
> On Sun, Apr 06, 2014 at 02:12:44PM -0500, Joel Pelaez Jorge wrote:
>> El 06/04/14 14:01, Greg Kroah-Hartman escribió:
>>> On Sun, Apr 06, 2014 at 01:43:38PM -0500, Joel Pelaez Jorge wrote:
>>>> 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 <joelpelaez@gmail.com>
>>>> ---
>>>>     drivers/staging/rtl8192e/rtl8192e/rtl_wx.c |    3 ++-
>>>>     1 file changed, 2 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/staging/rtl8192e/rtl8192e/rtl_wx.c
>>>> b/drivers/staging/rtl8192e/rtl8192e/rtl_wx.c
>>>> index 498995d..f17584d 100644
>>>> --- a/drivers/staging/rtl8192e/rtl8192e/rtl_wx.c
>>>> +++ b/drivers/staging/rtl8192e/rtl8192e/rtl_wx.c
>>>> @@ -19,6 +19,7 @@
>>>>
>>>>     #include <linux/string.h>
>>>>     #include "rtl_core.h"
>>>> +#include "rtl_wx.h"
>>>
>>> Why is this needed?
>>>
>>>>
>>>>     #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
>>>
>>   >
>> 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 :(
> 
>> 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'.

Re: [PATCH v2] Staging: rtl8192e: Fix sparse warning

[Greg Kroah-Hartman]

On Sun, Apr 06, 2014 at 03:43:42PM -0500, Joel Pelaez Jorge wrote:
> El 06/04/14 15:26, Greg Kroah-Hartman escribió:
> > On Sun, Apr 06, 2014 at 02:12:44PM -0500, Joel Pelaez Jorge wrote:
> >> El 06/04/14 14:01, Greg Kroah-Hartman escribió:
> >>> On Sun, Apr 06, 2014 at 01:43:38PM -0500, Joel Pelaez Jorge wrote:
> >>>> 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 <joelpelaez@gmail.com>
> >>>> ---
> >>>>     drivers/staging/rtl8192e/rtl8192e/rtl_wx.c |    3 ++-
> >>>>     1 file changed, 2 insertions(+), 1 deletion(-)
> >>>>
> >>>> diff --git a/drivers/staging/rtl8192e/rtl8192e/rtl_wx.c
> >>>> b/drivers/staging/rtl8192e/rtl8192e/rtl_wx.c
> >>>> index 498995d..f17584d 100644
> >>>> --- a/drivers/staging/rtl8192e/rtl8192e/rtl_wx.c
> >>>> +++ b/drivers/staging/rtl8192e/rtl8192e/rtl_wx.c
> >>>> @@ -19,6 +19,7 @@
> >>>>
> >>>>     #include <linux/string.h>
> >>>>     #include "rtl_core.h"
> >>>> +#include "rtl_wx.h"
> >>>
> >>> Why is this needed?
> >>>
> >>>>
> >>>>     #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
> >>>
> >>   >
> >> 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 :(
> > 
> >> 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.

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

> 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