[bug report] compiler warning: fs/f2fs/node.c: In function ‘__set_nat_cache_dirty’: ‘head’ may be used uninitialized

[bug report] compiler warning: fs/f2fs/node.c: In function ‘__set_nat_cache_dirty’: ‘head’ may be used uninitialized

[Ardelean, Alexandru]

Seems to have been introduced via:

----------------------------------------------------------------

commit 780de47cf6cb5f524cd98ec8ffbffc3da5696e17
Author: Chao Yu <yuchao0@huawei.com>
Date:   Tue Mar 20 23:08:30 2018 +0800

    f2fs: don't track new nat entry in nat set
    
    Nat entry set is used only in checkpoint(), and during checkpoint() we
    won't flush new nat entry with unallocated address, so we don't need to
    add new nat entry into nat set, then nat_entry_set::entry_cnt can
    indicate actual entry count we need to flush in checkpoint().
    
    Signed-off-by: Yunlei He <heyunlei@huawei.com>
    Signed-off-by: Chao Yu <yuchao0@huawei.com>
    Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
----------------------------------------------------------------

Compiler warning is:
----------------------------------------------------------------

  CC      fs/f2fs/node.o
In file included from ./include/linux/wait.h:7:0,
                 from ./include/linux/wait_bit.h:8,
                 from ./include/linux/fs.h:6,
                 from fs/f2fs/node.c:11:
fs/f2fs/node.c: In function ‘__set_nat_cache_dirty’:
./include/linux/list.h:63:13: error: ‘head’ may be used uninitialized in
this function [-Werror=maybe-uninitialized]
  next->prev = new;
             ^
fs/f2fs/node.c:238:24: note: ‘head’ was declared here
  struct nat_entry_set *head;
                        ^
cc1: all warnings being treated as errors
----------------------------------------------------------------

Thanks
Alex

Re: [f2fs-dev] [bug report] compiler warning: fs/f2fs/node.c: In function ‘__set_nat_cache_dirty’: ‘head’ may be used uninitialized

[Chao Yu]

On 2019/10/23 22:02, Ardelean, Alexandru wrote:
> Seems to have been introduced via:
> 
> ----------------------------------------------------------------
> 
> commit 780de47cf6cb5f524cd98ec8ffbffc3da5696e17
> Author: Chao Yu <yuchao0@huawei.com>
> Date:   Tue Mar 20 23:08:30 2018 +0800
> 
>     f2fs: don't track new nat entry in nat set
>     
>     Nat entry set is used only in checkpoint(), and during checkpoint() we
>     won't flush new nat entry with unallocated address, so we don't need to
>     add new nat entry into nat set, then nat_entry_set::entry_cnt can
>     indicate actual entry count we need to flush in checkpoint().
>     
>     Signed-off-by: Yunlei He <heyunlei@huawei.com>
>     Signed-off-by: Chao Yu <yuchao0@huawei.com>
>     Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
> ----------------------------------------------------------------
> 
> Compiler warning is:
> ----------------------------------------------------------------
> 
>   CC      fs/f2fs/node.o
> In file included from ./include/linux/wait.h:7:0,
>                  from ./include/linux/wait_bit.h:8,
>                  from ./include/linux/fs.h:6,
>                  from fs/f2fs/node.c:11:
> fs/f2fs/node.c: In function ‘__set_nat_cache_dirty’:
> ./include/linux/list.h:63:13: error: ‘head’ may be used uninitialized in
> this function [-Werror=maybe-uninitialized]
>   next->prev = new;
>              ^
> fs/f2fs/node.c:238:24: note: ‘head’ was declared here
>   struct nat_entry_set *head;

That's not correct, @head will only be assigned and used if new_ne equals NULL.

Thanks,

>                         ^
> cc1: all warnings being treated as errors
> ----------------------------------------------------------------
> 
> Thanks
> Alex
> 
> _______________________________________________
> Linux-f2fs-devel mailing list
> Linux-f2fs-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
> 

Re: [f2fs-dev] [bug report] compiler warning: fs/f2fs/node.c: In function ‘__set_nat_cache_dirty’: ‘head’ may be used uninitialized

[Ardelean, Alexandru]

On Thu, 2019-10-24 at 17:12 +0800, Chao Yu wrote:
> [External]
> 
> On 2019/10/23 22:02, Ardelean, Alexandru wrote:
> > Seems to have been introduced via:
> > 
> > ----------------------------------------------------------------
> > 
> > commit 780de47cf6cb5f524cd98ec8ffbffc3da5696e17
> > Author: Chao Yu <yuchao0@huawei.com>
> > Date:   Tue Mar 20 23:08:30 2018 +0800
> > 
> >     f2fs: don't track new nat entry in nat set
> >     
> >     Nat entry set is used only in checkpoint(), and during checkpoint()
> > we
> >     won't flush new nat entry with unallocated address, so we don't
> > need to
> >     add new nat entry into nat set, then nat_entry_set::entry_cnt can
> >     indicate actual entry count we need to flush in checkpoint().
> >     
> >     Signed-off-by: Yunlei He <heyunlei@huawei.com>
> >     Signed-off-by: Chao Yu <yuchao0@huawei.com>
> >     Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
> > ----------------------------------------------------------------
> > 
> > Compiler warning is:
> > ----------------------------------------------------------------
> > 
> >   CC      fs/f2fs/node.o
> > In file included from ./include/linux/wait.h:7:0,
> >                  from ./include/linux/wait_bit.h:8,
> >                  from ./include/linux/fs.h:6,
> >                  from fs/f2fs/node.c:11:
> > fs/f2fs/node.c: In function ‘__set_nat_cache_dirty’:
> > ./include/linux/list.h:63:13: error: ‘head’ may be used uninitialized
> > in
> > this function [-Werror=maybe-uninitialized]
> >   next->prev = new;
> >              ^
> > fs/f2fs/node.c:238:24: note: ‘head’ was declared here
> >   struct nat_entry_set *head;
> 
> That's not correct, @head will only be assigned and used if new_ne equals
> NULL.

Ack.
I admit that I don't understand the code, and don't claim to understand it.

This may be just a weird compiler issue.
I thought I'd send it just as a heads-up.
I saw this on a Raspberry Pi branch [after we enabled warnings as errors]:
https://travis-ci.org/analogdevicesinc/linux/jobs/601844926#L1208

Looking in the latest f2fs/dev[-test] tree, it looks like the code is
similar as in 4.19.
https://github.com/analogdevicesinc/linux/blob/rpi-4.19.y/fs/f2fs/node.c#L235

Could be that the RPi branch has some more compiler-stuff enabled.

In any case, feel free to disregard this.
We will see how we fix this on our end for that branch specifically.

Thanks
Alex

> 
> Thanks,
> 
> >                         ^
> > cc1: all warnings being treated as errors
> > ----------------------------------------------------------------
> > 
> > Thanks
> > Alex
> > 
> > _______________________________________________
> > Linux-f2fs-devel mailing list
> > Linux-f2fs-devel@lists.sourceforge.net
> > https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
> > 

Re: [f2fs-dev] [bug report] compiler warning: fs/f2fs/node.c: In function ‘__set_nat_cache_dirty’: ‘head’ may be used uninitialized

[Chao Yu]

On 2019/10/24 17:54, Ardelean, Alexandru wrote:
> On Thu, 2019-10-24 at 17:12 +0800, Chao Yu wrote:
>> [External]
>>
>> On 2019/10/23 22:02, Ardelean, Alexandru wrote:
>>> Seems to have been introduced via:
>>>
>>> ----------------------------------------------------------------
>>>
>>> commit 780de47cf6cb5f524cd98ec8ffbffc3da5696e17
>>> Author: Chao Yu <yuchao0@huawei.com>
>>> Date:   Tue Mar 20 23:08:30 2018 +0800
>>>
>>>     f2fs: don't track new nat entry in nat set
>>>     
>>>     Nat entry set is used only in checkpoint(), and during checkpoint()
>>> we
>>>     won't flush new nat entry with unallocated address, so we don't
>>> need to
>>>     add new nat entry into nat set, then nat_entry_set::entry_cnt can
>>>     indicate actual entry count we need to flush in checkpoint().
>>>     
>>>     Signed-off-by: Yunlei He <heyunlei@huawei.com>
>>>     Signed-off-by: Chao Yu <yuchao0@huawei.com>
>>>     Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
>>> ----------------------------------------------------------------
>>>
>>> Compiler warning is:
>>> ----------------------------------------------------------------
>>>
>>>   CC      fs/f2fs/node.o
>>> In file included from ./include/linux/wait.h:7:0,
>>>                  from ./include/linux/wait_bit.h:8,
>>>                  from ./include/linux/fs.h:6,
>>>                  from fs/f2fs/node.c:11:
>>> fs/f2fs/node.c: In function ‘__set_nat_cache_dirty’:
>>> ./include/linux/list.h:63:13: error: ‘head’ may be used uninitialized
>>> in
>>> this function [-Werror=maybe-uninitialized]
>>>   next->prev = new;
>>>              ^
>>> fs/f2fs/node.c:238:24: note: ‘head’ was declared here
>>>   struct nat_entry_set *head;
>>
>> That's not correct, @head will only be assigned and used if new_ne equals
>> NULL.
> 
> Ack.
> I admit that I don't understand the code, and don't claim to understand it.
> 
> This may be just a weird compiler issue.
> I thought I'd send it just as a heads-up.

I think that's the right thing to do.

> I saw this on a Raspberry Pi branch [after we enabled warnings as errors]:
> https://travis-ci.org/analogdevicesinc/linux/jobs/601844926#L1208
> 
> Looking in the latest f2fs/dev[-test] tree, it looks like the code is
> similar as in 4.19.
> https://github.com/analogdevicesinc/linux/blob/rpi-4.19.y/fs/f2fs/node.c#L235
> 
> Could be that the RPi branch has some more compiler-stuff enabled.
> 
> In any case, feel free to disregard this.
> We will see how we fix this on our end for that branch specifically.

That would make sense, let me know if you have any other suspicious compiler
warnings. :)

Thanks,

> 
> Thanks
> Alex
> 
>>
>> Thanks,
>>
>>>                         ^
>>> cc1: all warnings being treated as errors
>>> ----------------------------------------------------------------
>>>
>>> Thanks
>>> Alex
>>>
>>> _______________________________________________
>>> Linux-f2fs-devel mailing list
>>> Linux-f2fs-devel@lists.sourceforge.net
>>> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
>>>