Re: [PATCH 1/8] btrfs: fix uninit warning in run_one_async_start

[David Sterba <dsterba@suse.cz>]

On Mon, Dec 19, 2022 at 07:51:31AM +0000, Johannes Thumshirn wrote:
> On 16.12.22 21:17, Josef Bacik wrote:
> > With -Wmaybe-uninitialized complains about ret being possibly
> > uninitialized, which isn't possible, however we can init the value to
> > get rid of the warning.
> > 
> > Signed-off-by: Josef Bacik <josef@toxicpanda.com>
> > ---
> >  fs/btrfs/disk-io.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> > index 0888d484df80..c25b444027d6 100644
> > --- a/fs/btrfs/disk-io.c
> > +++ b/fs/btrfs/disk-io.c
> > @@ -693,7 +693,7 @@ int btrfs_validate_metadata_buffer(struct btrfs_bio *bbio,
> >  static void run_one_async_start(struct btrfs_work *work)
> >  {
> >  	struct async_submit_bio *async;
> > -	blk_status_t ret;
> > +	blk_status_t ret = BLK_STS_OK;
> >  
> >  	async = container_of(work, struct  async_submit_bio, work);
> >  	switch (async->submit_cmd) {
> 
> Wouldn't that be more future proof:
> 
> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> index 40f9c99aa44a..6ad5e5c6c858 100644
> --- a/fs/btrfs/disk-io.c
> +++ b/fs/btrfs/disk-io.c
> @@ -707,6 +707,9 @@ static void run_one_async_start(struct btrfs_work *work)
>                 ret = btrfs_submit_bio_start_direct_io(async->inode,
>                                 async->bio, async->dio_file_offset);
>                 break;
> +       default:
> +               ret = BLK_STS_NOTSUPP;
> +               ASSERT(0);

The assert and default: would probably make sense to catch any
accidental misuse of the async API. All the submit_cmd values are from a
fixed set and all are in the source code so any problem should be caught
at development time.

I'm not sure if there should be any fallback for the case where asserts
are complied out and what's the expected outcome of the BLK_STS_NOTSUPP.
This is a case that should never happen.