netdev - Re: [PATCH v2 net-next 1/5] ionic: simplify returns in devlink info

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20190930162602.3d0ada9f@cakuba.netronome.com>
Date:   Mon, 30 Sep 2019 16:26:02 -0700
From:   Jakub Kicinski <jakub.kicinski@...ronome.com>
To:     Shannon Nelson <snelson@...sando.io>
Cc:     netdev@...r.kernel.org, davem@...emloft.net
Subject: Re: [PATCH v2 net-next 1/5] ionic: simplify returns in devlink info

On Mon, 30 Sep 2019 14:49:16 -0700, Shannon Nelson wrote:
> There is no need for a goto in this bit of code.
> 
> Fixes: fbfb8031533c9 ("ionic: Add hardware init and device commands")

IMHO the fixes tag is disputable here, since this doesn't even generate
a warning.

> Signed-off-by: Shannon Nelson <snelson@...sando.io>
> ---
>  drivers/net/ethernet/pensando/ionic/ionic_devlink.c | 9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/net/ethernet/pensando/ionic/ionic_devlink.c b/drivers/net/ethernet/pensando/ionic/ionic_devlink.c
> index af1647afa4e8..6fb27dcc5787 100644
> --- a/drivers/net/ethernet/pensando/ionic/ionic_devlink.c
> +++ b/drivers/net/ethernet/pensando/ionic/ionic_devlink.c
> @@ -19,31 +19,30 @@ static int ionic_dl_info_get(struct devlink *dl, struct devlink_info_req *req,
>  
>  	err = devlink_info_driver_name_put(req, IONIC_DRV_NAME);
>  	if (err)
> -		goto info_out;
> +		return err;
>  
>  	err = devlink_info_version_running_put(req,
>  					       DEVLINK_INFO_VERSION_GENERIC_FW,
>  					       idev->dev_info.fw_version);
>  	if (err)
> -		goto info_out;
> +		return err;
>  
>  	snprintf(buf, sizeof(buf), "0x%x", idev->dev_info.asic_type);
>  	err = devlink_info_version_fixed_put(req,
>  					     DEVLINK_INFO_VERSION_GENERIC_ASIC_ID,
>  					     buf);
>  	if (err)
> -		goto info_out;
> +		return err;
>  
>  	snprintf(buf, sizeof(buf), "0x%x", idev->dev_info.asic_rev);
>  	err = devlink_info_version_fixed_put(req,
>  					     DEVLINK_INFO_VERSION_GENERIC_ASIC_REV,
>  					     buf);
>  	if (err)
> -		goto info_out;
> +		return err;
>  
>  	err = devlink_info_serial_number_put(req, idev->dev_info.serial_num);
>  
> -info_out:
>  	return err;

Perhaps return the result directly while at it? I'm pretty sure you'll
get a half-automated patch from someone soon if you don't do it ;)

>  }
>