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

[Dan Carpenter]

On Mon, Sep 13, 2021 at 10:30:24AM +0200, Jerome Pouiller wrote:
> From: Jérôme Pouiller <jerome.pouiller@xxxxxxxxxx>
> 
> 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@xxxxxxxxxx>
> ---
>  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