[PATCH v2 12/33] staging: wfx: simplify API coherency check

Dan Carpenter dan.carpenter at oracle.com
Mon Sep 13 10:02:05 UTC 2021


On Mon, Sep 13, 2021 at 10:30:24AM +0200, Jerome Pouiller wrote:
> From: Jérôme Pouiller <jerome.pouiller at silabs.com>
> 
> The 'channel' argument of hif_join() should never be NULL. hif_join()
> does not have the responsibility to recover bug of caller. A call to
> WARN() at the beginning of the function reminds this constraint to the
> developer.
> 
> In current code, if the argument channel is NULL, memory leaks. The new
> code just emit a warning and does not give the illusion that it is
> supported (and indeed a Oops will probably raise a few lines below).
> 
> Signed-off-by: Jérôme Pouiller <jerome.pouiller at silabs.com>
> ---
>  drivers/staging/wfx/hif_tx.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/wfx/hif_tx.c b/drivers/staging/wfx/hif_tx.c
> index 14b7e047916e..6ffbae32028b 100644
> --- a/drivers/staging/wfx/hif_tx.c
> +++ b/drivers/staging/wfx/hif_tx.c
> @@ -299,10 +299,9 @@ int hif_join(struct wfx_vif *wvif, const struct ieee80211_bss_conf *conf,
>  
>  	WARN_ON(!conf->beacon_int);
>  	WARN_ON(!conf->basic_rates);
> +	WARN_ON(!channel);

This fine.  I'm not trying to make people redo their patches especially
when you're doing a great job as a maintainer.

But generally these WARN_ON()s are pointless.  It's never going to
happen and if we try to handle all the thing which will not happen that's
an impossible task.  But specificically with NULL dereferences, the
WARN() will generate a stack trace and also the Oops will generate a
stack trace.  It's duplicative.

regards,
dan carpenter



More information about the devel mailing list