On Sat, May 01, 2021 at 11:50:46PM +0100, Khaled ROMDHANI wrote: > Fix the warning: variable 'zone' is used > uninitialized whenever '?:' condition is true. > > Fix that by preventing the code to reach > the last assertion. If the variable 'mirror' > is invalid, the assertion fails and we return > immediately. > > Reported-by: kernel test robot <lkp@xxxxxxxxx> > Signed-off-by: Khaled ROMDHANI <khaledromdhani216@xxxxxxxxx> This took several rounds and none of them was close to what I'd consider a proper fix, for something that's not really important. As Dan said, smatch does understand the values passed from the callers and the function is a static inline so the complete information is available. No tricky analysis is required, so why does not coverity see that too? We use assertions to namely catch programmer errors and API misuse, anything that can happen at runtime or depends on input needs proper checks and error handling. But for the super block copies, the constant won't change so all we want is to catch the stupid errors. > --- > 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 8250ab3f0868..23da9d8dc184 100644 > --- a/fs/btrfs/zoned.c > +++ b/fs/btrfs/zoned.c > @@ -145,7 +145,7 @@ static inline u32 sb_zone_number(int shift, int mirror) > case 2: zone = 1ULL << (BTRFS_SB_LOG_SECOND_SHIFT - shift); break; > default: > ASSERT((u32)mirror < 3); > - break; > + return 0; It's been pointed out that this does not apply on the current code but on top of previous versions, so it's not making it easy for me to apply the patch and do maybe some tweaks only. I don't mind merging trivial patches, people can learn the process and few iterations are not a big deal. What I also hope for is to get some understanding of the code being changed and not just silencing some tools' warnings.
