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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Sep 21, 2016 at 02:49:06PM +0200, Christian Gromm wrote:
> From: Andrey Shvetsov <andrey.shvetsov@xxxxxx>
> 
> 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@xxxxxx>
> Signed-off-by: Christian Gromm <christian.gromm@xxxxxxxxxxxxx>
> ---
>  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
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel



[Index of Archives]     [Linux Driver Backports]     [DMA Engine]     [Linux GPIO]     [Linux SPI]     [Video for Linux]     [Linux USB Devel]     [Linux Coverity]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux