From mboxrd@z Thu Jan  1 00:00:00 1970
From: Qu Wenruo <wqu@suse.com>
Date: Mon, 2 Nov 2020 08:27:16 +0800
Subject: [PATCH 4/4] fs: btrfs: initialize @ii in show_dir() to make
 coverity happy
In-Reply-To: <20201102000656.241101c5@nic.cz>
References: <20201031010752.23974-1-wqu@suse.com>
 <20201031010752.23974-5-wqu@suse.com> <20201102000656.241101c5@nic.cz>
Message-ID: <2e5530d6-e5e9-8c4b-686c-5c39b8339c86@suse.com>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de



On 2020/11/2 ??7:06, Marek Behun wrote:
> On Sat, 31 Oct 2020 09:07:52 +0800
> Qu Wenruo <wqu@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