[PATCH 2/6] staging: most: aim-cdev: remove labels

Greg KH gregkh at linuxfoundation.org
Thu Sep 22 07:04:17 UTC 2016


On Wed, Sep 21, 2016 at 02:49:06PM +0200, Christian Gromm wrote:
> From: Andrey Shvetsov <andrey.shvetsov at k2l.de>
> 
> This patch removes the labels 'put_mbo' and 'unlock' and relocates the
> code accordingly making the code look simpler.

But this is the opposite of what you should normally do.  Now you have
to keep track of what needs to be unlocked within each error section.

> 
> Signed-off-by: Andrey Shvetsov <andrey.shvetsov at k2l.de>
> Signed-off-by: Christian Gromm <christian.gromm at microchip.com>
> ---
>  drivers/staging/most/aim-cdev/cdev.c | 15 +++++----------
>  1 file changed, 5 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/staging/most/aim-cdev/cdev.c b/drivers/staging/most/aim-cdev/cdev.c
> index 5458fb9..f6a7b75 100644
> --- a/drivers/staging/most/aim-cdev/cdev.c
> +++ b/drivers/staging/most/aim-cdev/cdev.c
> @@ -183,7 +183,6 @@ static int aim_close(struct inode *inode, struct file *filp)
>  static ssize_t aim_write(struct file *filp, const char __user *buf,
>  			 size_t count, loff_t *offset)
>  {
> -	int ret;
>  	size_t actual_len;
>  	size_t max_len;
>  	struct mbo *mbo = NULL;
> @@ -201,8 +200,8 @@ static ssize_t aim_write(struct file *filp, const char __user *buf,
>  	}
>  
>  	if (unlikely(!c->dev)) {
> -		ret = -ENODEV;
> -		goto unlock;
> +		mutex_unlock(&c->io_mutex);
> +		return -ENODEV;
>  	}
>  
>  	max_len = c->cfg->buffer_size;
> @@ -210,18 +209,14 @@ static ssize_t aim_write(struct file *filp, const char __user *buf,
>  	mbo->buffer_length = actual_len;
>  
>  	if (copy_from_user(mbo->virt_address, buf, mbo->buffer_length)) {
> -		ret = -EFAULT;
> -		goto put_mbo;
> +		most_put_mbo(mbo);
> +		mutex_unlock(&c->io_mutex);
> +		return -EFAULT;
>  	}
>  
>  	most_submit_mbo(mbo);
>  	mutex_unlock(&c->io_mutex);
>  	return actual_len;
> -put_mbo:
> -	most_put_mbo(mbo);
> -unlock:
> -	mutex_unlock(&c->io_mutex);
> -	return ret;

The code is "simpler" as-is, as it follows the normal good kernel coding
style.  I don't understand why you are changing this...

greg k-h


More information about the devel mailing list