Re: CHECKPATCH: missing a warning soon after include files decl -c

2021-03-24 Thread Fabio Aiuto 
On Sat, Mar 20, 2021 at 04:28:51AM -0700, Joe Perches wrote:
> 
> Actually, these would seem to be better as one or multiple functions with
> local statics or even as static inlines functions in the .h file
> 
> $ git grep -w RTW_WPA_OUI drivers/staging/rtl8723bs/core
> drivers/staging/rtl8723bs/core/rtw_ap.c:extern unsigned char RTW_WPA_OUI[];
> drivers/staging/rtl8723bs/core/rtw_ap.c:if (!memcmp(RTW_WPA_OUI, oui, 
> 4))
> drivers/staging/rtl8723bs/core/rtw_mlme_ext.c:unsigned char RTW_WPA_OUI[] = 
> {0x00, 0x50, 0xf2, 0x01};
> drivers/staging/rtl8723bs/core/rtw_mlme_ext.c:  if 
> ((!memcmp(pIE->data, RTW_WPA_OUI, 4)) ||
> drivers/staging/rtl8723bs/core/rtw_wlan_util.c:extern unsigned char 
> RTW_WPA_OUI[];
> drivers/staging/rtl8723bs/core/rtw_wlan_util.c: if 
> ((!memcmp(pIE->data, RTW_WPA_OUI, 4)) && (!memcmp((pIE->data + 12), 
> WPA_TKIP_CIPHER, 4)))
> 
> $ git grep -w WMM_OUI drivers/staging/rtl8723bs/core
> drivers/staging/rtl8723bs/core/rtw_ap.c:extern unsigned char WMM_OUI[];
> drivers/staging/rtl8723bs/core/rtw_ap.c:else if (!memcmp(WMM_OUI, 
> oui, 4))
> drivers/staging/rtl8723bs/core/rtw_mlme_ext.c:unsigned char WMM_OUI[] = 
> {0x00, 0x50, 0xf2, 0x02};
> drivers/staging/rtl8723bs/core/rtw_mlme_ext.c:
>   (!memcmp(pIE->data, WMM_OUI, 4)) ||
> drivers/staging/rtl8723bs/core/rtw_mlme_ext.c:  if 
> (!memcmp(pIE->data, WMM_OUI, 4))
> drivers/staging/rtl8723bs/include/rtw_mlme_ext.h:extern unsigned char 
> WMM_OUI[];
> 
> $ git grep -w WPS_OUI drivers/staging/rtl8723bs/core
> drivers/staging/rtl8723bs/core/rtw_ap.c:extern unsigned char WPS_OUI[];
> drivers/staging/rtl8723bs/core/rtw_ap.c:else if (!memcmp(WPS_OUI, 
> oui, 4))
> drivers/staging/rtl8723bs/core/rtw_mlme_ext.c:unsigned char WPS_OUI[] = 
> {0x00, 0x50, 0xf2, 0x04};
> drivers/staging/rtl8723bs/core/rtw_mlme_ext.c:
>   (!memcmp(pIE->data, WPS_OUI, 4))) {
> drivers/staging/rtl8723bs/core/rtw_mlme_ext.c:  if 
> ((!padapter->registrypriv.wifi_spec) && (!memcmp(pIE->data, WPS_OUI, 4))) {
> 
> $ git grep -w P2P_OUI drivers/staging/rtl8723bs/core
> drivers/staging/rtl8723bs/core/rtw_ap.c:extern unsigned char P2P_OUI[];
> drivers/staging/rtl8723bs/core/rtw_ap.c:else if (!memcmp(P2P_OUI, 
> oui, 4))
> drivers/staging/rtl8723bs/core/rtw_mlme_ext.c:unsigned char P2P_OUI[] = 
> {0x50, 0x6F, 0x9A, 0x09};
> drivers/staging/rtl8723bs/core/rtw_mlme_ext.c:  if (!memcmp(frame_body + 2, 
> P2P_OUI, 4)) {
> 
> So maybe something like the below (written in email client, maybe typos)
> 
> enum oui_type {
>   RTW_WPA,
>   WMM,
>   WPS,
>   P2P
> };
> 
> bool is_oui_type(const u8 *mem, enum oui_type type)
> {
>   static const u8 rtw_wpa[] = {0x00, 0x50, 0xf2, 0x01};
>   static const u8 wmm[] = {0x00, 0x50, 0xf2, 0x02};
>   static const u8 wps[] = {0x00, 0x50, 0xf2, 0x04};
>   static const u8 p2p[] = {0x50, 0x6F, 0x9A, 0x09};
> 
>   const u8 *oui;
> 
>   if (type == RTW_WPA)
>   oui = rtw_wpa;
>   else if (type == WMM)
>   oui = wmm;
>   else if (type == WPS)
>   oui = wps;
>   else if (type == P2P)
>   oui = p2p;
>   else
>   return false;
> 
>   return !memcmp(mem, oui, 4);
> }
> 
> so for instance the P2P uses would become
> 
>   else if (is_oui_type(oui, P2P))
> and
>   if (is_oui_type(frame_body + 2, P2P)) {
> 
> though I think 4 byte OUIs are just odd.
> 
> https://en.wikipedia.org/wiki/Organizationally_unique_identifier
> 
> An organizationally unique identifier (OUI) is a 24-bit number that uniquely 
> identifies a vendor, manufacturer, or other organization.
> 
> 
> 

Hi,

is that good? May I do the same for others ouis? One small inline for each oui 
type instead
of a switch...

diff --git a/drivers/staging/rtl8723bs/core/rtw_ap.c 
b/drivers/staging/rtl8723bs/core/rtw_ap.c
index 3cd9c61eec99..7d31f359cf37 100644
--- a/drivers/staging/rtl8723bs/core/rtw_ap.c
+++ b/drivers/staging/rtl8723bs/core/rtw_ap.c
@@ -1664,7 +1664,7 @@ static void update_bcn_p2p_ie(struct adapter *padapter)
 
 static void update_bcn_vendor_spec_ie(struct adapter *padapter, u8 *oui)
 {
-   if (!memcmp(RTW_WPA_OUI, oui, 4))
+   if (is_rtw_wpa_oui(oui))
update_bcn_wpa_ie(padapter);
 
else if (!memcmp(WMM_OUI, oui, 4))
diff --git a/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c 
b/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c
index 8aadcf72a7ba..e05f70e434a2 100644
--- a/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c
+++ b/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c
@@ -57,7 +57,6 @@ static u8 null_addr[ETH_ALEN] = {0, 0, 0, 0, 0, 0};
 /**
 OUI definitions for the vendor specific IE
 ***/
-unsigned char RTW_WPA_OUI[] = {0x00, 0x50, 0xf2, 0x01};
 unsigned char

Re: CHECKPATCH: missing a warning soon after include files decl -c

2021-03-21 Thread Greg KH 
On Sat, Mar 20, 2021 at 03:49:12PM +0100, Fabio Aiuto wrote:
> On Sat, Mar 20, 2021 at 11:59:44AM +0100, Greg KH wrote:
> > On Sat, Mar 20, 2021 at 11:54:24AM +0100, Fabio Aiuto wrote:
> > > Hi,
> > > 
> > > here's an issue in checkpatch.pl
> > > 
> > > $ perl script/checkpatch.pl -f drivers/staging/rtl8723bs/core/rtw_ap.c
> > > 
> > > I get three warning related to an extern declaration
> > > 
> > > WARNING: externs should be avoided in .c files
> > > #14: FILE: drivers/staging/rtl8723bs/core/rtw_ap.c:14:
> > > +extern unsigned char WMM_OUI[];
> > > --
> > > WARNING: externs should be avoided in .c files
> > > #15: FILE: drivers/staging/rtl8723bs/core/rtw_ap.c:15:
> > > +extern unsigned char WPS_OUI[];
> > > --
> > > WARNING: externs should be avoided in .c files
> > > #16: FILE: drivers/staging/rtl8723bs/core/rtw_ap.c:16:
> > > +extern unsigned char P2P_OUI[];
> > > --
> > > 
> > > but the file checked has 4 extern declaration:
> > > -
> > > #define _RTW_AP_C_
> > > 
> > > #include 
> > > #include 
> > > #include 
> > > 
> > > extern unsigned char RTW_WPA_OUI[];
> > > extern unsigned char WMM_OUI[];
> > > extern unsigned char WPS_OUI[];
> > > extern unsigned char P2P_OUI[];
> > > 
> > > void init_mlme_ap_info(struct adapter *padapter)
> > > ---
> > > 
> > > If I add a ';' this way:
> > > 
> > > #define _RTW_AP_C_
> > > 
> > > #include 
> > > #include 
> > > #include 
> > > ;
> > > extern unsigned char RTW_WPA_OUI[];
> > > extern unsigned char WMM_OUI[];
> > > extern unsigned char WPS_OUI[];
> > > extern unsigned char P2P_OUI[];
> > > 
> > > void init_mlme_ap_info(struct adapter *padapter)
> > > 
> > 
> > Wait, why would you do the above?
> > 
> > Don't try to trick a perl script that has a hard time parsing C files,
> > try to resolve the original issue here.
> > 
> > And that is that the above definitions should be in a .h file somewhere.
> > If you make that move then all should be fine.
> > 
> > thanks,
> > 
> > greg k-h
> 
> that's another issue
> 
> WARNING: externs should be avoided in .c files
> #35: FILE: drivers/staging/rtl8723bs/core/rtw_efuse.c:35:
> +bool
> 
> CHECK: Lines should not end with a '('
> #36: FILE: drivers/staging/rtl8723bs/core/rtw_efuse.c:36:
> ---
> 
> but that's what I see in line 35
> 
> 
> #define REG_EFUSE_CTRL0x0030
> #define EFUSE_CTRLREG_EFUSE_CTRL  /*  E-Fuse 
> Control. */
> 
> bool<- line 35
> Efuse_Read1ByteFromFakeContent(
>   struct adapter *padapter,
>   u16 Offset,
>   u8 *Value);
> bool
> Efuse_Read1ByteFromFakeContent(
>   struct adapter *padapter,


That's some horrid code formatting, just clean it up to look normal and
all will be fine.

> another one...
> 
> WARNING: externs should be avoided in .c files
> #40: FILE: drivers/staging/rtl8723bs/core/rtw_ioctl_set.c:40:
> +u8 rtw_do_join(struct adapter *padapter); < do I miss 
>   something about extern keyword?

Having a global function prototype in a .c file is not a good idea.
Again, fix it up and all will be fine.

thanks,

greg k-h

Re: CHECKPATCH: missing a warning soon after include files decl -c

2021-03-20 Thread Fabio Aiuto 
On Sat, Mar 20, 2021 at 11:59:44AM +0100, Greg KH wrote:
> On Sat, Mar 20, 2021 at 11:54:24AM +0100, Fabio Aiuto wrote:
> > Hi,
> > 
> > here's an issue in checkpatch.pl
> > 
> > $ perl script/checkpatch.pl -f drivers/staging/rtl8723bs/core/rtw_ap.c
> > 
> > I get three warning related to an extern declaration
> > 
> > WARNING: externs should be avoided in .c files
> > #14: FILE: drivers/staging/rtl8723bs/core/rtw_ap.c:14:
> > +extern unsigned char WMM_OUI[];
> > --
> > WARNING: externs should be avoided in .c files
> > #15: FILE: drivers/staging/rtl8723bs/core/rtw_ap.c:15:
> > +extern unsigned char WPS_OUI[];
> > --
> > WARNING: externs should be avoided in .c files
> > #16: FILE: drivers/staging/rtl8723bs/core/rtw_ap.c:16:
> > +extern unsigned char P2P_OUI[];
> > --
> > 
> > but the file checked has 4 extern declaration:
> > -
> > #define _RTW_AP_C_
> > 
> > #include 
> > #include 
> > #include 
> > 
> > extern unsigned char RTW_WPA_OUI[];
> > extern unsigned char WMM_OUI[];
> > extern unsigned char WPS_OUI[];
> > extern unsigned char P2P_OUI[];
> > 
> > void init_mlme_ap_info(struct adapter *padapter)
> > ---
> > 
> > If I add a ';' this way:
> > 
> > #define _RTW_AP_C_
> > 
> > #include 
> > #include 
> > #include 
> > ;
> > extern unsigned char RTW_WPA_OUI[];
> > extern unsigned char WMM_OUI[];
> > extern unsigned char WPS_OUI[];
> > extern unsigned char P2P_OUI[];
> > 
> > void init_mlme_ap_info(struct adapter *padapter)
> > 
> 
> Wait, why would you do the above?
> 
> Don't try to trick a perl script that has a hard time parsing C files,
> try to resolve the original issue here.
> 
> And that is that the above definitions should be in a .h file somewhere.
> If you make that move then all should be fine.
> 
> thanks,
> 
> greg k-h

that's another issue

WARNING: externs should be avoided in .c files
#35: FILE: drivers/staging/rtl8723bs/core/rtw_efuse.c:35:
+bool

CHECK: Lines should not end with a '('
#36: FILE: drivers/staging/rtl8723bs/core/rtw_efuse.c:36:
---

but that's what I see in line 35


#define REG_EFUSE_CTRL  0x0030
#define EFUSE_CTRL  REG_EFUSE_CTRL  /*  E-Fuse 
Control. */

bool<- line 35
Efuse_Read1ByteFromFakeContent(
struct adapter *padapter,
u16 Offset,
u8 *Value);
bool
Efuse_Read1ByteFromFakeContent(
struct adapter *padapter,

another one...

WARNING: externs should be avoided in .c files
#40: FILE: drivers/staging/rtl8723bs/core/rtw_ioctl_set.c:40:
+u8 rtw_do_join(struct adapter *padapter); < do I miss 
something about extern keyword?

CHECK: Unnecessary parentheses around padapter->mlmepriv
#45: FILE: drivers/staging/rtl8723bs/core/rtw_ioctl_set.c:45:

thank you,

fabio

Re: CHECKPATCH: missing a warning soon after include files decl -c

2021-03-20 Thread Fabio Aiuto 
On Sat, Mar 20, 2021 at 04:28:51AM -0700, Joe Perches wrote:
> On Sat, 2021-03-20 at 11:59 +0100, Greg KH wrote:
> > On Sat, Mar 20, 2021 at 11:54:24AM +0100, Fabio Aiuto wrote:
> > > Hi,
> > > 
> > > here's an issue in checkpatch.pl
> > > 
> > > $ perl script/checkpatch.pl -f drivers/staging/rtl8723bs/core/rtw_ap.c
> > > 
> > > I get three warning related to an extern declaration
> > > 
> > > WARNING: externs should be avoided in .c files
> > > #14: FILE: drivers/staging/rtl8723bs/core/rtw_ap.c:14:
> > > +extern unsigned char WMM_OUI[];
> > > --
> > > WARNING: externs should be avoided in .c files
> > > #15: FILE: drivers/staging/rtl8723bs/core/rtw_ap.c:15:
> > > +extern unsigned char WPS_OUI[];
> > > --
> > > WARNING: externs should be avoided in .c files
> > > #16: FILE: drivers/staging/rtl8723bs/core/rtw_ap.c:16:
> > > +extern unsigned char P2P_OUI[];
> > > --
> > > 
> > > but the file checked has 4 extern declaration:
> > > -
> > > #define _RTW_AP_C_
> > > 
> > > #include 
> > > #include 
> > > #include 
> > > 
> > > extern unsigned char RTW_WPA_OUI[];
> > > extern unsigned char WMM_OUI[];
> > > extern unsigned char WPS_OUI[];
> > > extern unsigned char P2P_OUI[];
> > > 
> > > void init_mlme_ap_info(struct adapter *padapter)
> > > ---
> > > 
> > > If I add a ';' this way:
> > > 
> > > #define _RTW_AP_C_
> > > 
> > > #include 
> > > #include 
> > > #include 
> > > ;
> > > extern unsigned char RTW_WPA_OUI[];
> > > extern unsigned char WMM_OUI[];
> > > extern unsigned char WPS_OUI[];
> > > extern unsigned char P2P_OUI[];
> > > 
> > > void init_mlme_ap_info(struct adapter *padapter)
> > > 
> > 
> > Wait, why would you do the above?
> 
> It is rather an ugly hack.

yes it is, it was just a local and temporary one to verify that checkpatch.pl
recognizes a statement even in multiple lines, until the next ';'. In my case 
it has his own
drawbacks, but I will follow Greg's advice, to let checkpatch.pl doing his 
heavy duty:) 

> 
> > Don't try to trick a perl script that has a hard time parsing C files,
> > try to resolve the original issue here.
> > 
> > And that is that the above definitions should be in a .h file somewhere.
> > If you make that move then all should be fine.
> 
> Actually, these would seem to be better as one or multiple functions with
> local statics or even as static inlines functions in the .h file
> 
> $ git grep -w RTW_WPA_OUI drivers/staging/rtl8723bs/core
> drivers/staging/rtl8723bs/core/rtw_ap.c:extern unsigned char RTW_WPA_OUI[];
> drivers/staging/rtl8723bs/core/rtw_ap.c:if (!memcmp(RTW_WPA_OUI, oui, 
> 4))
> drivers/staging/rtl8723bs/core/rtw_mlme_ext.c:unsigned char RTW_WPA_OUI[] = 
> {0x00, 0x50, 0xf2, 0x01};
> drivers/staging/rtl8723bs/core/rtw_mlme_ext.c:  if 
> ((!memcmp(pIE->data, RTW_WPA_OUI, 4)) ||
> drivers/staging/rtl8723bs/core/rtw_wlan_util.c:extern unsigned char 
> RTW_WPA_OUI[];
> drivers/staging/rtl8723bs/core/rtw_wlan_util.c: if 
> ((!memcmp(pIE->data, RTW_WPA_OUI, 4)) && (!memcmp((pIE->data + 12), 
> WPA_TKIP_CIPHER, 4)))
> 
> $ git grep -w WMM_OUI drivers/staging/rtl8723bs/core
> drivers/staging/rtl8723bs/core/rtw_ap.c:extern unsigned char WMM_OUI[];
> drivers/staging/rtl8723bs/core/rtw_ap.c:else if (!memcmp(WMM_OUI, 
> oui, 4))
> drivers/staging/rtl8723bs/core/rtw_mlme_ext.c:unsigned char WMM_OUI[] = 
> {0x00, 0x50, 0xf2, 0x02};
> drivers/staging/rtl8723bs/core/rtw_mlme_ext.c:
>   (!memcmp(pIE->data, WMM_OUI, 4)) ||
> drivers/staging/rtl8723bs/core/rtw_mlme_ext.c:  if 
> (!memcmp(pIE->data, WMM_OUI, 4))
> drivers/staging/rtl8723bs/include/rtw_mlme_ext.h:extern unsigned char 
> WMM_OUI[];
> 
> $ git grep -w WPS_OUI drivers/staging/rtl8723bs/core
> drivers/staging/rtl8723bs/core/rtw_ap.c:extern unsigned char WPS_OUI[];
> drivers/staging/rtl8723bs/core/rtw_ap.c:else if (!memcmp(WPS_OUI, 
> oui, 4))
> drivers/staging/rtl8723bs/core/rtw_mlme_ext.c:unsigned char WPS_OUI[] = 
> {0x00, 0x50, 0xf2, 0x04};
> drivers/staging/rtl8723bs/core/rtw_mlme_ext.c:
>   (!memcmp(pIE->data, WPS_OUI, 4))) {
> drivers/staging/rtl8723bs/core/rtw_mlme_ext.c:  if 
> ((!padapter->registrypriv.wifi_spec) && (!memcmp(pIE->data, WPS_OUI, 4))) {
> 
> $ git grep -w P2P_OUI drivers/staging/rtl8723bs/core
> drivers/staging/rtl8723bs/core/rtw_ap.c:extern unsigned char P2P_OUI[];
> drivers/staging/rtl8723bs/core/rtw_ap.c:else if (!memcmp(P2P_OUI, 
> oui, 4))
> drivers/staging/rtl8723bs/core/rtw_mlme_ext.c:unsigned char P2P_OUI[] = 
> {0x50, 0x6F, 0x9A, 0x09};
> drivers/staging/rtl8723bs/core/rtw_mlme_ext.c:  if (!memcmp(frame_body + 2, 
> P2P_OUI, 4)) {
> 
> So maybe something like the below (written in email client, maybe typos)
> 
> enum oui_type {
>   RTW_WPA,
>   WMM,
>   WPS,

Re: CHECKPATCH: missing a warning soon after include files decl -c

2021-03-20 Thread Joe Perches 
On Sat, 2021-03-20 at 11:59 +0100, Greg KH wrote:
> On Sat, Mar 20, 2021 at 11:54:24AM +0100, Fabio Aiuto wrote:
> > Hi,
> > 
> > here's an issue in checkpatch.pl
> > 
> > $ perl script/checkpatch.pl -f drivers/staging/rtl8723bs/core/rtw_ap.c
> > 
> > I get three warning related to an extern declaration
> > 
> > WARNING: externs should be avoided in .c files
> > #14: FILE: drivers/staging/rtl8723bs/core/rtw_ap.c:14:
> > +extern unsigned char WMM_OUI[];
> > --
> > WARNING: externs should be avoided in .c files
> > #15: FILE: drivers/staging/rtl8723bs/core/rtw_ap.c:15:
> > +extern unsigned char WPS_OUI[];
> > --
> > WARNING: externs should be avoided in .c files
> > #16: FILE: drivers/staging/rtl8723bs/core/rtw_ap.c:16:
> > +extern unsigned char P2P_OUI[];
> > --
> > 
> > but the file checked has 4 extern declaration:
> > -
> > #define _RTW_AP_C_
> > 
> > #include 
> > #include 
> > #include 
> > 
> > extern unsigned char RTW_WPA_OUI[];
> > extern unsigned char WMM_OUI[];
> > extern unsigned char WPS_OUI[];
> > extern unsigned char P2P_OUI[];
> > 
> > void init_mlme_ap_info(struct adapter *padapter)
> > ---
> > 
> > If I add a ';' this way:
> > 
> > #define _RTW_AP_C_
> > 
> > #include 
> > #include 
> > #include 
> > ;
> > extern unsigned char RTW_WPA_OUI[];
> > extern unsigned char WMM_OUI[];
> > extern unsigned char WPS_OUI[];
> > extern unsigned char P2P_OUI[];
> > 
> > void init_mlme_ap_info(struct adapter *padapter)
> > 
> 
> Wait, why would you do the above?

It is rather an ugly hack.

> Don't try to trick a perl script that has a hard time parsing C files,
> try to resolve the original issue here.
> 
> And that is that the above definitions should be in a .h file somewhere.
> If you make that move then all should be fine.

Actually, these would seem to be better as one or multiple functions with
local statics or even as static inlines functions in the .h file

$ git grep -w RTW_WPA_OUI drivers/staging/rtl8723bs/core
drivers/staging/rtl8723bs/core/rtw_ap.c:extern unsigned char RTW_WPA_OUI[];
drivers/staging/rtl8723bs/core/rtw_ap.c:if (!memcmp(RTW_WPA_OUI, oui, 
4))
drivers/staging/rtl8723bs/core/rtw_mlme_ext.c:unsigned char RTW_WPA_OUI[] = 
{0x00, 0x50, 0xf2, 0x01};
drivers/staging/rtl8723bs/core/rtw_mlme_ext.c:  if 
((!memcmp(pIE->data, RTW_WPA_OUI, 4)) ||
drivers/staging/rtl8723bs/core/rtw_wlan_util.c:extern unsigned char 
RTW_WPA_OUI[];
drivers/staging/rtl8723bs/core/rtw_wlan_util.c: if 
((!memcmp(pIE->data, RTW_WPA_OUI, 4)) && (!memcmp((pIE->data + 12), 
WPA_TKIP_CIPHER, 4)))

$ git grep -w WMM_OUI drivers/staging/rtl8723bs/core
drivers/staging/rtl8723bs/core/rtw_ap.c:extern unsigned char WMM_OUI[];
drivers/staging/rtl8723bs/core/rtw_ap.c:else if (!memcmp(WMM_OUI, oui, 
4))
drivers/staging/rtl8723bs/core/rtw_mlme_ext.c:unsigned char WMM_OUI[] = {0x00, 
0x50, 0xf2, 0x02};
drivers/staging/rtl8723bs/core/rtw_mlme_ext.c:  
(!memcmp(pIE->data, WMM_OUI, 4)) ||
drivers/staging/rtl8723bs/core/rtw_mlme_ext.c:  if 
(!memcmp(pIE->data, WMM_OUI, 4))
drivers/staging/rtl8723bs/include/rtw_mlme_ext.h:extern unsigned char WMM_OUI[];

$ git grep -w WPS_OUI drivers/staging/rtl8723bs/core
drivers/staging/rtl8723bs/core/rtw_ap.c:extern unsigned char WPS_OUI[];
drivers/staging/rtl8723bs/core/rtw_ap.c:else if (!memcmp(WPS_OUI, oui, 
4))
drivers/staging/rtl8723bs/core/rtw_mlme_ext.c:unsigned char WPS_OUI[] = {0x00, 
0x50, 0xf2, 0x04};
drivers/staging/rtl8723bs/core/rtw_mlme_ext.c:  
(!memcmp(pIE->data, WPS_OUI, 4))) {
drivers/staging/rtl8723bs/core/rtw_mlme_ext.c:  if 
((!padapter->registrypriv.wifi_spec) && (!memcmp(pIE->data, WPS_OUI, 4))) {

$ git grep -w P2P_OUI drivers/staging/rtl8723bs/core
drivers/staging/rtl8723bs/core/rtw_ap.c:extern unsigned char P2P_OUI[];
drivers/staging/rtl8723bs/core/rtw_ap.c:else if (!memcmp(P2P_OUI, oui, 
4))
drivers/staging/rtl8723bs/core/rtw_mlme_ext.c:unsigned char P2P_OUI[] = {0x50, 
0x6F, 0x9A, 0x09};
drivers/staging/rtl8723bs/core/rtw_mlme_ext.c:  if (!memcmp(frame_body + 2, 
P2P_OUI, 4)) {

So maybe something like the below (written in email client, maybe typos)

enum oui_type {
RTW_WPA,
WMM,
WPS,
P2P
};

bool is_oui_type(const u8 *mem, enum oui_type type)
{
static const u8 rtw_wpa[] = {0x00, 0x50, 0xf2, 0x01};
static const u8 wmm[] = {0x00, 0x50, 0xf2, 0x02};
static const u8 wps[] = {0x00, 0x50, 0xf2, 0x04};
static const u8 p2p[] = {0x50, 0x6F, 0x9A, 0x09};

const u8 *oui;

if (type == RTW_WPA)
oui = rtw_wpa;
else if (type == WMM)
oui = wmm;
else if (type == WPS)
oui = wps;
else if (type == P2P)
oui = p2p;

CHECKPATCH: missing a warning soon after include files decl -c

2021-03-20 Thread Fabio Aiuto 
Hi,

here's an issue in checkpatch.pl

$ perl script/checkpatch.pl -f drivers/staging/rtl8723bs/core/rtw_ap.c

I get three warning related to an extern declaration

WARNING: externs should be avoided in .c files
#14: FILE: drivers/staging/rtl8723bs/core/rtw_ap.c:14:
+extern unsigned char WMM_OUI[];
--
WARNING: externs should be avoided in .c files
#15: FILE: drivers/staging/rtl8723bs/core/rtw_ap.c:15:
+extern unsigned char WPS_OUI[];
--
WARNING: externs should be avoided in .c files
#16: FILE: drivers/staging/rtl8723bs/core/rtw_ap.c:16:
+extern unsigned char P2P_OUI[];
--

but the file checked has 4 extern declaration:
-
#define _RTW_AP_C_

#include 
#include 
#include 

extern unsigned char RTW_WPA_OUI[];
extern unsigned char WMM_OUI[];
extern unsigned char WPS_OUI[];
extern unsigned char P2P_OUI[];

void init_mlme_ap_info(struct adapter *padapter)
---

If I add a ';' this way:

#define _RTW_AP_C_

#include 
#include 
#include 
;
extern unsigned char RTW_WPA_OUI[];
extern unsigned char WMM_OUI[];
extern unsigned char WPS_OUI[];
extern unsigned char P2P_OUI[];

void init_mlme_ap_info(struct adapter *padapter)


the missing warning appears:
--
WARNING: externs should be avoided in .c files
#13: FILE: drivers/staging/rtl8723bs/core/rtw_ap.c:13:
+extern unsigned char RTW_WPA_OUI[];
--
WARNING: externs should be avoided in .c files
#14: FILE: drivers/staging/rtl8723bs/core/rtw_ap.c:14:
+extern unsigned char WMM_OUI[];
--
WARNING: externs should be avoided in .c files
#15: FILE: drivers/staging/rtl8723bs/core/rtw_ap.c:15:
+extern unsigned char WPS_OUI[];
--
WARNING: externs should be avoided in .c files
#16: FILE: drivers/staging/rtl8723bs/core/rtw_ap.c:16:
+extern unsigned char P2P_OUI[];
---
Applying this ugly patch to debug:

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index df8b23dc1eb0..ecbbb731361c 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -6810,6 +6810,11 @@ sub process {
{
WARN("AVOID_EXTERNS",
 "externs should be avoided in .c files\n" .  
$herecurr);
+   } elsif ($realfile =~ /\.c$/ && defined $stat &&
+   $stat =~ /extern\s+/)
+   {
+   WARN("AVOID_EXTERNS",
+"print stat value\n" .  $stat);
}
 
 # check for function declarations that have arguments without identifier names

--
I get:

WARNING: print stat value
+#include 
 #include 
 #include 
 
 extern unsigned char RTW_WPA_OUI[];
WARNING: externs should be avoided in .c files
#14: FILE: drivers/staging/rtl8723bs/core/rtw_ap.c:14:
+extern unsigned char WMM_OUI[];

WARNING: externs should be avoided in .c files
#15: FILE: drivers/staging/rtl8723bs/core/rtw_ap.c:15:
+extern unsigned char WPS_OUI[];

WARNING: externs should be avoided in .c files
#16: FILE: drivers/staging/rtl8723bs/core/rtw_ap.c:16:
+extern unsigned char P2P_OUI[];

I don't know if it's best to fix the regex of the particular "extern in .c 
files issue"
or maybe how the statement is computed. Should a statement represent a single 
line?

Suggestions are welcome,

fabio

Re: CHECKPATCH: missing a warning soon after include files decl -c

2021-03-20 Thread Greg KH 
On Sat, Mar 20, 2021 at 11:54:24AM +0100, Fabio Aiuto wrote:
> Hi,
> 
> here's an issue in checkpatch.pl
> 
> $ perl script/checkpatch.pl -f drivers/staging/rtl8723bs/core/rtw_ap.c
> 
> I get three warning related to an extern declaration
> 
> WARNING: externs should be avoided in .c files
> #14: FILE: drivers/staging/rtl8723bs/core/rtw_ap.c:14:
> +extern unsigned char WMM_OUI[];
> --
> WARNING: externs should be avoided in .c files
> #15: FILE: drivers/staging/rtl8723bs/core/rtw_ap.c:15:
> +extern unsigned char WPS_OUI[];
> --
> WARNING: externs should be avoided in .c files
> #16: FILE: drivers/staging/rtl8723bs/core/rtw_ap.c:16:
> +extern unsigned char P2P_OUI[];
> --
> 
> but the file checked has 4 extern declaration:
> -
> #define _RTW_AP_C_
> 
> #include 
> #include 
> #include 
> 
> extern unsigned char RTW_WPA_OUI[];
> extern unsigned char WMM_OUI[];
> extern unsigned char WPS_OUI[];
> extern unsigned char P2P_OUI[];
> 
> void init_mlme_ap_info(struct adapter *padapter)
> ---
> 
> If I add a ';' this way:
> 
> #define _RTW_AP_C_
> 
> #include 
> #include 
> #include 
> ;
> extern unsigned char RTW_WPA_OUI[];
> extern unsigned char WMM_OUI[];
> extern unsigned char WPS_OUI[];
> extern unsigned char P2P_OUI[];
> 
> void init_mlme_ap_info(struct adapter *padapter)
> 

Wait, why would you do the above?

Don't try to trick a perl script that has a hard time parsing C files,
try to resolve the original issue here.

And that is that the above definitions should be in a .h file somewhere.
If you make that move then all should be fine.

thanks,

greg k-h