[PATCH 0/2] btrfs: Fix returned errno codes

[PATCH 0/2] btrfs: Fix returned errno codes

[Luis de Bethencourt]

From: Luis de Bethencourt <luis@debethencourt.com>

Hi,

These two patches fix instances where -1 is used to specify a buffer
allocation fail, instead of using -ENOMEM.

I could merge the two patches into one if that's more appropriate.

Thanks,
Luis

Luis de Bethencourt (2):
  btrfs: check-integrity: Fix returned errno codes
  btrfs: reada: Fix returned errno code

 fs/btrfs/check-integrity.c | 4 ++--
 fs/btrfs/reada.c           | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

-- 
2.5.3

[PATCH 1/2] btrfs: check-integrity: Fix returned errno codes

[Luis de Bethencourt]

check-integrity is using -1 instead of the -ENOMEM defined macro to
specify that a buffer allocation failed. Since the error number is
propagated, the caller will get a -EPERM which is the wrong error
condition.

Also, the smatch tool complains with the following warnings:
btrfsic_process_superblock() warn: returning -1 instead of -ENOMEM is sloppy
btrfsic_read_block() warn: returning -1 instead of -ENOMEM is sloppy

Signed-off-by: Luis de Bethencourt <luisbg@osg.samsung.com>
---
 fs/btrfs/check-integrity.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/btrfs/check-integrity.c b/fs/btrfs/check-integrity.c
index ce7dec8..a6257e1 100644
--- a/fs/btrfs/check-integrity.c
+++ b/fs/btrfs/check-integrity.c
@@ -667,7 +667,7 @@ static int btrfsic_process_superblock(struct btrfsic_state *state,
 	selected_super = kzalloc(sizeof(*selected_super), GFP_NOFS);
 	if (NULL == selected_super) {
 		printk(KERN_INFO "btrfsic: error, kmalloc failed!\n");
-		return -1;
+		return -ENOMEM;
 	}
 
 	list_for_each_entry(device, dev_head, dev_list) {
@@ -1660,7 +1660,7 @@ static int btrfsic_read_block(struct btrfsic_state *state,
 					  sizeof(*block_ctx->pagev)) *
 					 num_pages, GFP_NOFS);
 	if (!block_ctx->mem_to_free)
-		return -1;
+		return -ENOMEM;
 	block_ctx->datav = block_ctx->mem_to_free;
 	block_ctx->pagev = (struct page **)(block_ctx->datav + num_pages);
 	for (i = 0; i < num_pages; i++) {
-- 
2.5.3

Re: [PATCH 1/2] btrfs: check-integrity: Fix returned errno codes

[David Sterba]

On Tue, Sep 22, 2015 at 05:29:38PM +0100, Luis de Bethencourt wrote:
> check-integrity is using -1 instead of the -ENOMEM defined macro to
> specify that a buffer allocation failed. Since the error number is
> propagated, the caller will get a -EPERM which is the wrong error
> condition.

Agreed. btrfsic_process_superblock can be called from the mount path so
getting EPERM would be confusing.

> Also, the smatch tool complains with the following warnings:
> btrfsic_process_superblock() warn: returning -1 instead of -ENOMEM is sloppy
> btrfsic_read_block() warn: returning -1 instead of -ENOMEM is sloppy
> 
> Signed-off-by: Luis de Bethencourt <luisbg@osg.samsung.com>

Reviewed-by: David Sterba <dsterba@suse.com>

[PATCH 2/2] btrfs: reada: Fix returned errno code

[Luis de Bethencourt]

reada is using -1 instead of the -ENOMEM defined macro to specify that
a buffer allocation failed. Since the error number is propagated, the
caller will get a -EPERM which is the wrong error condition.

Smatch tool warning:
reada_add_block() warn: returning -1 instead of -ENOMEM is sloppy

Signed-off-by: Luis de Bethencourt <luisbg@osg.samsung.com>
---
 fs/btrfs/reada.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/btrfs/reada.c b/fs/btrfs/reada.c
index 4645cd1..5bfd3cd 100644
--- a/fs/btrfs/reada.c
+++ b/fs/btrfs/reada.c
@@ -569,7 +569,7 @@ static int reada_add_block(struct reada_control *rc, u64 logical,
 	rec = kzalloc(sizeof(*rec), GFP_NOFS);
 	if (!rec) {
 		reada_extent_put(root->fs_info, re);
-		return -1;
+		return -ENOMEM;
 	}
 
 	rec->rc = rc;
-- 
2.5.3

Re: [PATCH 2/2] btrfs: reada: Fix returned errno code

[David Sterba]

On Tue, Sep 22, 2015 at 05:29:39PM +0100, Luis de Bethencourt wrote:
> reada is using -1 instead of the -ENOMEM defined macro to specify that
> a buffer allocation failed. Since the error number is propagated, the
> caller will get a -EPERM which is the wrong error condition.
> 
> Smatch tool warning:
> reada_add_block() warn: returning -1 instead of -ENOMEM is sloppy
> 
> Signed-off-by: Luis de Bethencourt <luisbg@osg.samsung.com>
> ---
>  fs/btrfs/reada.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/btrfs/reada.c b/fs/btrfs/reada.c
> index 4645cd1..5bfd3cd 100644
> --- a/fs/btrfs/reada.c
> +++ b/fs/btrfs/reada.c
> @@ -569,7 +569,7 @@ static int reada_add_block(struct reada_control *rc, u64 logical,
>  	rec = kzalloc(sizeof(*rec), GFP_NOFS);
>  	if (!rec) {
>  		reada_extent_put(root->fs_info, re);
> -		return -1;
> +		return -ENOMEM;

When called from btrfs_reada_add, the return value is ignored and
overriden as ENOMEM. Please also update the caller to return the exact
value from reada_add_block.

Re: [PATCH 0/2] btrfs: Fix returned errno codes

[David Sterba]

On Tue, Sep 22, 2015 at 05:29:37PM +0100, Luis de Bethencourt wrote:
> These two patches fix instances where -1 is used to specify a buffer
> allocation fail, instead of using -ENOMEM.
> 
> I could merge the two patches into one if that's more appropriate.

No, it's better to change them separately. If you change the return
value, also look at all the callers so you can see the impact of the
change and count that into the "size of the patch".