diff mbox series

ext4: Fix unused iterator variable warnings

Message ID 20230420-ext4-unused-variables-super-c-v1-1-138b6db6c21c@kernel.org
State Awaiting Upstream
Headers show
Series ext4: Fix unused iterator variable warnings | expand

Commit Message

Nathan Chancellor April 20, 2023, 4:51 p.m. UTC 
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 <nathan@kernel.org>
---
 fs/ext4/super.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)


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

Best regards,

Comments

Jason Yan April 21, 2023, 1:52 a.m. UTC | #1 
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<nathan@kernel.org>
> ---
>   fs/ext4/super.c | 7 +++----
>   1 file changed, 3 insertions(+), 4 deletions(-)

Thanks for the fix:

Reviewed-by: Jason Yan <yanaijie@huawei.com>
Jan Kara April 24, 2023, 4:25 p.m. UTC | #2 
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 <nathan@kernel.org>

Looks good. Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								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 <nathan@kernel.org>
>
Geert Uytterhoeven April 27, 2023, 12:36 p.m. UTC | #3 
On Thu, Apr 20, 2023 at 6:56 PM Nathan Chancellor <nathan@kernel.org> 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 <nathan@kernel.org>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

> --- 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?

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
Nathan Chancellor April 27, 2023, 6:29 p.m. UTC | #4 
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 <nathan@kernel.org> 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 <nathan@kernel.org>
> 
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

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/20230421070815.2260326-1-arnd@kernel.org/

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/7ca8f790-c14e-6449-f3b5-4214d3fb1e61@googlemail.com/

Cheers,
Nathan
Theodore Ts'o April 28, 2023, 4 a.m. UTC | #5 
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
Theodore Ts'o April 28, 2023, 5:21 a.m. UTC | #6 
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,
Geert Uytterhoeven April 28, 2023, 7:45 a.m. UTC | #7 
Hi Nathan,

On Thu, Apr 27, 2023 at 8:30 PM Nathan Chancellor <nathan@kernel.org> wrote:
> On Thu, Apr 27, 2023 at 02:36:10PM +0200, Geert Uytterhoeven wrote:
> > On Thu, Apr 20, 2023 at 6:56 PM Nathan Chancellor <nathan@kernel.org> 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 <nathan@kernel.org>

> > > --- 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/20230421070815.2260326-1-arnd@kernel.org/
>
> 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/7ca8f790-c14e-6449-f3b5-4214d3fb1e61@googlemail.com/

I definitely don't want to delay fixing this, we already have too many
fixes (and the first ones arrived before the opening of the merge window).

Gr{oetje,eeting}s,

                        Geert
Theodore Ts'o April 30, 2023, 5:59 p.m. UTC | #8 
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: 856dd6c5981260b4d1aa84b78373ad54a203db48

Best regards,
diff mbox series

Patch

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);