[PATCH] btrfs: fix a compiler warning of may be used uninitialized

[PATCH] btrfs: fix a compiler warning of may be used uninitialized

[Zhao Lei]

Not real problem, just avoid warning of:
 fs/btrfs/inode-map.c: In function 'btrfs_unpin_free_ino':
 fs/btrfs/inode-map.c:252: warning: 'count' may be used uninitialized in this function
In gcc 4.8.3

Signed-off-by: Zhao Lei <zhaolei@cn.fujitsu.com>
---
 fs/btrfs/inode-map.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/btrfs/inode-map.c b/fs/btrfs/inode-map.c
index d4a582a..e094e3b 100644
--- a/fs/btrfs/inode-map.c
+++ b/fs/btrfs/inode-map.c
@@ -249,7 +249,7 @@ void btrfs_unpin_free_ino(struct btrfs_root *root)
 	spinlock_t *rbroot_lock = &root->free_ino_pinned->tree_lock;
 	struct btrfs_free_space *info;
 	struct rb_node *n;
-	u64 count;
+	u64 count = 0;
 
 	if (!btrfs_test_opt(root, INODE_MAP_CACHE))
 		return;
-- 
1.8.5.1

Re: [PATCH] btrfs: fix a compiler warning of may be used uninitialized

[David Sterba]

On Wed, Sep 02, 2015 at 03:19:59PM +0800, Zhao Lei wrote:
> Not real problem, just avoid warning of:
>  fs/btrfs/inode-map.c: In function 'btrfs_unpin_free_ino':
>  fs/btrfs/inode-map.c:252: warning: 'count' may be used uninitialized in this function
> In gcc 4.8.3
> 
> Signed-off-by: Zhao Lei <zhaolei@cn.fujitsu.com>
> ---
>  fs/btrfs/inode-map.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/btrfs/inode-map.c b/fs/btrfs/inode-map.c
> index d4a582a..e094e3b 100644
> --- a/fs/btrfs/inode-map.c
> +++ b/fs/btrfs/inode-map.c
> @@ -249,7 +249,7 @@ void btrfs_unpin_free_ino(struct btrfs_root *root)
>  	spinlock_t *rbroot_lock = &root->free_ino_pinned->tree_lock;
>  	struct btrfs_free_space *info;
>  	struct rb_node *n;
> -	u64 count;
> +	u64 count = 0;

AFAICS the codepath that would use uninitialized value of count is not
reachable:

		    add_to_ctl = true

270                 if (info->offset > root->ino_cache_progress)
271                         add_to_ctl = false;
272                 else if (info->offset + info->bytes > root->ino_cache_progress)
273                         count = root->ino_cache_progress - info->offset + 1;
274                 else
275                         count = info->bytes;
276
277                 rb_erase(&info->offset_index, rbroot);
278                 spin_unlock(rbroot_lock);
279                 if (add_to_ctl)
280                         __btrfs_add_free_space(ctl, info->offset, count);

count is defined iff add_to_ctl == true, so the patch is not necessary. And I'm
not quite sure that 0 passed down to __btrfs_add_free_space as 'bytes' makes
sense at all.

RE: [PATCH] btrfs: fix a compiler warning of may be used uninitialized

[Zhao Lei]

Hi, David Sterba

Thanks for reviewing.

> -----Original Message-----
> From: David Sterba [mailto:dsterba@suse.cz]
> Sent: Tuesday, September 29, 2015 10:22 PM
> To: Zhao Lei <zhaolei@cn.fujitsu.com>
> Cc: linux-btrfs@vger.kernel.org
> Subject: Re: [PATCH] btrfs: fix a compiler warning of may be used uninitialized
> 
> On Wed, Sep 02, 2015 at 03:19:59PM +0800, Zhao Lei wrote:
> > Not real problem, just avoid warning of:
> >  fs/btrfs/inode-map.c: In function 'btrfs_unpin_free_ino':
> >  fs/btrfs/inode-map.c:252: warning: 'count' may be used uninitialized
> > in this function In gcc 4.8.3
> >
> > Signed-off-by: Zhao Lei <zhaolei@cn.fujitsu.com>
> > ---
> >  fs/btrfs/inode-map.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/fs/btrfs/inode-map.c b/fs/btrfs/inode-map.c index
> > d4a582a..e094e3b 100644
> > --- a/fs/btrfs/inode-map.c
> > +++ b/fs/btrfs/inode-map.c
> > @@ -249,7 +249,7 @@ void btrfs_unpin_free_ino(struct btrfs_root *root)
> >  	spinlock_t *rbroot_lock = &root->free_ino_pinned->tree_lock;
> >  	struct btrfs_free_space *info;
> >  	struct rb_node *n;
> > -	u64 count;
> > +	u64 count = 0;
> 
> AFAICS the codepath that would use uninitialized value of count is not
> reachable:
> 
> 		    add_to_ctl = true
> 
> 270                 if (info->offset > root->ino_cache_progress)
> 271                         add_to_ctl = false;
> 272                 else if (info->offset + info->bytes >
> root->ino_cache_progress)
> 273                         count = root->ino_cache_progress -
> info->offset + 1;
> 274                 else
> 275                         count = info->bytes;
> 276
> 277                 rb_erase(&info->offset_index, rbroot);
> 278                 spin_unlock(rbroot_lock);
> 279                 if (add_to_ctl)
> 280                         __btrfs_add_free_space(ctl, info->offset,
> count);
> 
> count is defined iff add_to_ctl == true, so the patch is not necessary. And I'm
> not quite sure that 0 passed down to __btrfs_add_free_space as 'bytes' makes
> sense at all.

Agree above all.

So I write following description in changelog:
  "Not real problem, just avoid warning of: ..."

It is just to avoid complier warning, no function changed.
A warning in compiler output is not pretty:)

Thanks
Zhaolei

Re: [PATCH] btrfs: fix a compiler warning of may be used uninitialized

[Holger Hoffstätte]

On 09/30/15 05:55, Zhao Lei wrote:
>> count is defined iff add_to_ctl == true, so the patch is not necessary. And I'm
>> not quite sure that 0 passed down to __btrfs_add_free_space as 'bytes' makes
>> sense at all.
> 
> Agree above all.
> 
> So I write following description in changelog:
>   "Not real problem, just avoid warning of: ..."
> 
> It is just to avoid complier warning, no function changed.
> A warning in compiler output is not pretty:)

This looks more like a false-positive with gcc 4.8.3.
With 5.2:

..
  CC [M]  fs/btrfs/file-item.o
  CC [M]  fs/btrfs/inode-item.o
  CC [M]  fs/btrfs/inode-map.o
  CC [M]  fs/btrfs/disk-io.o
  CC [M]  fs/btrfs/transaction.o
..

No warning, as expected.

-h

Re: [PATCH] btrfs: fix a compiler warning of may be used uninitialized

[David Sterba]

On Wed, Sep 30, 2015 at 11:55:13AM +0800, Zhao Lei wrote:
> > AFAICS the codepath that would use uninitialized value of count is not
> > reachable:
> > 
> > 		    add_to_ctl = true
> > 
> > 270                 if (info->offset > root->ino_cache_progress)
> > 271                         add_to_ctl = false;
> > 272                 else if (info->offset + info->bytes >
> > root->ino_cache_progress)
> > 273                         count = root->ino_cache_progress -
> > info->offset + 1;
> > 274                 else
> > 275                         count = info->bytes;
> > 276
> > 277                 rb_erase(&info->offset_index, rbroot);
> > 278                 spin_unlock(rbroot_lock);
> > 279                 if (add_to_ctl)
> > 280                         __btrfs_add_free_space(ctl, info->offset,
> > count);
> > 
> > count is defined iff add_to_ctl == true, so the patch is not necessary. And I'm
> > not quite sure that 0 passed down to __btrfs_add_free_space as 'bytes' makes
> > sense at all.
> 
> Agree above all.
> 
> So I write following description in changelog:
>   "Not real problem, just avoid warning of: ..."
> 
> It is just to avoid complier warning, no function changed.
> A warning in compiler output is not pretty:)

And the compiler is wrong in this case, the code is fine as is. I'd say
go fix you compiler and the output will be pretty :) No really, this
kind of fixes brings false sense of "fixing something in the code".