[PATCH 3/6] staging: unisys: visorbus: git rid of gotos in devmajorminor_create_file

Neil Horman nhorman at redhat.com
Wed Mar 9 16:08:30 UTC 2016


On Tue, Mar 08, 2016 at 08:22:45PM -0500, David Kershner wrote:
> The gotos in devmajorminor_create_file aren't needed, get rid of them.
> 
> Signed-off-by: David Kershner <david.kershner at unisys.com>
> Signed-off-by: Timothy Sell <timothy.sell at unisys.com>
> ---
>  drivers/staging/unisys/visorbus/visorbus_main.c | 25 ++++++++-----------------
>  1 file changed, 8 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/staging/unisys/visorbus/visorbus_main.c b/drivers/staging/unisys/visorbus/visorbus_main.c
> index e1ec0b8..37a60ec 100644
> --- a/drivers/staging/unisys/visorbus/visorbus_main.c
> +++ b/drivers/staging/unisys/visorbus/visorbus_main.c
> @@ -306,21 +306,17 @@ devmajorminor_create_file(struct visor_device *dev, const char *name,
>  {
>  	int maxdevnodes = ARRAY_SIZE(dev->devnodes) / sizeof(dev->devnodes[0]);
>  	struct devmajorminor_attribute *myattr = NULL;
> -	int x = -1, rc = 0, slot = -1;
> +	int x = -1, slot = -1;
>  
>  	register_devmajorminor_attributes(dev);
>  	for (slot = 0; slot < maxdevnodes; slot++)
>  		if (!dev->devnodes[slot].attr)
>  			break;
> -	if (slot == maxdevnodes) {
> -		rc = -ENOMEM;
> -		goto away;
> -	}
> +	if (slot == maxdevnodes)
> +		return -ENOMEM;
>  	myattr = kzalloc(sizeof(*myattr), GFP_KERNEL);
> -	if (!myattr) {
> -		rc = -ENOMEM;
> -		goto away;
> -	}
> +	if (!myattr)
> +		return -ENOMEM;
>  	myattr->show = DEVMAJORMINOR_ATTR;
>  	myattr->store = NULL;
>  	myattr->slot = slot;
> @@ -331,17 +327,12 @@ devmajorminor_create_file(struct visor_device *dev, const char *name,
>  	dev->devnodes[slot].minor = minor;
>  	x = sysfs_create_file(&dev->kobjdevmajorminor, &myattr->attr);
>  	if (x < 0) {
> -		rc = x;
> -		goto away;
> -	}
> -	kobject_uevent(&dev->device.kobj, KOBJ_ONLINE);
> -away:
> -	if (rc < 0) {
>  		kfree(myattr);
> -		myattr = NULL;
>  		dev->devnodes[slot].attr = NULL;
> +		return x;
>  	}
> -	return rc;
> +	kobject_uevent(&dev->device.kobj, KOBJ_ONLINE);
> +	return 0;
>  }
>  
>  static void
> -- 
> 1.9.1
> 

This problem wasn't introduced by this patch, but removing the gotos makes it
harder to fix.  The first thing you do is call
register_devmajorminor_attributes, and if the code fails, you should really
unregister those.  What you really need to do is keep the label and gotos, and
add an unregister call at the same place you use the kfree call

Neil



More information about the devel mailing list