btrfs: Fix returned errno codes
cover btrfs: Fix returned errno codes
From: Luis de Bethencourt <[email protected]>
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(-)
Signature · 1 line
2.5.3
PATCH 1/2 btrfs: check-integrity: Fix returned errno codes
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
--- fs/btrfs/check-integrity.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
fs/btrfs/check-integrity.c +2 -2
@@ -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++) {
Signature · 1 line
2.5.3
Re: [PATCH 1/2] btrfs: check-integrity: Fix returned errno codes
Quoted 4 lines · Luis de Bethencourt
check-integrity is using -1 instead of the
-ENOMEMdefined macro to specify that a buffer allocation failed. Since the error number is propagated, the caller will get a-EPERMwhich is the wrong error condition.
Agreed. btrfsic_process_superblock can be called from the mount path so getting EPERM would be confusing.
Quoted 5 lines
Also, the smatch tool complains with the following warnings:
btrfsic_process_superblock()warn: returning -1 instead of-ENOMEMis sloppybtrfsic_read_block()warn: returning -1 instead of-ENOMEMis sloppySigned-off-by Luis de Bethencourt <[email protected]>
PATCH 2/2 btrfs: reada: Fix returned errno code
In reply to Luis de Bethencourtreada 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
--- fs/btrfs/reada.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
fs/btrfs/reada.c +1 -1
@@ -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;
Signature · 1 line
2.5.3
Re: [PATCH 2/2] btrfs: reada: Fix returned errno code
Quoted 17 lines · Luis de Bethencourt
reada is using -1 instead of the
-ENOMEMdefined macro to specify that a buffer allocation failed. Since the error number is propagated, the caller will get a-EPERMwhich is the wrong error condition.Smatch tool warning:
reada_add_block()warn: returning -1 instead of-ENOMEMis sloppySigned-off-by Luis de Bethencourt <[email protected]>--- fs/btrfs/reada.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)fs/btrfs/reada.c +0
-0
fs/btrfs/reada.c static int reada_add_block(struct reada_control *rc, u64 logical, · line 572
Quoted 5 lines
fs/btrfs/reada.c +1
-1@@ -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.
Quoted 4 lines · Luis de Bethencourt
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".