Re: [PATCH] ext4: Fix unused iterator variable warnings

[Geert Uytterhoeven]

Hi Nathan,

On Thu, Apr 27, 2023 at 8:30 PM Nathan Chancellor <nathan@xxxxxxxxxx> 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@xxxxxxxxxx> 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@xxxxxxxxxx>

> > > --- 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@xxxxxxxxxx/
>
> 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@xxxxxxxxxxxxxx/

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

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx

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