[PATCH] btrfs: fix initialiser warning in fs/btrfs/tree-checker.c

[PATCH] btrfs: fix initialiser warning in fs/btrfs/tree-checker.c

[Russell King (Oracle)]

Debian gcc 10.2.1 complains thusly:

fs/btrfs/tree-checker.c:1071:9: warning: missing braces
around initializer [-Wmissing-braces]
  struct btrfs_root_item ri = { 0 };
         ^
fs/btrfs/tree-checker.c:1071:9: warning: (near initialization for 'ri.inode') [-Wmissing-braces]

Fix it by removing the unnecessary '0' initialiser, leaving the
braces.

Fixes: 1465af12e254 ("btrfs: tree-checker: fix false alert caused by legacy btrfs root item")
Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
 fs/btrfs/tree-checker.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/btrfs/tree-checker.c b/fs/btrfs/tree-checker.c
index a8b2e0d2c025..1737b62756a6 100644
--- a/fs/btrfs/tree-checker.c
+++ b/fs/btrfs/tree-checker.c
@@ -1068,7 +1068,7 @@ static int check_root_item(struct extent_buffer *leaf, struct btrfs_key *key,
 			   int slot)
 {
 	struct btrfs_fs_info *fs_info = leaf->fs_info;
-	struct btrfs_root_item ri = { 0 };
+	struct btrfs_root_item ri = { };
 	const u64 valid_root_flags = BTRFS_ROOT_SUBVOL_RDONLY |
 				     BTRFS_ROOT_SUBVOL_DEAD;
 	int ret;
-- 
2.30.2

Re: [PATCH] btrfs: fix initialiser warning in fs/btrfs/tree-checker.c

[Qu Wenruo]

On 2021/9/20 06:40, Russell King (Oracle) wrote:
> Debian gcc 10.2.1 complains thusly:
>
> fs/btrfs/tree-checker.c:1071:9: warning: missing braces
> around initializer [-Wmissing-braces]
>    struct btrfs_root_item ri = { 0 };
>           ^
> fs/btrfs/tree-checker.c:1071:9: warning: (near initialization for 'ri.inode') [-Wmissing-braces]
>
> Fix it by removing the unnecessary '0' initialiser, leaving the
> braces.

This should be a compiler bug.

= { 0 }; is completely fine here, in fact = { }; would be more problematic.

What's the compiler version? I haven't hit such problem for GCC 11.1.0
nor clang 12.0.1.

Thanks,
Qu

>
> Fixes: 1465af12e254 ("btrfs: tree-checker: fix false alert caused by legacy btrfs root item")
> Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
> ---
>   fs/btrfs/tree-checker.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/btrfs/tree-checker.c b/fs/btrfs/tree-checker.c
> index a8b2e0d2c025..1737b62756a6 100644
> --- a/fs/btrfs/tree-checker.c
> +++ b/fs/btrfs/tree-checker.c
> @@ -1068,7 +1068,7 @@ static int check_root_item(struct extent_buffer *leaf, struct btrfs_key *key,
>   			   int slot)
>   {
>   	struct btrfs_fs_info *fs_info = leaf->fs_info;
> -	struct btrfs_root_item ri = { 0 };
> +	struct btrfs_root_item ri = { };
>   	const u64 valid_root_flags = BTRFS_ROOT_SUBVOL_RDONLY |
>   				     BTRFS_ROOT_SUBVOL_DEAD;
>   	int ret;
>

Re: [PATCH] btrfs: fix initialiser warning in fs/btrfs/tree-checker.c

[Russell King (Oracle)]

On Mon, Sep 20, 2021 at 06:44:58AM +0800, Qu Wenruo wrote:
> 
> 
> On 2021/9/20 06:40, Russell King (Oracle) wrote:
> > Debian gcc 10.2.1 complains thusly:
> > 
> > fs/btrfs/tree-checker.c:1071:9: warning: missing braces
> > around initializer [-Wmissing-braces]
> >    struct btrfs_root_item ri = { 0 };
> >           ^
> > fs/btrfs/tree-checker.c:1071:9: warning: (near initialization for 'ri.inode') [-Wmissing-braces]
> > 
> > Fix it by removing the unnecessary '0' initialiser, leaving the
> > braces.
> 
> This should be a compiler bug.
> 
> = { 0 }; is completely fine here, in fact = { }; would be more problematic.
> 
> What's the compiler version? I haven't hit such problem for GCC 11.1.0
> nor clang 12.0.1.

I just realised that it's not the compiler I thought it was - It's
GCC 4.9.4 used on 32-bit ARM.

= { }; is not a problem for that version of GCC. Why do you think it's
problematic?

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH] btrfs: fix initialiser warning in fs/btrfs/tree-checker.c

[Russell King (Oracle)]

On Sun, Sep 19, 2021 at 11:46:59PM +0100, Russell King (Oracle) wrote:
> On Mon, Sep 20, 2021 at 06:44:58AM +0800, Qu Wenruo wrote:
> > 
> > 
> > On 2021/9/20 06:40, Russell King (Oracle) wrote:
> > > Debian gcc 10.2.1 complains thusly:
> > > 
> > > fs/btrfs/tree-checker.c:1071:9: warning: missing braces
> > > around initializer [-Wmissing-braces]
> > >    struct btrfs_root_item ri = { 0 };
> > >           ^
> > > fs/btrfs/tree-checker.c:1071:9: warning: (near initialization for 'ri.inode') [-Wmissing-braces]
> > > 
> > > Fix it by removing the unnecessary '0' initialiser, leaving the
> > > braces.
> > 
> > This should be a compiler bug.
> > 
> > = { 0 }; is completely fine here, in fact = { }; would be more problematic.
> > 
> > What's the compiler version? I haven't hit such problem for GCC 11.1.0
> > nor clang 12.0.1.
> 
> I just realised that it's not the compiler I thought it was - It's
> GCC 4.9.4 used on 32-bit ARM.

Okay, ignore this - the warning exists with 5.14, where 4.9.4 is still
a valid compiler, but no longer with 5.15-rc.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH] btrfs: fix initialiser warning in fs/btrfs/tree-checker.c

[Qu Wenruo]

On 2021/9/20 06:46, Russell King (Oracle) wrote:
> On Mon, Sep 20, 2021 at 06:44:58AM +0800, Qu Wenruo wrote:
>>
>>
>> On 2021/9/20 06:40, Russell King (Oracle) wrote:
>>> Debian gcc 10.2.1 complains thusly:
>>>
>>> fs/btrfs/tree-checker.c:1071:9: warning: missing braces
>>> around initializer [-Wmissing-braces]
>>>     struct btrfs_root_item ri = { 0 };
>>>            ^
>>> fs/btrfs/tree-checker.c:1071:9: warning: (near initialization for 'ri.inode') [-Wmissing-braces]
>>>
>>> Fix it by removing the unnecessary '0' initialiser, leaving the
>>> braces.
>>
>> This should be a compiler bug.
>>
>> = { 0 }; is completely fine here, in fact = { }; would be more problematic.
>>
>> What's the compiler version? I haven't hit such problem for GCC 11.1.0
>> nor clang 12.0.1.
> 
> I just realised that it's not the compiler I thought it was - It's
> GCC 4.9.4 used on 32-bit ARM.

That's bare minimal GCC version required for kernel build.

I'm not sure if that old version has all the correct backports.

> 
> = { }; is not a problem for that version of GCC. Why do you think it's
> problematic?
> 

Isn't initialization with empty initializers against C89/90 standard?

Thanks,
Qu

Re: [PATCH] btrfs: fix initialiser warning in fs/btrfs/tree-checker.c

[David Sterba]

On Sun, Sep 19, 2021 at 11:40:39PM +0100, Russell King (Oracle) wrote:
> Debian gcc 10.2.1 complains thusly:
> 
> fs/btrfs/tree-checker.c:1071:9: warning: missing braces
> around initializer [-Wmissing-braces]
>   struct btrfs_root_item ri = { 0 };
>          ^
> fs/btrfs/tree-checker.c:1071:9: warning: (near initialization for 'ri.inode') [-Wmissing-braces]
> 
> Fix it by removing the unnecessary '0' initialiser, leaving the
> braces.
> 
> Fixes: 1465af12e254 ("btrfs: tree-checker: fix false alert caused by legacy btrfs root item")
> Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
> ---
>  fs/btrfs/tree-checker.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/btrfs/tree-checker.c b/fs/btrfs/tree-checker.c
> index a8b2e0d2c025..1737b62756a6 100644
> --- a/fs/btrfs/tree-checker.c
> +++ b/fs/btrfs/tree-checker.c
> @@ -1068,7 +1068,7 @@ static int check_root_item(struct extent_buffer *leaf, struct btrfs_key *key,
>  			   int slot)
>  {
>  	struct btrfs_fs_info *fs_info = leaf->fs_info;
> -	struct btrfs_root_item ri = { 0 };
> +	struct btrfs_root_item ri = { };

Kees Cook does a tree wide unification to { } because there are some
oddities with partial initialization and { 0 } so this will get fixed
eventually. But again the minimum gcc version is now 5.1 so it won't
matter for 4.9 anyway.

Re: [PATCH] btrfs: fix initialiser warning in fs/btrfs/tree-checker.c

[Qu Wenruo]

On 2021/9/20 16:12, David Sterba wrote:
> On Sun, Sep 19, 2021 at 11:40:39PM +0100, Russell King (Oracle) wrote:
>> Debian gcc 10.2.1 complains thusly:
>>
>> fs/btrfs/tree-checker.c:1071:9: warning: missing braces
>> around initializer [-Wmissing-braces]
>>    struct btrfs_root_item ri = { 0 };
>>           ^
>> fs/btrfs/tree-checker.c:1071:9: warning: (near initialization for 'ri.inode') [-Wmissing-braces]
>>
>> Fix it by removing the unnecessary '0' initialiser, leaving the
>> braces.
>>
>> Fixes: 1465af12e254 ("btrfs: tree-checker: fix false alert caused by legacy btrfs root item")
>> Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
>> ---
>>   fs/btrfs/tree-checker.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/fs/btrfs/tree-checker.c b/fs/btrfs/tree-checker.c
>> index a8b2e0d2c025..1737b62756a6 100644
>> --- a/fs/btrfs/tree-checker.c
>> +++ b/fs/btrfs/tree-checker.c
>> @@ -1068,7 +1068,7 @@ static int check_root_item(struct extent_buffer *leaf, struct btrfs_key *key,
>>   			   int slot)
>>   {
>>   	struct btrfs_fs_info *fs_info = leaf->fs_info;
>> -	struct btrfs_root_item ri = { 0 };
>> +	struct btrfs_root_item ri = { };
> 
> Kees Cook does a tree wide unification to { } because there are some
> oddities with partial initialization and { 0 } so this will get fixed
> eventually.

Does this mean in the future only "= { }" is preferred?

It would looks a little weird to me as C89/C90 doesn't allow this.

And I guess checkpatch would also be updated to handle that?

Thanks,
Qu

> But again the minimum gcc version is now 5.1 so it won't
> matter for 4.9 anyway.
> 

Re: [PATCH] btrfs: fix initialiser warning in fs/btrfs/tree-checker.c

[David Sterba]

On Mon, Sep 20, 2021 at 04:25:24PM +0800, Qu Wenruo wrote:
> > Kees Cook does a tree wide unification to { } because there are some
> > oddities with partial initialization and { 0 } so this will get fixed
> > eventually.
> 
> Does this mean in the future only "= { }" is preferred?

Yes,
https://lore.kernel.org/all/20210910225207.3272766-1-keescook@chromium.org/

> It would looks a little weird to me as C89/C90 doesn't allow this.
> 
> And I guess checkpatch would also be updated to handle that?

I think so but we can always override what checkpatch suggests.