[PATCH] Remove an extra out label in _fcoe_create function

[PATCH] Remove an extra out label in _fcoe_create function

[Milan P. Gandhi]

This patch removes an extra out label in _fcoe_create function
where we return if creation of FCOE interface is failed.

Signed-off-by: Milan P. Gandhi <mgandhi@redhat.com>
---
 drivers/scsi/fcoe/fcoe.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/fcoe/fcoe.c b/drivers/scsi/fcoe/fcoe.c
index 7b960d3..ea21e7b 100644
--- a/drivers/scsi/fcoe/fcoe.c
+++ b/drivers/scsi/fcoe/fcoe.c
@@ -2258,7 +2258,7 @@ static int _fcoe_create(struct net_device *netdev, enum fip_mode fip_mode,
 		fcoe_interface_cleanup(fcoe);
 		mutex_unlock(&fcoe_config_mutex);
 		fcoe_ctlr_device_delete(ctlr_dev);
-		goto out;
+		return rc;
 	}
 
 	/* Make this the "master" N_Port */
@@ -2299,7 +2299,7 @@ static int _fcoe_create(struct net_device *netdev, enum fip_mode fip_mode,
 out_nodev:
 	rtnl_unlock();
 	mutex_unlock(&fcoe_config_mutex);
-out:
+
 	return rc;
 }
 

Re: [PATCH] Remove an extra out label in _fcoe_create function

[Johannes Thumshirn]

On 06/01/2017 02:08 PM, Milan P. Gandhi wrote:
> This patch removes an extra out label in _fcoe_create function
> where we return if creation of FCOE interface is failed.
> 
> Signed-off-by: Milan P. Gandhi <mgandhi@redhat.com>
> ---

Thanks,
Acked-by: Johannes Thumshirn <jth@kernel.org>

-- 
Johannes Thumshirn                                          Storage
jthumshirn@suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

Re: [PATCH] Remove an extra out label in _fcoe_create function

[Martin K. Petersen]

Milan,

> This patch removes an extra out label in _fcoe_create function
> where we return if creation of FCOE interface is failed.

Applied to 4.13/scsi-queue.

-- 
Martin K. Petersen	Oracle Linux Engineering

Re: [PATCH] Remove an extra out label in _fcoe_create function

[Dan Carpenter]

On Thu, Jun 01, 2017 at 05:38:55PM +0530, Milan P. Gandhi wrote:
> This patch removes an extra out label in _fcoe_create function
> where we return if creation of FCOE interface is failed.
> 
> Signed-off-by: Milan P. Gandhi <mgandhi@redhat.com>
> ---
>  drivers/scsi/fcoe/fcoe.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/scsi/fcoe/fcoe.c b/drivers/scsi/fcoe/fcoe.c
> index 7b960d3..ea21e7b 100644
> --- a/drivers/scsi/fcoe/fcoe.c
> +++ b/drivers/scsi/fcoe/fcoe.c
> @@ -2258,7 +2258,7 @@ static int _fcoe_create(struct net_device *netdev, enum fip_mode fip_mode,
>  		fcoe_interface_cleanup(fcoe);
>  		mutex_unlock(&fcoe_config_mutex);
>  		fcoe_ctlr_device_delete(ctlr_dev);
> -		goto out;
> +		return rc;

I don't like do nothing gotos, but I also don't like churn too much.

It doesn't really make sense to do:

		rc = -EIO;
		...
		return rc;

We should probably preserve the error code?  There is a second
"return rc;" later which is more annoying.

drivers/scsi/fcoe/fcoe.c
  2274          /*
  2275           * If the fcoe_ctlr_device is to be set to DISABLED
  2276           * it must be done after the lport is added to the
  2277           * hostlist, but before the rtnl_lock is released.
  2278           * This is because the rtnl_lock protects the
  2279           * hostlist that fcoe_device_notification uses. If
  2280           * the FCoE Controller is intended to be created
  2281           * DISABLED then 'enabled' needs to be considered
  2282           * handling link events. 'enabled' must be set
  2283           * before the lport can be found in the hostlist
  2284           * when a link up event is received.
  2285           */
  2286          if (link_state = FCOE_CREATE_LINK_UP)
  2287                  ctlr_dev->enabled = FCOE_CTLR_ENABLED;
  2288          else
  2289                  ctlr_dev->enabled = FCOE_CTLR_DISABLED;
  2290  
  2291          if (link_state = FCOE_CREATE_LINK_UP &&
  2292              !fcoe_link_ok(lport)) {
  2293                  rtnl_unlock();
  2294                  fcoe_ctlr_link_up(ctlr);
  2295                  mutex_unlock(&fcoe_config_mutex);
  2296                  return rc;
                        ^^^^^^^^^
This is the same as "return 0;" and I guess it's supposed to be a
success return?  But it would look more clear if we changed it to return
a literal instead of rc.

  2297          }
  2298  
  2299  out_nodev:
  2300          rtnl_unlock();

regards,
dan carpenter