From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 82DB972 for ; Thu, 9 Sep 2021 10:03:55 +0000 (UTC) Received: by mail.kernel.org (Postfix) with ESMTPSA id AEF6161104; Thu, 9 Sep 2021 10:03:54 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=linuxfoundation.org; s=korg; t=1631181835; bh=aNdNVuOHQN5in236EpLXyDhP5u8tvEK2yD/1lia4+b4=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=m3/QlHVj/fckqSe3t/Fb/99JM1BygLomd5IwSSiFxf/zy5l77jKm6+Dw4WAAPreV3 9LkRkr4HdCB90k0x6zROAejj3E7C3b1KcSV8fr5siMX1UTQg6aL+BmLHIRhHTuqDNv uN8nxT2T2vS2vYuc74BNI1Gn1fFvkIBsUw0tgYhs= Date: Thu, 9 Sep 2021 12:03:52 +0200 From: Greg KH To: Saurav Girepunje Cc: saurav.girepunje@hotmail.com, Larry.Finger@lwfinger.net, phil@philpotter.co.uk, straube.linux@gmail.com, martin@kaiser.cx, nathan@kernel.org, linux-staging@lists.linux.dev, linux-kernel@vger.kernel.org Subject: Re: [PATCH v2] staging: r8188eu: os_dep: simplifiy the rtw_resume function Message-ID: References: <41bd374e-3122-0580-f38d-bb03401730b6@gmail.com> Precedence: bulk X-Mailing-List: linux-staging@lists.linux.dev List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <41bd374e-3122-0580-f38d-bb03401730b6@gmail.com> 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 > > > --- > > > > > > 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