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

Reply via email to