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 <drv_types.h>
> > > #include <rtw_debug.h>
> > > #include <asm/unaligned.h>
> > >
> > > 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 <drv_types.h>
> > > #include <rtw_debug.h>
> > > #include <asm/unaligned.h>
> > > ;
> > > 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,
> 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.
>
>
>
Thank you Joe, is the whole analisys involved with the simple task of removing
them from .c files?
