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

[Greg KH]

On Wed, Sep 21, 2016 at 02:49:06PM +0200, Christian Gromm wrote:
> From: Andrey Shvetsov 
> 
> 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 
> Signed-off-by: Christian Gromm 
> ---
>  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(>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(>io_mutex);
> + return -EFAULT;
>   }
>  
>   most_submit_mbo(mbo);
>   mutex_unlock(>io_mutex);
>   return actual_len;
> -put_mbo:
> - most_put_mbo(mbo);
> -unlock:
> - mutex_unlock(>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

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

[Christian Gromm]

From: Andrey Shvetsov 

This patch removes the labels 'put_mbo' and 'unlock' and relocates the
code accordingly making the code look simpler.

Signed-off-by: Andrey Shvetsov 
Signed-off-by: Christian Gromm 
---
 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(>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(>io_mutex);
+   return -EFAULT;
}
 
most_submit_mbo(mbo);
mutex_unlock(>io_mutex);
return actual_len;
-put_mbo:
-   most_put_mbo(mbo);
-unlock:
-   mutex_unlock(>io_mutex);
-   return ret;
 }
 
 /**
-- 
1.9.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel