* [PATCH] staging: r8188eu: remove unnecessary NULL check
@ 2021-11-22 19:53 Vihas Mak
2021-11-22 21:22 ` Pavel Skripkin
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Vihas Mak @ 2021-11-22 19:53 UTC (permalink / raw)
To: Larry.Finger, phil
Cc: gregkh, straube.linux, martin, paskripkin, linux-staging, linux-kernel
remove unnecessary NULL check surrounding rtw_free_netdev(), as the check
is already performed inside rtw_free_netdev() in
drivers/staging/r8188eu/os_dep/osdep_service.c.
Signed-off-by: Vihas Mak <makvihas@gmail.com>
---
drivers/staging/r8188eu/os_dep/usb_intf.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/staging/r8188eu/os_dep/usb_intf.c b/drivers/staging/r8188eu/os_dep/usb_intf.c
index 5a35d9fe3fc9..392bd7868519 100644
--- a/drivers/staging/r8188eu/os_dep/usb_intf.c
+++ b/drivers/staging/r8188eu/os_dep/usb_intf.c
@@ -466,8 +466,7 @@ static void rtw_usb_if1_deinit(struct adapter *if1)
if1->hw_init_completed);
rtw_handle_dualmac(if1, 0);
rtw_free_drv_sw(if1);
- if (pnetdev)
- rtw_free_netdev(pnetdev);
+ rtw_free_netdev(pnetdev);
}
static int rtw_drv_init(struct usb_interface *pusb_intf, const struct usb_device_id *pdid)
--
2.25.1
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH] staging: r8188eu: remove unnecessary NULL check
2021-11-22 19:53 [PATCH] staging: r8188eu: remove unnecessary NULL check Vihas Mak
@ 2021-11-22 21:22 ` Pavel Skripkin
2021-11-23 23:49 ` Vihas Mak
2021-11-23 8:06 ` Dan Carpenter
2021-11-24 10:00 ` Greg KH
2 siblings, 1 reply; 9+ messages in thread
From: Pavel Skripkin @ 2021-11-22 21:22 UTC (permalink / raw)
To: Vihas Mak, Larry.Finger, phil
Cc: gregkh, straube.linux, martin, linux-staging, linux-kernel
On 11/22/21 22:53, Vihas Mak wrote:
> remove unnecessary NULL check surrounding rtw_free_netdev(), as the check
> is already performed inside rtw_free_netdev() in
> drivers/staging/r8188eu/os_dep/osdep_service.c.
>
> Signed-off-by: Vihas Mak <makvihas@gmail.com>
Reviewed-by: Pavel Skripkin <paskripkin@gmail.com>
BTW, same can be done in rtw_usb_if1_init().
With regards,
Pavel Skripkin
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] staging: r8188eu: remove unnecessary NULL check
2021-11-22 21:22 ` Pavel Skripkin
@ 2021-11-23 23:49 ` Vihas Mak
0 siblings, 0 replies; 9+ messages in thread
From: Vihas Mak @ 2021-11-23 23:49 UTC (permalink / raw)
To: Pavel Skripkin
Cc: Larry.Finger, Phillip Potter, Greg KH, straube.linux, martin,
linux-staging, linux-kernel
>> BTW, same can be done in rtw_usb_if1_init().
Yea, I noticed that too. But the NULL check in rtw_usb_if1_init() has
a follow up "else if", so I didn't change it.
Thanks,
Vihas
On Tue, Nov 23, 2021 at 2:52 AM Pavel Skripkin <paskripkin@gmail.com> wrote:
>
> On 11/22/21 22:53, Vihas Mak wrote:
> > remove unnecessary NULL check surrounding rtw_free_netdev(), as the check
> > is already performed inside rtw_free_netdev() in
> > drivers/staging/r8188eu/os_dep/osdep_service.c.
> >
> > Signed-off-by: Vihas Mak <makvihas@gmail.com>
>
> Reviewed-by: Pavel Skripkin <paskripkin@gmail.com>
>
> BTW, same can be done in rtw_usb_if1_init().
>
>
>
> With regards,
> Pavel Skripkin
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] staging: r8188eu: remove unnecessary NULL check
2021-11-22 19:53 [PATCH] staging: r8188eu: remove unnecessary NULL check Vihas Mak
2021-11-22 21:22 ` Pavel Skripkin
@ 2021-11-23 8:06 ` Dan Carpenter
2021-11-24 10:00 ` Greg KH
2 siblings, 0 replies; 9+ messages in thread
From: Dan Carpenter @ 2021-11-23 8:06 UTC (permalink / raw)
To: Vihas Mak
Cc: Larry.Finger, phil, gregkh, straube.linux, martin, paskripkin,
linux-staging, linux-kernel
On Tue, Nov 23, 2021 at 01:23:50AM +0530, Vihas Mak wrote:
> remove unnecessary NULL check surrounding rtw_free_netdev(), as the check
> is already performed inside rtw_free_netdev() in
> drivers/staging/r8188eu/os_dep/osdep_service.c.
>
> Signed-off-by: Vihas Mak <makvihas@gmail.com>
> ---
> drivers/staging/r8188eu/os_dep/usb_intf.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/staging/r8188eu/os_dep/usb_intf.c b/drivers/staging/r8188eu/os_dep/usb_intf.c
> index 5a35d9fe3fc9..392bd7868519 100644
> --- a/drivers/staging/r8188eu/os_dep/usb_intf.c
> +++ b/drivers/staging/r8188eu/os_dep/usb_intf.c
> @@ -466,8 +466,7 @@ static void rtw_usb_if1_deinit(struct adapter *if1)
> if1->hw_init_completed);
> rtw_handle_dualmac(if1, 0);
> rtw_free_drv_sw(if1);
> - if (pnetdev)
> - rtw_free_netdev(pnetdev);
> + rtw_free_netdev(pnetdev);
> }
I'm not a huge fan of these sorts of patches. They don't make the code
more readable because they hide the complexity.
Occasionally we will get a forest cobra in our yard and everyone is
screaming and panicking. I'm like, "Calm down. Once you've spotted the
snake, even a deadly snake, then the danger has passed." You can just
stay two or three meters away and you're fine. Call a snake catcher.
What you're doing here is you've got a potential NULL dereference which
is the snake. And this patch is saying, "Snakes are so messy! Let's
hide it in the bushes next to the sidewalk where no one can see it."
Hash tag, folksy wisdom. #snakes
On the other hand, it might be worth checking if "pnetdev" can even be
NULL at this point, and then deleting both of the NULL checks. That
would be a very good clean up.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] staging: r8188eu: remove unnecessary NULL check
2021-11-22 19:53 [PATCH] staging: r8188eu: remove unnecessary NULL check Vihas Mak
2021-11-22 21:22 ` Pavel Skripkin
2021-11-23 8:06 ` Dan Carpenter
@ 2021-11-24 10:00 ` Greg KH
2021-11-25 1:32 ` Vihas Mak
2 siblings, 1 reply; 9+ messages in thread
From: Greg KH @ 2021-11-24 10:00 UTC (permalink / raw)
To: Vihas Mak
Cc: Larry.Finger, phil, straube.linux, martin, paskripkin,
linux-staging, linux-kernel
On Tue, Nov 23, 2021 at 01:23:50AM +0530, Vihas Mak wrote:
> remove unnecessary NULL check surrounding rtw_free_netdev(), as the check
> is already performed inside rtw_free_netdev() in
> drivers/staging/r8188eu/os_dep/osdep_service.c.
>
> Signed-off-by: Vihas Mak <makvihas@gmail.com>
> ---
> drivers/staging/r8188eu/os_dep/usb_intf.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/staging/r8188eu/os_dep/usb_intf.c b/drivers/staging/r8188eu/os_dep/usb_intf.c
> index 5a35d9fe3fc9..392bd7868519 100644
> --- a/drivers/staging/r8188eu/os_dep/usb_intf.c
> +++ b/drivers/staging/r8188eu/os_dep/usb_intf.c
> @@ -466,8 +466,7 @@ static void rtw_usb_if1_deinit(struct adapter *if1)
> if1->hw_init_completed);
> rtw_handle_dualmac(if1, 0);
> rtw_free_drv_sw(if1);
> - if (pnetdev)
> - rtw_free_netdev(pnetdev);
> + rtw_free_netdev(pnetdev);
As Dan said, this isn't usually a good idea to hide this.
And are you sure this ever could be NULL?
thanks,
greg k-h
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] staging: r8188eu: remove unnecessary NULL check
2021-11-24 10:00 ` Greg KH
@ 2021-11-25 1:32 ` Vihas Mak
2021-11-25 7:09 ` Dan Carpenter
0 siblings, 1 reply; 9+ messages in thread
From: Vihas Mak @ 2021-11-25 1:32 UTC (permalink / raw)
To: Greg KH
Cc: Larry.Finger, Phillip Potter, straube.linux, martin,
Pavel Skripkin, linux-staging, linux-kernel
>> And are you sure this ever could be NULL?
Yes.
The function rtw_free_netdev() performs a NULL check before actually
freeing the structure, so the "if (pnetdev)" check isn't really
necessary before calling rtw_free_netdev().
That's the reason why I removed that check.
On Wed, Nov 24, 2021 at 3:30 PM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Tue, Nov 23, 2021 at 01:23:50AM +0530, Vihas Mak wrote:
> > remove unnecessary NULL check surrounding rtw_free_netdev(), as the check
> > is already performed inside rtw_free_netdev() in
> > drivers/staging/r8188eu/os_dep/osdep_service.c.
> >
> > Signed-off-by: Vihas Mak <makvihas@gmail.com>
> > ---
> > drivers/staging/r8188eu/os_dep/usb_intf.c | 3 +--
> > 1 file changed, 1 insertion(+), 2 deletions(-)
> >
> > diff --git a/drivers/staging/r8188eu/os_dep/usb_intf.c b/drivers/staging/r8188eu/os_dep/usb_intf.c
> > index 5a35d9fe3fc9..392bd7868519 100644
> > --- a/drivers/staging/r8188eu/os_dep/usb_intf.c
> > +++ b/drivers/staging/r8188eu/os_dep/usb_intf.c
> > @@ -466,8 +466,7 @@ static void rtw_usb_if1_deinit(struct adapter *if1)
> > if1->hw_init_completed);
> > rtw_handle_dualmac(if1, 0);
> > rtw_free_drv_sw(if1);
> > - if (pnetdev)
> > - rtw_free_netdev(pnetdev);
> > + rtw_free_netdev(pnetdev);
>
> As Dan said, this isn't usually a good idea to hide this.
>
> And are you sure this ever could be NULL?
>
> thanks,
>
> greg k-h
--
Thanks,
Vihas
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] staging: r8188eu: remove unnecessary NULL check
2021-11-25 1:32 ` Vihas Mak
@ 2021-11-25 7:09 ` Dan Carpenter
0 siblings, 0 replies; 9+ messages in thread
From: Dan Carpenter @ 2021-11-25 7:09 UTC (permalink / raw)
To: Vihas Mak
Cc: Greg KH, Larry.Finger, Phillip Potter, straube.linux, martin,
Pavel Skripkin, linux-staging, linux-kernel
On Thu, Nov 25, 2021 at 07:02:20AM +0530, Vihas Mak wrote:
> >> And are you sure this ever could be NULL?
> Yes.
>
> The function rtw_free_netdev() performs a NULL check before actually
> freeing the structure, so the "if (pnetdev)" check isn't really
> necessary before calling rtw_free_netdev().
> That's the reason why I removed that check.
>
That's not the question Greg and I asked. Re-read the emails.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH] staging: r8188eu: remove unnecessary null check
@ 2022-08-24 8:03 cgel.zte
2022-08-25 6:36 ` Dan Carpenter
0 siblings, 1 reply; 9+ messages in thread
From: cgel.zte @ 2022-08-24 8:03 UTC (permalink / raw)
To: Larry.Finger
Cc: phil, paskripkin, gregkh, straube.linux, martin, jhpark1013,
makvihas, linux-staging, linux-kernel, Minghao Chi, Zeal Robot
From: Minghao Chi <chi.minghao@zte.com.cn>
container_of is never null, so this null check is
unnecessary.
Reported-by: Zeal Robot <zealci@zte.com.cn>
Signed-off-by: Minghao Chi <chi.minghao@zte.com.cn>
---
drivers/staging/r8188eu/core/rtw_mlme.c | 4 ----
1 file changed, 4 deletions(-)
diff --git a/drivers/staging/r8188eu/core/rtw_mlme.c b/drivers/staging/r8188eu/core/rtw_mlme.c
index 56c8bd5f4c60..d089da7e90e0 100644
--- a/drivers/staging/r8188eu/core/rtw_mlme.c
+++ b/drivers/staging/r8188eu/core/rtw_mlme.c
@@ -1442,10 +1442,6 @@ int rtw_select_and_join_from_scanned_queue(struct mlme_priv *pmlmepriv)
pmlmepriv->pscanned = phead->next;
while (phead != pmlmepriv->pscanned) {
pnetwork = container_of(pmlmepriv->pscanned, struct wlan_network, list);
- if (!pnetwork) {
- ret = _FAIL;
- goto exit;
- }
pmlmepriv->pscanned = pmlmepriv->pscanned->next;
rtw_check_join_candidate(pmlmepriv, &candidate, pnetwork);
}
--
2.25.1
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH] staging: r8188eu: remove unnecessary null check
2022-08-24 8:03 [PATCH] staging: r8188eu: remove unnecessary null check cgel.zte
@ 2022-08-25 6:36 ` Dan Carpenter
0 siblings, 0 replies; 9+ messages in thread
From: Dan Carpenter @ 2022-08-25 6:36 UTC (permalink / raw)
To: cgel.zte
Cc: Larry.Finger, phil, paskripkin, gregkh, straube.linux, martin,
jhpark1013, makvihas, linux-staging, linux-kernel, Minghao Chi,
Zeal Robot
On Wed, Aug 24, 2022 at 08:03:50AM +0000, cgel.zte@gmail.com wrote:
> From: Minghao Chi <chi.minghao@zte.com.cn>
>
> container_of is never null, so this null check is
> unnecessary.
>
I can't Ack a patch with this commit message because container_of()
*CAN* be NULL. Here, it requires two things:
1) That ->list is the first struct member of struct wlan_network which
is true.
2) That "pmlmepriv->pscanned" is NULL. Which I have not looked at.
It's really ugly to check container_of() for NULL but some people do it
deliberately. Some people also will add a build time assert to ensure
that ->list is always the first element so that the check always works.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2022-08-25 6:36 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-22 19:53 [PATCH] staging: r8188eu: remove unnecessary NULL check Vihas Mak
2021-11-22 21:22 ` Pavel Skripkin
2021-11-23 23:49 ` Vihas Mak
2021-11-23 8:06 ` Dan Carpenter
2021-11-24 10:00 ` Greg KH
2021-11-25 1:32 ` Vihas Mak
2021-11-25 7:09 ` Dan Carpenter
2022-08-24 8:03 [PATCH] staging: r8188eu: remove unnecessary null check cgel.zte
2022-08-25 6:36 ` Dan Carpenter
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).