Re: [PATCH -next 5/6] staging: rtl8723bs: mark some variables as __maybe_unused

[Edmundo Carmona Antoranz <eantoranz@gmail.com>]

On Sat, Mar 27, 2021 at 2:18 AM Dan Carpenter <dan.carpenter@oracle.com> wrote:
>
> This is just papering over badness.  Leave the warnings as is so someone
> will notice it and fix it properly in the future.
>

Interesting.... I wonder what the right way to handle this would be
then. Is it a more appropriate solution defining the debug macros
_only_ if we are debugging and then also use #ifs in code to define
the variables and handle them?

So, currently, in rtw_debug.h, we have this (lines ~134-140):

#define RT_TRACE(_Comp, _Level, Fmt) do {} while (0)
#define RT_PRINT_DATA(_Comp, _Level, _TitleString, _HexData,
_HexDataLen) do {} while (0)

#define DBG_871X(x, ...) do {} while (0)
#define MSG_8192C(x, ...) do {} while (0)
#define DBG_8192C(x, ...) do {} while (0)
#define DBG_871X_LEVEL(x, ...) do {} while (0)


And then, a bit further down the road in the same file:
#ifdef DEBUG
#if defined(_dbgdump)
    #undef DBG_871X
    #define DBG_871X(...)     do {\
        _dbgdump(DRIVER_PREFIX __VA_ARGS__);\
    } while (0)

    #undef MSG_8192C
    #define MSG_8192C(...)     do {\
        _dbgdump(DRIVER_PREFIX __VA_ARGS__);\
    } while (0)

    #undef DBG_8192C
    #define DBG_8192C(...)     do {\
        _dbgdump(DRIVER_PREFIX __VA_ARGS__);\
    } while (0)
#endif /* defined(_dbgdump) */
#endif /* DEBUG */

If we remove the definition of the macros that were around line 134,
we need to make sure that those macros are not used in code if the
same conditions are not met... so, it might mean that we control if we
define the variables we are using... one of the examples from this
patch:


    unsigned char *p, *pos, *wpa_ie;
#ifdef DEBUG && defined(_dbgdump)
    unsigned char * reassoc;
#endif
    unsigned char WMM_IE[] = {0x00, 0x50, 0xf2, 0x02, 0x00, 0x01};
.
.
.
    if (frame_type == WIFI_ASSOCREQ) {
#ifdef DEBUG && defined(_dbgdump)
        reassoc = 0;
#endif
        ie_offset = _ASOCREQ_IE_OFFSET_;
    } else { /*  WIFI_REASSOCREQ */
#ifdef DEBUG && defined(_dbgdump)
        reassoc = 1;
#endif
        ie_offset = _REASOCREQ_IE_OFFSET_;
    }

    if (pkt_len < sizeof(struct ieee80211_hdr_3addr) + ie_offset) {
#ifdef DEBUG && defined(_dbgdump)
        DBG_871X("handle_assoc(reassoc =%d) - too short payload (len =%lu)"
             "\n", reassoc, (unsigned long)pkt_len);
#endif
        return _FAIL;
    }

> regards,
> dan carpenter
>