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