From: Jérôme Pouiller <[email protected]>
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 <[email protected]>
---
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);
WARN_ON(sizeof(body->ssid) < ssidlen);
WARN(!conf->ibss_joined && !ssidlen, "joining an unknown BSS");
- if (WARN_ON(!channel))
- return -EINVAL;
if (!hif)
return -ENOMEM;
body->infrastructure_bss_mode = !conf->ibss_joined;
--
2.33.0
On Mon, Sep 13, 2021 at 10:30:24AM +0200, Jerome Pouiller wrote:
> From: J?r?me Pouiller <[email protected]>
>
> 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 <[email protected]>
> ---
> 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