Re: [PATCH 4/4] fs: btrfs: initialize @ii in show_dir() to make coverity happy

[Qu Wenruo]

On 2020/11/2 上午7:06, Marek Behun wrote:
> On Sat, 31 Oct 2020 09:07:52 +0800
> Qu Wenruo <w...@suse.com> wrote:
> 
>> In show_dir() if we hit file type FT_CHRDEV or FT_BLKDEV, we expect an
>> BTRFS_INODE_ITEM_KEY, and for that case, we should have @ii filled with
>> data read from disk.
>>
>> We even have ASSERT() for this purpose, but unfortunately coverity can't
>> understand the ASSERT() nor if we get key type BTRFS_INODE_ITEM_KEY,
>> previous if() branch must go to the read_extent_buffer() branch to fill
>> the @ii.
>>
>> So to make coverity happy, just initialize the variable @ii to all zero.
> 
> WTF. If ASSERT excludes this from happening, coverity should understand
> this. We live in 2020. I don't much like the idea to do things just to
> get coverity happy if it can't understand and infer from things like
> ASSERT.
> 
It's not ASSERT(), it's the previous if () branch. The ASSERT() is just to 
ensure we didn't miss anything.

The previous if () looks like this:
if (key.type == BTRFS_ROOT_ITEM_KEY) {
        /* @ii is not touched */
        /* Further more, in this branch type should only be FT_DIR */
} else {
        /* This includes the key.type == INODE_ITEM_KEY case */
        read_extent_buffer( &ii ); /* Here we fill &ii */
}

Then in the offending code we have:
        if (type == BTRFS_FT_CHRDEV || type == BTRFS_FT_BLKDEV) {
                ASSERT(key.type == BTRFS_INODE_ITEM_KEY);
                /*
                 * If the type is above type, we must have key type INODE_ITEM
                 * Thus the ii must be filled.
                 */
                printf("%4llu,%5llu  ", btrfs_stack_inode_rdev(&ii) >> 20,
                                btrfs_stack_inode_rdev(&ii) & 0xfffff);
        } else {
                if (key.type == BTRFS_INODE_ITEM_KEY)
                        printf("%10llu  ", btrfs_stack_inode_size(&ii));
                else
                        printf("%10llu  ", 0ULL);
        }

Thus I really tend to believe it's just a bug in coverity.
All locations accessing @ii all have its key.type checked to ensure it get 
filled in the first place.

Thanks,
Qu