2023-04-20 16:55:50

by Nathan Chancellor

[permalink] [raw]
Subject: [PATCH] ext4: Fix unused iterator variable warnings

When CONFIG_QUOTA is disabled, there are warnings around unused iterator
variables:

fs/ext4/super.c: In function 'ext4_put_super':
fs/ext4/super.c:1262:13: error: unused variable 'i' [-Werror=unused-variable]
1262 | int i, err;
| ^
fs/ext4/super.c: In function '__ext4_fill_super':
fs/ext4/super.c:5200:22: error: unused variable 'i' [-Werror=unused-variable]
5200 | unsigned int i;
| ^
cc1: all warnings being treated as errors

The kernel has updated to gnu11, allowing the variables to be declared
within the for loop. Do so to clear up the warnings.

Fixes: dcbf87589d90 ("ext4: factor out ext4_flex_groups_free()")
Signed-off-by: Nathan Chancellor <[email protected]>
---
fs/ext4/super.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 403cc0e6cd65..f16492b8c98d 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -1259,7 +1259,7 @@ static void ext4_put_super(struct super_block *sb)
struct ext4_sb_info *sbi = EXT4_SB(sb);
struct ext4_super_block *es = sbi->s_es;
int aborted = 0;
- int i, err;
+ int err;

/*
* Unregister sysfs before destroying jbd2 journal.
@@ -1311,7 +1311,7 @@ static void ext4_put_super(struct super_block *sb)
ext4_flex_groups_free(sbi);
ext4_percpu_param_destroy(sbi);
#ifdef CONFIG_QUOTA
- for (i = 0; i < EXT4_MAXQUOTAS; i++)
+ for (int i = 0; i < EXT4_MAXQUOTAS; i++)
kfree(get_qf_name(sb, sbi, i));
#endif

@@ -5197,7 +5197,6 @@ static int __ext4_fill_super(struct fs_context *fc, struct super_block *sb)
ext4_fsblk_t logical_sb_block;
struct inode *root;
int ret = -ENOMEM;
- unsigned int i;
int needs_recovery;
int err = 0;
ext4_group_t first_not_zeroed;
@@ -5628,7 +5627,7 @@ static int __ext4_fill_super(struct fs_context *fc, struct super_block *sb)
#endif

#ifdef CONFIG_QUOTA
- for (i = 0; i < EXT4_MAXQUOTAS; i++)
+ for (unsigned int i = 0; i < EXT4_MAXQUOTAS; i++)
kfree(get_qf_name(sb, sbi, i));
#endif
fscrypt_free_dummy_policy(&sbi->s_dummy_enc_policy);

---
base-commit: 519fe1bae7e20fc4e7f179d50b6102b49980e85d
change-id: 20230420-ext4-unused-variables-super-c-cabda558d931

Best regards,
--
Nathan Chancellor <[email protected]>


2023-04-21 01:58:47

by Jason Yan

[permalink] [raw]
Subject: Re: [PATCH] ext4: Fix unused iterator variable warnings

On 2023/4/21 0:51, Nathan Chancellor wrote:
> When CONFIG_QUOTA is disabled, there are warnings around unused iterator
> variables:
>
> fs/ext4/super.c: In function 'ext4_put_super':
> fs/ext4/super.c:1262:13: error: unused variable 'i' [-Werror=unused-variable]
> 1262 | int i, err;
> | ^
> fs/ext4/super.c: In function '__ext4_fill_super':
> fs/ext4/super.c:5200:22: error: unused variable 'i' [-Werror=unused-variable]
> 5200 | unsigned int i;
> | ^
> cc1: all warnings being treated as errors
>
> The kernel has updated to gnu11, allowing the variables to be declared
> within the for loop. Do so to clear up the warnings.
>
> Fixes: dcbf87589d90 ("ext4: factor out ext4_flex_groups_free()")
> Signed-off-by: Nathan Chancellor<[email protected]>
> ---
> fs/ext4/super.c | 7 +++----
> 1 file changed, 3 insertions(+), 4 deletions(-)

Thanks for the fix:

Reviewed-by: Jason Yan <[email protected]>

2023-04-24 16:39:10

by Jan Kara

[permalink] [raw]
Subject: Re: [PATCH] ext4: Fix unused iterator variable warnings

On Thu 20-04-23 09:51:24, Nathan Chancellor wrote:
> When CONFIG_QUOTA is disabled, there are warnings around unused iterator
> variables:
>
> fs/ext4/super.c: In function 'ext4_put_super':
> fs/ext4/super.c:1262:13: error: unused variable 'i' [-Werror=unused-variable]
> 1262 | int i, err;
> | ^
> fs/ext4/super.c: In function '__ext4_fill_super':
> fs/ext4/super.c:5200:22: error: unused variable 'i' [-Werror=unused-variable]
> 5200 | unsigned int i;
> | ^
> cc1: all warnings being treated as errors
>
> The kernel has updated to gnu11, allowing the variables to be declared
> within the for loop. Do so to clear up the warnings.
>
> Fixes: dcbf87589d90 ("ext4: factor out ext4_flex_groups_free()")
> Signed-off-by: Nathan Chancellor <[email protected]>

Looks good. Feel free to add:

Reviewed-by: Jan Kara <[email protected]>

Honza

> ---
> fs/ext4/super.c | 7 +++----
> 1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index 403cc0e6cd65..f16492b8c98d 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -1259,7 +1259,7 @@ static void ext4_put_super(struct super_block *sb)
> struct ext4_sb_info *sbi = EXT4_SB(sb);
> struct ext4_super_block *es = sbi->s_es;
> int aborted = 0;
> - int i, err;
> + int err;
>
> /*
> * Unregister sysfs before destroying jbd2 journal.
> @@ -1311,7 +1311,7 @@ static void ext4_put_super(struct super_block *sb)
> ext4_flex_groups_free(sbi);
> ext4_percpu_param_destroy(sbi);
> #ifdef CONFIG_QUOTA
> - for (i = 0; i < EXT4_MAXQUOTAS; i++)
> + for (int i = 0; i < EXT4_MAXQUOTAS; i++)
> kfree(get_qf_name(sb, sbi, i));
> #endif
>
> @@ -5197,7 +5197,6 @@ static int __ext4_fill_super(struct fs_context *fc, struct super_block *sb)
> ext4_fsblk_t logical_sb_block;
> struct inode *root;
> int ret = -ENOMEM;
> - unsigned int i;
> int needs_recovery;
> int err = 0;
> ext4_group_t first_not_zeroed;
> @@ -5628,7 +5627,7 @@ static int __ext4_fill_super(struct fs_context *fc, struct super_block *sb)
> #endif
>
> #ifdef CONFIG_QUOTA
> - for (i = 0; i < EXT4_MAXQUOTAS; i++)
> + for (unsigned int i = 0; i < EXT4_MAXQUOTAS; i++)
> kfree(get_qf_name(sb, sbi, i));
> #endif
> fscrypt_free_dummy_policy(&sbi->s_dummy_enc_policy);
>
> ---
> base-commit: 519fe1bae7e20fc4e7f179d50b6102b49980e85d
> change-id: 20230420-ext4-unused-variables-super-c-cabda558d931
>
> Best regards,
> --
> Nathan Chancellor <[email protected]>
>
--
Jan Kara <[email protected]>
SUSE Labs, CR

2023-04-27 18:31:07

by Nathan Chancellor

[permalink] [raw]
Subject: Re: [PATCH] ext4: Fix unused iterator variable warnings

Hi Geert,

On Thu, Apr 27, 2023 at 02:36:10PM +0200, Geert Uytterhoeven wrote:
> On Thu, Apr 20, 2023 at 6:56 PM Nathan Chancellor <[email protected]> wrote:
> > When CONFIG_QUOTA is disabled, there are warnings around unused iterator
> > variables:
> >
> > fs/ext4/super.c: In function 'ext4_put_super':
> > fs/ext4/super.c:1262:13: error: unused variable 'i' [-Werror=unused-variable]
> > 1262 | int i, err;
> > | ^
> > fs/ext4/super.c: In function '__ext4_fill_super':
> > fs/ext4/super.c:5200:22: error: unused variable 'i' [-Werror=unused-variable]
> > 5200 | unsigned int i;
> > | ^
> > cc1: all warnings being treated as errors
> >
> > The kernel has updated to gnu11, allowing the variables to be declared
> > within the for loop. Do so to clear up the warnings.
> >
> > Fixes: dcbf87589d90 ("ext4: factor out ext4_flex_groups_free()")
> > Signed-off-by: Nathan Chancellor <[email protected]>
>
> Reviewed-by: Geert Uytterhoeven <[email protected]>

Thank you for the review!

> > --- a/fs/ext4/super.c
> > +++ b/fs/ext4/super.c
>
> > @@ -1311,7 +1311,7 @@ static void ext4_put_super(struct super_block *sb)
> > ext4_flex_groups_free(sbi);
> > ext4_percpu_param_destroy(sbi);
> > #ifdef CONFIG_QUOTA
> > - for (i = 0; i < EXT4_MAXQUOTAS; i++)
> > + for (int i = 0; i < EXT4_MAXQUOTAS; i++)
>
> int
>
> > kfree(get_qf_name(sb, sbi, i));
> > #endif
> >
>
> > @@ -5628,7 +5627,7 @@ static int __ext4_fill_super(struct fs_context *fc, struct super_block *sb)
> > #endif
> >
> > #ifdef CONFIG_QUOTA
> > - for (i = 0; i < EXT4_MAXQUOTAS; i++)
> > + for (unsigned int i = 0; i < EXT4_MAXQUOTAS; i++)
>
> unsigned int
>
> > kfree(get_qf_name(sb, sbi, i));
> > #endif
> > fscrypt_free_dummy_policy(&sbi->s_dummy_enc_policy);
>
> I do see an opportunity to make this more consistent.
> get_qf_name() takes an int for the last parameter, but that should probably
> become unsigned int?

Yes, or I could have just changed the type of this variable to 'int', as
Arnd did in his version; I just chose to keep the existing type so this
was basically a "no functional change" patch.

https://lore.kernel.org/[email protected]/

I do not think it fundamentally matters, EXT4_MAXQUOTAS is defined as 3
so I do not think unsigned versus signed semantics matter much here :) I
can make that change in a v2 or separate change or we can just take
Arnd's patch, but this is now in mainline and there is another patch
trying to fix this warning so it would be good to get this dealt with
sooner rather than later...

https://lore.kernel.org/[email protected]/

Cheers,
Nathan

2023-04-28 04:06:09

by Theodore Ts'o

[permalink] [raw]
Subject: Re: [PATCH] ext4: Fix unused iterator variable warnings

On Thu, Apr 20, 2023 at 09:51:24AM -0700, Nathan Chancellor wrote:
>
> The kernel has updated to gnu11, allowing the variables to be declared
> within the for loop. Do so to clear up the warnings.

This is OK for this patch, since it's fixing a patch that landed
during this merge window, and it's unlikely this cause problems with
any future security patches that will need to be backported into 5.15,
5.10, etc. --- or result in patch conflicts that will be any worse
than what we already have.

However, for anything that might be something that needs to be
backported into LTS kernels, or be in code that might get be involved
with a LTS backport patch, let's not use any C11/GNU11 whenever
possible.

- Ted

2023-04-28 05:34:34

by Theodore Ts'o

[permalink] [raw]
Subject: Re: [PATCH] ext4: Fix unused iterator variable warnings


On Thu, 20 Apr 2023 09:51:24 -0700, Nathan Chancellor wrote:
> When CONFIG_QUOTA is disabled, there are warnings around unused iterator
> variables:
>
> fs/ext4/super.c: In function 'ext4_put_super':
> fs/ext4/super.c:1262:13: error: unused variable 'i' [-Werror=unused-variable]
> 1262 | int i, err;
> | ^
> fs/ext4/super.c: In function '__ext4_fill_super':
> fs/ext4/super.c:5200:22: error: unused variable 'i' [-Werror=unused-variable]
> 5200 | unsigned int i;
> | ^
> cc1: all warnings being treated as errors
>
> [...]

Applied, thanks!

[1/1] ext4: Fix unused iterator variable warnings
commit: 5b3e67cadf469110068175fb1e088d9bebaa4d7c

Best regards,
--
Theodore Ts'o <[email protected]>