| Message ID | 5501d33f6ac5af3f371c8734793baeddcde75b4d.1671221596.git.josef@toxicpanda.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Fixup uninitialized warnings and enable extra checks | expand |
On 2022/12/17 04:15, 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> Reviewed-by: Qu Wenruo <wqu@suse.com> Thanks, Qu > --- > 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) {
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); } if (ret) async->status = ret;
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.
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; I've committed version with BLK_STS_IOERR as it's a noisier error, I'm not sure how much the op-not-supported would be noticed by any userspace application if we ever reach this code but this is in the 'cannot happen' realm. Naohiro sent an update to the zoned fix so now we have all the warnings fixed and I'll add the last patch to enable the warning.
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) {
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(-)