Re: compile time warnings

[Ingo Molnar]

* Jesper Juhl <jj@xxxxxxxxxxxxx> wrote:

> On Fri, 2 Jan 2009, Tom Spink wrote:
> 
> > 2009/1/1 Jesper Juhl <jj@xxxxxxxxxxxxx>:
> > > On Thu, 1 Jan 2009, Ingo Brueckl wrote:
> > [snip]
> > 
> > Hi,
> > 
> > > pgd_base is very much used...
> > 
> > It's probably something to do with:
> > 
> > # define permanent_kmaps_init(pgd_base)         do { } while (0)
> > 
> > Which is within the #else part of #if CONFIG_HIGHMEM.  So, if
> > CONFIG_HIGHMEM is not set, permanent_kmaps_init gets wiped out, and
> > therefore that warning will be issued.
> > 
> > Perhaps changing that to an empty inline would remove the warning?
> > 
> Yeah, I noticed that as well after sending the mail.
> Another way to silence the warning (which I think is nicer) would be 
> something like this;
> 
> 
> Silence 'unused variable' warning in arch/x86/mm/init_32.c::pagetable_init
> 
> Signed-off-by: Jesper Juhl <jj@xxxxxxxxxxxxx>
> ---
> 
> diff --git a/arch/x86/mm/init_32.c b/arch/x86/mm/init_32.c
> index 8655b5b..0affa8e 100644
> --- a/arch/x86/mm/init_32.c
> +++ b/arch/x86/mm/init_32.c
> @@ -511,9 +511,7 @@ static void __init early_ioremap_page_table_range_init(pgd_t *pgd_base)
>  
>  static void __init pagetable_init(void)
>  {
> -	pgd_t *pgd_base = swapper_pg_dir;
> -
> -	permanent_kmaps_init(pgd_base);
> +	permanent_kmaps_init((pgd_t *)swapper_pg_dir);
>  }

no, this only works around the warning by 'silencing' it (it also includes 
an ugly type cast) - instead of fixing the core problem.

The core problem is that permanent_kmaps_init() is a CPP macro in the 
!HIGHMEM case - so the right fix would be to convert that to a proper C 
inline function. (same for set_highmem_pages_init() while at it)

Would you mind to send a patch for that that we could push to Linus?

<soapbox>

The highest quality fixes that are motivated by compiler warnings are the 
ones that do not actually 'fix a warning', but instead improve/reshape 
some code so that as a side-effect the warning goes away.

If you ever see a patch that 'silences a warning', it usually shows that 
the deeper problem has not been fully understood. (Except of course if the 
warning shows a genuine bug in the code - but in that case we fix the bug 
and not the warning - the warning was just the canary to it.)

</soapbox>

	Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/