Btrfs devel - Oct 2012 - [PATCH] Btrfs: fix error processing in btrfs_set_acl()

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(-)

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)
-- 
1.7.12.2

--
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

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

(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