Re: [PATCH 4/5] btrfs: Use common error handling code in update_ref_path()

2017-08-21 Thread Dan Carpenter
On Mon, Aug 21, 2017 at 09:08:04AM -0400, Jeff Mahoney wrote:
> On 8/21/17 8:41 AM, SF Markus Elfring wrote:
> > From: Markus Elfring 
> > Date: Mon, 21 Aug 2017 13:34:29 +0200
> > 
> > Add a jump target so that a bit of exception handling can be better reused
> > in this function.
> > 
> > This issue was detected by using the Coccinelle software.
> 
> Adding a jump label in the middle of a conditional for "common" error
> handling makes the code more difficult to understand.
> 

I have said that a bunch of times.  It's like bashing my face into the
keyboard for all the good it does.  On the other hand, some people
accept these oddly placed labels...  No one else writes code like this
so far as I know.

regards,
dan carpenter



Re: [PATCH 4/5] btrfs: Use common error handling code in update_ref_path()

2017-08-21 Thread Dan Carpenter
On Mon, Aug 21, 2017 at 09:08:04AM -0400, Jeff Mahoney wrote:
> On 8/21/17 8:41 AM, SF Markus Elfring wrote:
> > From: Markus Elfring 
> > Date: Mon, 21 Aug 2017 13:34:29 +0200
> > 
> > Add a jump target so that a bit of exception handling can be better reused
> > in this function.
> > 
> > This issue was detected by using the Coccinelle software.
> 
> Adding a jump label in the middle of a conditional for "common" error
> handling makes the code more difficult to understand.
> 

I have said that a bunch of times.  It's like bashing my face into the
keyboard for all the good it does.  On the other hand, some people
accept these oddly placed labels...  No one else writes code like this
so far as I know.

regards,
dan carpenter



Re: [PATCH 4/5] btrfs: Use common error handling code in update_ref_path()

2017-08-21 Thread Jeff Mahoney
On 8/21/17 8:41 AM, SF Markus Elfring wrote:
> From: Markus Elfring 
> Date: Mon, 21 Aug 2017 13:34:29 +0200
> 
> Add a jump target so that a bit of exception handling can be better reused
> in this function.
> 
> This issue was detected by using the Coccinelle software.

Adding a jump label in the middle of a conditional for "common" error
handling makes the code more difficult to understand.

-Jeff

> Signed-off-by: Markus Elfring 
> ---
>  fs/btrfs/send.c | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c
> index 59fb1ed6ca20..a96edc91a101 100644
> --- a/fs/btrfs/send.c
> +++ b/fs/btrfs/send.c
> @@ -3697,12 +3697,12 @@ static int update_ref_path(struct send_ctx *sctx, 
> struct recorded_ref *ref)
>   return -ENOMEM;
>  
>   ret = get_cur_path(sctx, ref->dir, ref->dir_gen, new_path);
> - if (ret < 0) {
> - fs_path_free(new_path);
> - return ret;
> - }
> + if (ret < 0)
> + goto free_path;
> +
>   ret = fs_path_add(new_path, ref->name, ref->name_len);
>   if (ret < 0) {
> +free_path:
>   fs_path_free(new_path);
>   return ret;
>   }
> 


-- 
Jeff Mahoney
SUSE Labs



signature.asc
Description: OpenPGP digital signature


Re: [PATCH 4/5] btrfs: Use common error handling code in update_ref_path()

2017-08-21 Thread Jeff Mahoney
On 8/21/17 8:41 AM, SF Markus Elfring wrote:
> From: Markus Elfring 
> Date: Mon, 21 Aug 2017 13:34:29 +0200
> 
> Add a jump target so that a bit of exception handling can be better reused
> in this function.
> 
> This issue was detected by using the Coccinelle software.

Adding a jump label in the middle of a conditional for "common" error
handling makes the code more difficult to understand.

-Jeff

> Signed-off-by: Markus Elfring 
> ---
>  fs/btrfs/send.c | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c
> index 59fb1ed6ca20..a96edc91a101 100644
> --- a/fs/btrfs/send.c
> +++ b/fs/btrfs/send.c
> @@ -3697,12 +3697,12 @@ static int update_ref_path(struct send_ctx *sctx, 
> struct recorded_ref *ref)
>   return -ENOMEM;
>  
>   ret = get_cur_path(sctx, ref->dir, ref->dir_gen, new_path);
> - if (ret < 0) {
> - fs_path_free(new_path);
> - return ret;
> - }
> + if (ret < 0)
> + goto free_path;
> +
>   ret = fs_path_add(new_path, ref->name, ref->name_len);
>   if (ret < 0) {
> +free_path:
>   fs_path_free(new_path);
>   return ret;
>   }
> 


-- 
Jeff Mahoney
SUSE Labs



signature.asc
Description: OpenPGP digital signature


[PATCH 4/5] btrfs: Use common error handling code in update_ref_path()

2017-08-21 Thread SF Markus Elfring
From: Markus Elfring 
Date: Mon, 21 Aug 2017 13:34:29 +0200

Add a jump target so that a bit of exception handling can be better reused
in this function.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring 
---
 fs/btrfs/send.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c
index 59fb1ed6ca20..a96edc91a101 100644
--- a/fs/btrfs/send.c
+++ b/fs/btrfs/send.c
@@ -3697,12 +3697,12 @@ static int update_ref_path(struct send_ctx *sctx, 
struct recorded_ref *ref)
return -ENOMEM;
 
ret = get_cur_path(sctx, ref->dir, ref->dir_gen, new_path);
-   if (ret < 0) {
-   fs_path_free(new_path);
-   return ret;
-   }
+   if (ret < 0)
+   goto free_path;
+
ret = fs_path_add(new_path, ref->name, ref->name_len);
if (ret < 0) {
+free_path:
fs_path_free(new_path);
return ret;
}
-- 
2.14.0



[PATCH 4/5] btrfs: Use common error handling code in update_ref_path()

2017-08-21 Thread SF Markus Elfring
From: Markus Elfring 
Date: Mon, 21 Aug 2017 13:34:29 +0200

Add a jump target so that a bit of exception handling can be better reused
in this function.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring 
---
 fs/btrfs/send.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c
index 59fb1ed6ca20..a96edc91a101 100644
--- a/fs/btrfs/send.c
+++ b/fs/btrfs/send.c
@@ -3697,12 +3697,12 @@ static int update_ref_path(struct send_ctx *sctx, 
struct recorded_ref *ref)
return -ENOMEM;
 
ret = get_cur_path(sctx, ref->dir, ref->dir_gen, new_path);
-   if (ret < 0) {
-   fs_path_free(new_path);
-   return ret;
-   }
+   if (ret < 0)
+   goto free_path;
+
ret = fs_path_add(new_path, ref->name, ref->name_len);
if (ret < 0) {
+free_path:
fs_path_free(new_path);
return ret;
}
-- 
2.14.0