Re: [bug report] btrfs: extent_io: do extra check for extent buffer read write functions

[David Sterba <dsterba@suse.cz>]

On Sat, Sep 16, 2023 at 06:41:12AM +0930, Qu Wenruo wrote:
> 
> 
> On 2023/9/16 04:30, David Sterba wrote:
> > On Fri, Sep 15, 2023 at 10:10:13AM +0300, Dan Carpenter wrote:
> >> Hello Qu Wenruo,
> >>
> >> The patch f98b6215d7d1: "btrfs: extent_io: do extra check for extent
> >> buffer read write functions" from Aug 19, 2020 (linux-next), leads to
> >> the following Smatch static checker warning:
> >>
> >> fs/btrfs/print-tree.c:186 print_uuid_item() error: uninitialized symbol 'subvol_id'.
> >> fs/btrfs/tests/extent-io-tests.c:338 check_eb_bitmap() error: uninitialized symbol 'has'.
> >> fs/btrfs/tests/extent-io-tests.c:353 check_eb_bitmap() error: uninitialized symbol 'has'.
> >> fs/btrfs/uuid-tree.c:203 btrfs_uuid_tree_remove() error: uninitialized symbol 'read_subid'.
> >> fs/btrfs/uuid-tree.c:353 btrfs_uuid_tree_iterate() error: uninitialized symbol 'subid_le'.
> >> fs/btrfs/uuid-tree.c:72 btrfs_uuid_tree_lookup() error: uninitialized symbol 'data'.
> >> fs/btrfs/volumes.c:7415 btrfs_dev_stats_value() error: uninitialized symbol 'val'.
> >>
> >> fs/btrfs/extent_io.c
> >>    4096  void read_extent_buffer(const struct extent_buffer *eb, void *dstv,
> >>    4097                          unsigned long start, unsigned long len)
> >>    4098  {
> >>    4099          void *eb_addr = btrfs_get_eb_addr(eb);
> >>    4100
> >>    4101          if (check_eb_range(eb, start, len))
> >>    4102                  return;
> >>                          ^^^^^^^
> >>
> >> Originally this used to memset dstv to zero but now it just returns.
> >> I can easily just mark that error path as impossible.  These days
> >> everyone with a brain zeros their stack variables anyway so it shouldn't
> >> affect anyone who doesn't deserve it.
> >
> > Thanks, this explains the other errors reported on linux-next with
> > possibly uninitialized variables.
> 
> Mind me to fix those uninitialized variables?
> Or should I just revert to the old behavior?

I'd like to keep the branch in for-next so please fix the warnings. Also
please s/continuous/contiguous/. We can then continue the discussion
regarding the allocator behaviour.