linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] staging: r8188eu: os_dep: simplifiy the rtw_resume function
@ 2021-09-04 12:14 Saurav Girepunje
  2021-09-05 14:48 ` Greg KH
  0 siblings, 1 reply; 4+ messages in thread
From: Saurav Girepunje @ 2021-09-04 12:14 UTC (permalink / raw)
  To: Larry.Finger, phil, gregkh, saurav.girepunje, straube.linux,
	martin, nathan, linux-staging, linux-kernel
  Cc: saurav.girepunje

Remove unused variable ret and pwrpriv.
Remove the condition with no effect (if == else) in usb_intf.c
file.
Remove rtw_resume_process() and move whole thing to rtw_resume().

Signed-off-by: Saurav Girepunje <saurav.girepunje@gmail.com>
---

ChangeLog V2:
- Remove rtw_resume_process() and move whole thing to rtw_resume().
---
 drivers/staging/r8188eu/include/usb_osintf.h |  2 --
 drivers/staging/r8188eu/os_dep/usb_intf.c    | 12 ------------
 2 files changed, 14 deletions(-)

diff --git a/drivers/staging/r8188eu/include/usb_osintf.h b/drivers/staging/r8188eu/include/usb_osintf.h
index d1a1f739309c..34229b1cb081 100644
--- a/drivers/staging/r8188eu/include/usb_osintf.h
+++ b/drivers/staging/r8188eu/include/usb_osintf.h
@@ -24,6 +24,4 @@ void *scdb_findEntry(struct adapter *priv, unsigned char *macAddr,
 void nat25_db_expire(struct adapter *priv);
 int nat25_db_handle(struct adapter *priv, struct sk_buff *skb, int method);

-int rtw_resume_process(struct adapter *padapter);
-
 #endif
diff --git a/drivers/staging/r8188eu/os_dep/usb_intf.c b/drivers/staging/r8188eu/os_dep/usb_intf.c
index bb85ab77fd26..77b03e7631b7 100644
--- a/drivers/staging/r8188eu/os_dep/usb_intf.c
+++ b/drivers/staging/r8188eu/os_dep/usb_intf.c
@@ -493,18 +493,6 @@ static int rtw_resume(struct usb_interface *pusb_intf)
 {
 	struct dvobj_priv *dvobj = usb_get_intfdata(pusb_intf);
 	struct adapter *padapter = dvobj->if1;
-	struct pwrctrl_priv *pwrpriv = &padapter->pwrctrlpriv;
-	int ret = 0;
-
-	if (pwrpriv->bInternalAutoSuspend)
-		ret = rtw_resume_process(padapter);
-	else
-		ret = rtw_resume_process(padapter);
-	return ret;
-}
-
-int rtw_resume_process(struct adapter *padapter)
-{
 	struct net_device *pnetdev;
 	struct pwrctrl_priv *pwrpriv = NULL;
 	int ret = -1;
--
2.32.0


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] staging: r8188eu: os_dep: simplifiy the rtw_resume function
  2021-09-04 12:14 [PATCH v2] staging: r8188eu: os_dep: simplifiy the rtw_resume function Saurav Girepunje
@ 2021-09-05 14:48 ` Greg KH
  2021-09-08 17:47   ` Saurav Girepunje
  0 siblings, 1 reply; 4+ messages in thread
From: Greg KH @ 2021-09-05 14:48 UTC (permalink / raw)
  To: Saurav Girepunje
  Cc: Larry.Finger, phil, straube.linux, martin, nathan, linux-staging,
	linux-kernel, saurav.girepunje

On Sat, Sep 04, 2021 at 05:44:44PM +0530, Saurav Girepunje wrote:
> Remove unused variable ret and pwrpriv.
> Remove the condition with no effect (if == else) in usb_intf.c
> file.
> Remove rtw_resume_process() and move whole thing to rtw_resume().
> 
> Signed-off-by: Saurav Girepunje <saurav.girepunje@gmail.com>
> ---
> 
> ChangeLog V2:
> - Remove rtw_resume_process() and move whole thing to rtw_resume().
> ---
>  drivers/staging/r8188eu/include/usb_osintf.h |  2 --
>  drivers/staging/r8188eu/os_dep/usb_intf.c    | 12 ------------
>  2 files changed, 14 deletions(-)
> 
> diff --git a/drivers/staging/r8188eu/include/usb_osintf.h b/drivers/staging/r8188eu/include/usb_osintf.h
> index d1a1f739309c..34229b1cb081 100644
> --- a/drivers/staging/r8188eu/include/usb_osintf.h
> +++ b/drivers/staging/r8188eu/include/usb_osintf.h
> @@ -24,6 +24,4 @@ void *scdb_findEntry(struct adapter *priv, unsigned char *macAddr,
>  void nat25_db_expire(struct adapter *priv);
>  int nat25_db_handle(struct adapter *priv, struct sk_buff *skb, int method);
> 
> -int rtw_resume_process(struct adapter *padapter);
> -
>  #endif
> diff --git a/drivers/staging/r8188eu/os_dep/usb_intf.c b/drivers/staging/r8188eu/os_dep/usb_intf.c
> index bb85ab77fd26..77b03e7631b7 100644
> --- a/drivers/staging/r8188eu/os_dep/usb_intf.c
> +++ b/drivers/staging/r8188eu/os_dep/usb_intf.c
> @@ -493,18 +493,6 @@ static int rtw_resume(struct usb_interface *pusb_intf)
>  {
>  	struct dvobj_priv *dvobj = usb_get_intfdata(pusb_intf);
>  	struct adapter *padapter = dvobj->if1;
> -	struct pwrctrl_priv *pwrpriv = &padapter->pwrctrlpriv;
> -	int ret = 0;
> -
> -	if (pwrpriv->bInternalAutoSuspend)
> -		ret = rtw_resume_process(padapter);
> -	else
> -		ret = rtw_resume_process(padapter);
> -	return ret;
> -}
> -
> -int rtw_resume_process(struct adapter *padapter)
> -{
>  	struct net_device *pnetdev;
>  	struct pwrctrl_priv *pwrpriv = NULL;
>  	int ret = -1;

You can also remove the test for padapter being NULL in this function,
as you just proved it could never be null otherwise the above function
you removed would have crashed, right?  You should do that all at once
so we remember why that test was removed.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] staging: r8188eu: os_dep: simplifiy the rtw_resume function
  2021-09-05 14:48 ` Greg KH
@ 2021-09-08 17:47   ` Saurav Girepunje
  2021-09-09 10:03     ` Greg KH
  0 siblings, 1 reply; 4+ messages in thread
From: Saurav Girepunje @ 2021-09-08 17:47 UTC (permalink / raw)
  To: Greg KH
  Cc: saurav.girepunje, Larry.Finger, phil, straube.linux, martin,
	nathan, linux-staging, linux-kernel



On 05/09/21 8:18 pm, Greg KH wrote:
> On Sat, Sep 04, 2021 at 05:44:44PM +0530, Saurav Girepunje wrote:
>> Remove unused variable ret and pwrpriv.
>> Remove the condition with no effect (if == else) in usb_intf.c
>> file.
>> Remove rtw_resume_process() and move whole thing to rtw_resume().
>>
>> Signed-off-by: Saurav Girepunje <saurav.girepunje@gmail.com>
>> ---
>>
>> ChangeLog V2:
>> - Remove rtw_resume_process() and move whole thing to rtw_resume().
>> ---
>>   drivers/staging/r8188eu/include/usb_osintf.h |  2 --
>>   drivers/staging/r8188eu/os_dep/usb_intf.c    | 12 ------------
>>   2 files changed, 14 deletions(-)
>>
>> diff --git a/drivers/staging/r8188eu/include/usb_osintf.h b/drivers/staging/r8188eu/include/usb_osintf.h
>> index d1a1f739309c..34229b1cb081 100644
>> --- a/drivers/staging/r8188eu/include/usb_osintf.h
>> +++ b/drivers/staging/r8188eu/include/usb_osintf.h
>> @@ -24,6 +24,4 @@ void *scdb_findEntry(struct adapter *priv, unsigned char *macAddr,
>>   void nat25_db_expire(struct adapter *priv);
>>   int nat25_db_handle(struct adapter *priv, struct sk_buff *skb, int method);
>>
>> -int rtw_resume_process(struct adapter *padapter);
>> -
>>   #endif
>> diff --git a/drivers/staging/r8188eu/os_dep/usb_intf.c b/drivers/staging/r8188eu/os_dep/usb_intf.c
>> index bb85ab77fd26..77b03e7631b7 100644
>> --- a/drivers/staging/r8188eu/os_dep/usb_intf.c
>> +++ b/drivers/staging/r8188eu/os_dep/usb_intf.c
>> @@ -493,18 +493,6 @@ static int rtw_resume(struct usb_interface *pusb_intf)
>>   {
>>   	struct dvobj_priv *dvobj = usb_get_intfdata(pusb_intf);
>>   	struct adapter *padapter = dvobj->if1;
>> -	struct pwrctrl_priv *pwrpriv = &padapter->pwrctrlpriv;
>> -	int ret = 0;
>> -
>> -	if (pwrpriv->bInternalAutoSuspend)
>> -		ret = rtw_resume_process(padapter);
>> -	else
>> -		ret = rtw_resume_process(padapter);
>> -	return ret;
>> -}
>> -
>> -int rtw_resume_process(struct adapter *padapter)
>> -{
>>   	struct net_device *pnetdev;
>>   	struct pwrctrl_priv *pwrpriv = NULL;
>>   	int ret = -1;
> 
> You can also remove the test for padapter being NULL in this function,
> as you just proved it could never be null otherwise the above function
> you removed would have crashed, right?  You should do that all at once
> so we remember why that test was removed.
> 
> thanks,
> 
> greg k-h
> 

Yes, padapter was deference without checking it could be NULL on rtw_resume.

I think having NULL check would be safe. However if padapter is NULL 
then on goto exit "if (pwrpriv)" condition  will never true as pwrpriv 
is initialized with NULL. There should be one more goto exit just to 
return.!

let me know you thought greg,

Regards,
Saurav Girepunje

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] staging: r8188eu: os_dep: simplifiy the rtw_resume function
  2021-09-08 17:47   ` Saurav Girepunje
@ 2021-09-09 10:03     ` Greg KH
  0 siblings, 0 replies; 4+ messages in thread
From: Greg KH @ 2021-09-09 10:03 UTC (permalink / raw)
  To: Saurav Girepunje
  Cc: saurav.girepunje, Larry.Finger, phil, straube.linux, martin,
	nathan, linux-staging, linux-kernel

On Wed, Sep 08, 2021 at 11:17:20PM +0530, Saurav Girepunje wrote:
> 
> 
> On 05/09/21 8:18 pm, Greg KH wrote:
> > On Sat, Sep 04, 2021 at 05:44:44PM +0530, Saurav Girepunje wrote:
> > > Remove unused variable ret and pwrpriv.
> > > Remove the condition with no effect (if == else) in usb_intf.c
> > > file.
> > > Remove rtw_resume_process() and move whole thing to rtw_resume().
> > > 
> > > Signed-off-by: Saurav Girepunje <saurav.girepunje@gmail.com>
> > > ---
> > > 
> > > ChangeLog V2:
> > > - Remove rtw_resume_process() and move whole thing to rtw_resume().
> > > ---
> > >   drivers/staging/r8188eu/include/usb_osintf.h |  2 --
> > >   drivers/staging/r8188eu/os_dep/usb_intf.c    | 12 ------------
> > >   2 files changed, 14 deletions(-)
> > > 
> > > diff --git a/drivers/staging/r8188eu/include/usb_osintf.h b/drivers/staging/r8188eu/include/usb_osintf.h
> > > index d1a1f739309c..34229b1cb081 100644
> > > --- a/drivers/staging/r8188eu/include/usb_osintf.h
> > > +++ b/drivers/staging/r8188eu/include/usb_osintf.h
> > > @@ -24,6 +24,4 @@ void *scdb_findEntry(struct adapter *priv, unsigned char *macAddr,
> > >   void nat25_db_expire(struct adapter *priv);
> > >   int nat25_db_handle(struct adapter *priv, struct sk_buff *skb, int method);
> > > 
> > > -int rtw_resume_process(struct adapter *padapter);
> > > -
> > >   #endif
> > > diff --git a/drivers/staging/r8188eu/os_dep/usb_intf.c b/drivers/staging/r8188eu/os_dep/usb_intf.c
> > > index bb85ab77fd26..77b03e7631b7 100644
> > > --- a/drivers/staging/r8188eu/os_dep/usb_intf.c
> > > +++ b/drivers/staging/r8188eu/os_dep/usb_intf.c
> > > @@ -493,18 +493,6 @@ static int rtw_resume(struct usb_interface *pusb_intf)
> > >   {
> > >   	struct dvobj_priv *dvobj = usb_get_intfdata(pusb_intf);
> > >   	struct adapter *padapter = dvobj->if1;
> > > -	struct pwrctrl_priv *pwrpriv = &padapter->pwrctrlpriv;
> > > -	int ret = 0;
> > > -
> > > -	if (pwrpriv->bInternalAutoSuspend)
> > > -		ret = rtw_resume_process(padapter);
> > > -	else
> > > -		ret = rtw_resume_process(padapter);
> > > -	return ret;
> > > -}
> > > -
> > > -int rtw_resume_process(struct adapter *padapter)
> > > -{
> > >   	struct net_device *pnetdev;
> > >   	struct pwrctrl_priv *pwrpriv = NULL;
> > >   	int ret = -1;
> > 
> > You can also remove the test for padapter being NULL in this function,
> > as you just proved it could never be null otherwise the above function
> > you removed would have crashed, right?  You should do that all at once
> > so we remember why that test was removed.
> > 
> > thanks,
> > 
> > greg k-h
> > 
> 
> Yes, padapter was deference without checking it could be NULL on rtw_resume.

How can it ever be NULL there?

> I think having NULL check would be safe. However if padapter is NULL then on
> goto exit "if (pwrpriv)" condition  will never true as pwrpriv is
> initialized with NULL. There should be one more goto exit just to return.!
> 
> let me know you thought greg,

Don't check for things that can never happen.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2021-09-09 10:03 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-04 12:14 [PATCH v2] staging: r8188eu: os_dep: simplifiy the rtw_resume function Saurav Girepunje
2021-09-05 14:48 ` Greg KH
2021-09-08 17:47   ` Saurav Girepunje
2021-09-09 10:03     ` Greg KH

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).