Btrfs: fix error processing in btrfs_set_acl()
diff mbox

Message ID 201210050935.AA00014@FM-323941448.jp.fujitsu.com
State New, archived
Headers show

Commit Message

Tsutomu Itoh Oct. 5, 2012, 9:35 a.m. UTC
The error is only returned when kmalloc() fails.
Therefore, to return the error to the caller directly here, I changed the code.

Signed-off-by: Tsutomu Itoh <t-itoh@jp.fujitsu.com>
---
 fs/btrfs/acl.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

Comments

David Sterba Oct. 5, 2012, 10:02 a.m. UTC | #1
Hi,

On Fri, Oct 05, 2012 at 06:35:07PM +0900, Tsutomu Itoh wrote:
> The error is only returned when kmalloc() fails.
> Therefore, to return the error to the caller directly here, I changed the code.

  "fix error processing in btrfs_set_acl()"

what bug does it 'fix' ? IMHO this is a minor cleanup that is not much
needed. The return path after 'posix_acl_to_xattr' uses the if/goto
pattern in the same if (acl) { ... } block, visually this does not help
the original code is correct and "looks ok".
  
> --- a/fs/btrfs/acl.c
> +++ b/fs/btrfs/acl.c
> @@ -136,10 +136,8 @@ static int btrfs_set_acl(struct btrfs_trans_handle *trans,
>  	if (acl) {
>  		size = posix_acl_xattr_size(acl->a_count);
>  		value = kmalloc(size, GFP_NOFS);
> -		if (!value) {
> -			ret = -ENOMEM;
> -			goto out;
> -		}
> +		if (!value)
> +			return -ENOMEM;
>  
>  		ret = posix_acl_to_xattr(acl, value, size);
>  		if (ret < 0)


david
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Tsutomu Itoh Oct. 5, 2012, 4:59 p.m. UTC | #2
(2012/10/05 19:02), David Sterba wrote:
> Hi,
>
> On Fri, Oct 05, 2012 at 06:35:07PM +0900, Tsutomu Itoh wrote:
>> The error is only returned when kmalloc() fails.
>> Therefore, to return the error to the caller directly here, I changed the code.
>
>    "fix error processing in btrfs_set_acl()"
>
> what bug does it 'fix' ? IMHO this is a minor cleanup that is not much

Yes, this is a "cleanup". sorry.

- Tsutomu

> needed. The return path after 'posix_acl_to_xattr' uses the if/goto
> pattern in the same if (acl) { ... } block, visually this does not help
> the original code is correct and "looks ok".
>
>> --- a/fs/btrfs/acl.c
>> +++ b/fs/btrfs/acl.c
>> @@ -136,10 +136,8 @@ static int btrfs_set_acl(struct btrfs_trans_handle *trans,
>>   	if (acl) {
>>   		size = posix_acl_xattr_size(acl->a_count);
>>   		value = kmalloc(size, GFP_NOFS);
>> -		if (!value) {
>> -			ret = -ENOMEM;
>> -			goto out;
>> -		}
>> +		if (!value)
>> +			return -ENOMEM;
>>
>>   		ret = posix_acl_to_xattr(acl, value, size);
>>   		if (ret < 0)
>
>
> david


--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch
diff mbox

diff --git a/fs/btrfs/acl.c b/fs/btrfs/acl.c
index 761e2cd..74ee421 100644
--- a/fs/btrfs/acl.c
+++ b/fs/btrfs/acl.c
@@ -136,10 +136,8 @@  static int btrfs_set_acl(struct btrfs_trans_handle *trans,
 	if (acl) {
 		size = posix_acl_xattr_size(acl->a_count);
 		value = kmalloc(size, GFP_NOFS);
-		if (!value) {
-			ret = -ENOMEM;
-			goto out;
-		}
+		if (!value)
+			return -ENOMEM;
 
 		ret = posix_acl_to_xattr(acl, value, size);
 		if (ret < 0)