[PATCH 1/1] btrfs: scrub: Fix use of uninitialized variable

[PATCH 1/1] btrfs: scrub: Fix use of uninitialized variable

['Guanjun']

From: Guanjun <guanjun@linux.alibaba.com>

'ret' will be uninitialized in case that the logical_length
is 0. Even if the caller has already ensured that logical_length
is greater than 0, we still need to fix this issue. Due to the
compiler may complain like this:

fs/btrfs/scrub.c: In function ‘scrub_simple_mirror.constprop’:
fs/btrfs/scrub.c:2123:9: error: ‘ret’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
 2123 |  return ret;
      |         ^~~

Fixes: 09022b14fafc (btrfs: scrub: introduce dedicated helper to scrub simple-mirror based range)
Signed-off-by: Guanjun <guanjun@linux.alibaba.com>
---
 fs/btrfs/scrub.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/btrfs/scrub.c b/fs/btrfs/scrub.c
index a01807cbd4d4..13024131f77d 100644
--- a/fs/btrfs/scrub.c
+++ b/fs/btrfs/scrub.c
@@ -2071,7 +2071,7 @@ static int scrub_simple_mirror(struct scrub_ctx *sctx,
 	struct btrfs_fs_info *fs_info = sctx->fs_info;
 	const u64 logical_end = logical_start + logical_length;
 	u64 cur_logical = logical_start;
-	int ret;
+	int ret = 0;
 
 	/* The range must be inside the bg */
 	ASSERT(logical_start >= bg->start && logical_end <= bg->start + bg->length);
-- 
2.39.3

Re: [PATCH 1/1] btrfs: scrub: Fix use of uninitialized variable

[Qu Wenruo]

[-- Attachment #1.1.1: Type: text/plain, Size: 1991 bytes --]



On 2023/12/9 18:51, 'Guanjun' wrote:
> From: Guanjun <guanjun@linux.alibaba.com>
> 
> 'ret' will be uninitialized in case that the logical_length
> is 0. Even if the caller has already ensured that logical_length
> is greater than 0, we still need to fix this issue. Due to the
> compiler may complain like this:
> 
> fs/btrfs/scrub.c: In function ‘scrub_simple_mirror.constprop’:
> fs/btrfs/scrub.c:2123:9: error: ‘ret’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
>   2123 |  return ret;
>        |         ^~~
> 

Compiler version and config please.

You know btrfs has enabled -Wmaybe-uninitialized already and all 
warnings would be treated as error.
This means if this is really valid, tons of testers would already hit it.

Thus I'm wondering if it's some internal out-of-date toolchain on your side.


Furthermore, if you really want to fix the problem, I strongly 
discourage from blindly setting the @ret to 0.

But change all the break calls of the loop to return directly, so that 
the final return out of the loop can always return 0, so that @ret can 
be defined inside the loop, and be much safer.

Thanks,
Qu

> Fixes: 09022b14fafc (btrfs: scrub: introduce dedicated helper to scrub simple-mirror based range)
> Signed-off-by: Guanjun <guanjun@linux.alibaba.com>
> ---
>   fs/btrfs/scrub.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/btrfs/scrub.c b/fs/btrfs/scrub.c
> index a01807cbd4d4..13024131f77d 100644
> --- a/fs/btrfs/scrub.c
> +++ b/fs/btrfs/scrub.c
> @@ -2071,7 +2071,7 @@ static int scrub_simple_mirror(struct scrub_ctx *sctx,
>   	struct btrfs_fs_info *fs_info = sctx->fs_info;
>   	const u64 logical_end = logical_start + logical_length;
>   	u64 cur_logical = logical_start;
> -	int ret;
> +	int ret = 0;
>   
>   	/* The range must be inside the bg */
>   	ASSERT(logical_start >= bg->start && logical_end <= bg->start + bg->length);

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 7027 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]