[PATCH v3] fs/btrfs: Fix uninitialized variable
As reported by the Coverity static analysis. The variable zone is not initialized which may causes a failed assertion. Addresses-Coverity: ("Uninitialized variables") Signed-off-by: Khaled ROMDHANI --- v3: catch default as an assertion failure as proposed by David Sterba. --- fs/btrfs/zoned.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/fs/btrfs/zoned.c b/fs/btrfs/zoned.c index 70b23a0d03b1..432509f4b3ac 100644 --- a/fs/btrfs/zoned.c +++ b/fs/btrfs/zoned.c @@ -143,6 +143,9 @@ static inline u32 sb_zone_number(int shift, int mirror) case 0: zone = 0; break; case 1: zone = 1ULL << (BTRFS_SB_LOG_FIRST_SHIFT - shift); break; case 2: zone = 1ULL << (BTRFS_SB_LOG_SECOND_SHIFT - shift); break; + default: + ASSERT(zone); + break; } ASSERT(zone <= U32_MAX); -- 2.17.1
Re: [PATCH v2] fs/btrfs: Fix uninitialized variable
On Tue, Apr 20, 2021 at 01:22:14PM +0300, Dan Carpenter wrote: > On Sat, Apr 17, 2021 at 04:36:16PM +0100, Khaled ROMDHANI wrote: > > As reported by the Coverity static analysis. > > The variable zone is not initialized which > > may causes a failed assertion. > > > > Addresses-Coverity: ("Uninitialized variables") > > Signed-off-by: Khaled ROMDHANI > > --- > > v2: add a default case as proposed by David Sterba > > --- > > fs/btrfs/zoned.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/fs/btrfs/zoned.c b/fs/btrfs/zoned.c > > index eeb3ebe11d7a..82527308d165 100644 > > --- a/fs/btrfs/zoned.c > > +++ b/fs/btrfs/zoned.c > > @@ -143,6 +143,9 @@ static inline u32 sb_zone_number(int shift, int mirror) > > case 0: zone = 0; break; > > case 1: zone = 1ULL << (BTRFS_SB_LOG_FIRST_SHIFT - shift); break; > > case 2: zone = 1ULL << (BTRFS_SB_LOG_SECOND_SHIFT - shift); break; > > It took me a while to spot these break statements. > > > + default: > > + zone = 0; > > + break; > > This break needs to be indented one more tab. > > > } > > > > ASSERT(zone <= U32_MAX); > > regards, > dan carpenter Sorry, but I checked the patch using checkpatch.pl before sending it. Is that blocks some smatch parsing process? In any cases, I will send a V3. Thanks.
Re: [PATCH v2] fs/btrfs: Fix uninitialized variable
On Mon, Apr 19, 2021 at 07:32:25PM +0200, David Sterba wrote: > On Sat, Apr 17, 2021 at 04:36:16PM +0100, Khaled ROMDHANI wrote: > > As reported by the Coverity static analysis. > > The variable zone is not initialized which > > may causes a failed assertion. > > > > Addresses-Coverity: ("Uninitialized variables") > > Signed-off-by: Khaled ROMDHANI > > --- > > v2: add a default case as proposed by David Sterba > > --- > > fs/btrfs/zoned.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/fs/btrfs/zoned.c b/fs/btrfs/zoned.c > > index eeb3ebe11d7a..82527308d165 100644 > > --- a/fs/btrfs/zoned.c > > +++ b/fs/btrfs/zoned.c > > @@ -143,6 +143,9 @@ static inline u32 sb_zone_number(int shift, int mirror) > > case 0: zone = 0; break; > > case 1: zone = 1ULL << (BTRFS_SB_LOG_FIRST_SHIFT - shift); break; > > case 2: zone = 1ULL << (BTRFS_SB_LOG_SECOND_SHIFT - shift); break; > > + default: > > + zone = 0; > > Well yeah but this is not a valid case at all, we'd rather catch that as > an assertion failure than letting is silently continue. So, as all callers pass valid value. It would be better to catch that as an assertion failure.
[PATCH v2] fs/btrfs: Fix uninitialized variable
As reported by the Coverity static analysis. The variable zone is not initialized which may causes a failed assertion. Addresses-Coverity: ("Uninitialized variables") Signed-off-by: Khaled ROMDHANI --- v2: add a default case as proposed by David Sterba --- fs/btrfs/zoned.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/fs/btrfs/zoned.c b/fs/btrfs/zoned.c index eeb3ebe11d7a..82527308d165 100644 --- a/fs/btrfs/zoned.c +++ b/fs/btrfs/zoned.c @@ -143,6 +143,9 @@ static inline u32 sb_zone_number(int shift, int mirror) case 0: zone = 0; break; case 1: zone = 1ULL << (BTRFS_SB_LOG_FIRST_SHIFT - shift); break; case 2: zone = 1ULL << (BTRFS_SB_LOG_SECOND_SHIFT - shift); break; + default: + zone = 0; + break; } ASSERT(zone <= U32_MAX); -- 2.17.1
Re: [PATCH-next] fs/btrfs: Fix uninitialized variable
On Fri, Apr 16, 2021 at 07:32:03PM +0200, David Sterba wrote: > On Tue, Apr 13, 2021 at 02:06:04PM +0100, Khaled ROMDHANI wrote: > > The variable zone is not initialized. It > > may causes a failed assertion. > > Failed assertion means the 2nd one checking that the result still fits > to 32bit type. That would mean that none of the cases were hit, but all > callers pass valid values. > > It would be better to add a default: case to catch that explicitly, > though hitting that is considered 'will not happen'. Yes. I will send a V2.
[PATCH-next] fs/btrfs: Fix uninitialized variable
The variable zone is not initialized. It may causes a failed assertion. Addresses-Coverity: ("Uninitialized variables") Signed-off-by: Khaled ROMDHANI --- fs/btrfs/zoned.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fs/btrfs/zoned.c b/fs/btrfs/zoned.c index eeb3ebe11d7a..ee15ab8dccb5 100644 --- a/fs/btrfs/zoned.c +++ b/fs/btrfs/zoned.c @@ -136,7 +136,7 @@ static int sb_write_pointer(struct block_device *bdev, struct blk_zone *zones, */ static inline u32 sb_zone_number(int shift, int mirror) { - u64 zone; + u64 zone = 0; ASSERT(mirror < BTRFS_SUPER_MIRROR_MAX); switch (mirror) { -- 2.17.1