All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dan Carpenter <dan.carpenter@oracle.com>
To: Kangjie Lu <kjlu@umn.edu>
Cc: devel@driverdev.osuosl.org,
	"Florian Schilhabel" <florian.c.schilhabel@googlemail.com>,
	"Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
	linux-kernel@vger.kernel.org, "André Lehmann" <lenneman@web.de>,
	pakki001@umn.edu, "Larry Finger" <Larry.Finger@lwfinger.net>
Subject: Re: [PATCH v2] rtl8712: add a check for the status of register_netdev
Date: Wed, 2 Jan 2019 13:34:21 +0300	[thread overview]
Message-ID: <20190102103421.GG3781@kadam> (raw)
In-Reply-To: <20181226023955.72139-1-kjlu@umn.edu>

On Tue, Dec 25, 2018 at 08:39:53PM -0600, Kangjie Lu wrote:
> register_netdev() may fail, so let's check its return value, and if it
> fails, issue an error message.
> 
> Signed-off-by: Kangjie Lu <kjlu@umn.edu>
> ---
>  drivers/staging/rtl8712/hal_init.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/rtl8712/hal_init.c b/drivers/staging/rtl8712/hal_init.c
> index 7cdd609cab6c..ca0858660a45 100644
> --- a/drivers/staging/rtl8712/hal_init.c
> +++ b/drivers/staging/rtl8712/hal_init.c
> @@ -32,10 +32,10 @@
>  static void rtl871x_load_fw_cb(const struct firmware *firmware, void *context)
>  {
>  	struct _adapter *padapter = context;
> +	struct usb_device *udev = padapter->dvobjpriv.pusbdev;
>  
>  	complete(&padapter->rtl8712_fw_ready);
>  	if (!firmware) {
> -		struct usb_device *udev = padapter->dvobjpriv.pusbdev;
>  		struct usb_interface *pusb_intf = padapter->pusb_intf;
>  
>  		dev_err(&udev->dev, "r8712u: Firmware request failed\n");
> @@ -45,7 +45,8 @@ static void rtl871x_load_fw_cb(const struct firmware *firmware, void *context)
>  	}
>  	padapter->fw = firmware;
>  	/* firmware available - start netdev */
> -	register_netdev(padapter->pnetdev);
> +	if (register_netdev(padapter->pnetdev))
> +		dev_err(&udev->dev, "r8712u: Registering netdev failed\n");

This error handling doesn't seem complete.  You're just adding the
minimum to make the static analysis tool happy.

If you leave the code as-is, maybe someone else will see the static
analysis warning and fix it properly.  But you're silencing the warning
so now no one will fix it.

Warnings are valuable.  Don't silence them without fixing the bug.

regards,
dan carpenter


      reply	other threads:[~2019-01-02 10:34 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-12-26  2:39 [PATCH v2] rtl8712: add a check for the status of register_netdev Kangjie Lu
2019-01-02 10:34 ` Dan Carpenter [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190102103421.GG3781@kadam \
    --to=dan.carpenter@oracle.com \
    --cc=Larry.Finger@lwfinger.net \
    --cc=devel@driverdev.osuosl.org \
    --cc=florian.c.schilhabel@googlemail.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=kjlu@umn.edu \
    --cc=lenneman@web.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=pakki001@umn.edu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.