diff mbox

[OPW,kernel] Staging:wlan-ng: Fix sparse warning should it be static?

Message ID 20140313024411.GA10013@gmail.com
State New, archived
Headers show

Commit Message

Tugce Sirin March 13, 2014, 2:44 a.m. UTC
Fix sparse warning should it be static in staging/wlan-ng driver.

Signed-off-by: Tugce Sirin <ztugcesirin@gmail.com>
---
 drivers/staging/wlan-ng/cfg80211.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Nick Kossifidis March 13, 2014, 1:06 p.m. UTC | #1
2014-03-13 2:44 GMT+00:00 Tugce Sirin <ztugcesirin@gmail.com>:
> Fix sparse warning should it be static in staging/wlan-ng driver.
>
> Signed-off-by: Tugce Sirin <ztugcesirin@gmail.com>

Hello Tugce :-)

Your subject/title "Fix sparse warning should it be static?" indicates
that you ask a question on the list, it could be something like "fix
sparse non-static symbol warning" to make more sense.
Tugce Sirin March 13, 2014, 1:12 p.m. UTC | #2
Hello Nick,

I actually copy/pasted the warning but you're right. It seems like a
question. Even though I have questions about this particular sparse warning
this is not it and I'll be more careful in next patches. I can also resend
it with correct subject if you think that I should.


2014-03-13 15:06 GMT+02:00 Nick Kossifidis <mickflemm@gmail.com>:

> 2014-03-13 2:44 GMT+00:00 Tugce Sirin <ztugcesirin@gmail.com>:
> > Fix sparse warning should it be static in staging/wlan-ng driver.
> >
> > Signed-off-by: Tugce Sirin <ztugcesirin@gmail.com>
>
> Hello Tugce :-)
>
> Your subject/title "Fix sparse warning should it be static?" indicates
> that you ask a question on the list, it could be something like "fix
> sparse non-static symbol warning" to make more sense.
>
>
> --
> GPG ID: 0xEE878588
> As you read this post global entropy rises. Have Fun ;-)
> Nick
>
Nick Kossifidis March 13, 2014, 1:58 p.m. UTC | #3
2014-03-13 13:12 GMT+00:00 Tu?├že ?irin <ztugcesirin@gmail.com>:
> Hello Nick,
>
> I actually copy/pasted the warning but you're right. It seems like a
> question. Even though I have questions about this particular sparse warning
> this is not it and I'll be more careful in next patches. I can also resend
> it with correct subject if you think that I should.
>

About the non-static symbol warning, here is what sparse(1) man page says:

"Private symbols (functions and variables) internal to a given source
file should use static, to allow additional compiler optimizations,
allow detection of unused symbols, and prevent other code from relying
on these internal symbols. Public symbols used by other source files
will need declarations visible to those other source files, such as in
a header file. All declarations should fall into one of these two
categories. Thus, with -Wdecl, Sparse warns about any symbol
definition with neither static nor a declaration. To fix this warning,
declare private symbols static, and ensure that the files defining
public symbols have the symbol declarations available first (such as
by including the appropriate header file). "

I didn't check the patch content on my previous mail because the
question mark caught my eye first, so let's check this out...

Is wlan_create_wiphy local to cfg80211.c ?

makhs linux # grep -R wlan_create_wiphy
drivers/staging/wlan-ng/cfg80211.c:struct wiphy
*wlan_create_wiphy(struct device *dev, wlandevice_t *wlandev)
drivers/staging/wlan-ng/p80211netdev.c:    wiphy =
wlan_create_wiphy(physdev, wlandev);

At first it seems it's not, but let's check p80211netdev.c...

91 #include "cfg80211.c"

Ouch ! So cfg80211.c is actually part of  p80211netdev.c, let's check
the Makefile of wlan-ng

  1 obj-$(CONFIG_PRISM2_USB) += prism2_usb.o
  2
  3 prism2_usb-y := prism2usb.o \
  4                 p80211conv.o \
  5                 p80211req.o \
  6                 p80211wep.o \
  7                 p80211netdev.o

So cfg80211.c is only compiled as part of p80211netdev.o, to the
compiler this is a single .c source file so wlan_create_wiphy is local
to that file. That's why sparse gives you that warning, it says that
you have a symbol called wlan_create_wiphy that's local and is not
declared as static.

If that symbol was not local and was needed by other source files, it
should have a declaration. More on that here ->
https://www.linux.com/learn/linux-training/31161-the-kernel-newbie-corner-kernel-symbols-whats-available-to-your-module-what-isnt


As for the subject, please re-send, it'll look better on the commit logs ;-)
Bob Copeland March 13, 2014, 2:08 p.m. UTC | #4
On Thu, Mar 13, 2014 at 01:58:18PM +0000, Nick Kossifidis wrote:
> At first it seems it's not, but let's check p80211netdev.c...
> 
> 91 #include "cfg80211.c"

Ugh, #including C files, that is almost always a trip to crazy town.
diff mbox

Patch

diff --git a/drivers/staging/wlan-ng/cfg80211.c b/drivers/staging/wlan-ng/cfg80211.c
index 6c31730..f76f95c 100644
--- a/drivers/staging/wlan-ng/cfg80211.c
+++ b/drivers/staging/wlan-ng/cfg80211.c
@@ -749,7 +749,7 @@  static const struct cfg80211_ops prism2_usb_cfg_ops = {
 
 
 /* Functions to create/free wiphy interface */
-struct wiphy *wlan_create_wiphy(struct device *dev, wlandevice_t *wlandev)
+static struct wiphy *wlan_create_wiphy(struct device *dev, wlandevice_t *wlandev)
 {
 	struct wiphy *wiphy;
 	struct prism2_wiphy_private *priv;
@@ -786,7 +786,7 @@  struct wiphy *wlan_create_wiphy(struct device *dev, wlandevice_t *wlandev)
 }
 
 
-void wlan_free_wiphy(struct wiphy *wiphy)
+static void wlan_free_wiphy(struct wiphy *wiphy)
 {
 	wiphy_unregister(wiphy);
 	wiphy_free(wiphy);