Re: [RFC/PATCH] m68k: fix compiler warning by properly inlining flat_set_persistent()

[Greg Ungerer <gerg@snapgear.com>]

Hi Ezequiel,

On 24/04/12 08:43, Ezequiel García wrote:
> On Mon, Apr 23, 2012 at 6:07 PM, Geert Uytterhoeven
> <geert@linux-m68k.org>  wrote:
>> On Mon, Apr 23, 2012 at 15:50, Ezequiel Garcia<elezegarcia@gmail.com>  wrote:
>>> Signed-off-by: Ezequiel Garcia<elezegarcia@gmail.com>

Normally this signed-off-by goes after your patch description
(Documentation/SubmittingPatches section 12).


>>> ---
>>> To define or to inline that is the question:
>>> The current definition of flat_set_persistent produces a compiler
>>> warning; arch/sh/ does it in a different way defining it to
>>> a macro that uses persistent var. IMHO, an inline is easier to read.
>>
>> What's the compiler warning?
>> It seems several other nommu arches use the same definition for
>> flat_set_persistent()?
>>
>
> Here's the warning:
>    fs/binfmt_flat.c: In function æload_flat_fileÆ:
>    fs/binfmt_flat.c:752: warning: unused variable æpersistentÆ

I like to see the actual compiler messages in the git message
description as well.


> Yes, every arch except sh is using the same definition.
> My first thought was to extend the arch/sh definition:
>
>    #define flat_set_persistent(relval, p)          ({ (void)p; 0; })
>
> but in a conversation in the janitors list I was told that inlining
> to a return-only function is a common pattern.
> Plus, if you compare fs/built-in.o there is no extra code generated:
>
> $ size fs-built-in-flat-inline
>     text	   data	    bss	    dec	    hex	filename
>   233332	   1908	   1640	 236880	  39d50	built-in-flat-inline
> $ size fs/built-in.o
>     text	   data	    bss	    dec	    hex	filename
>   233332	   1908	   1640	 236880	  39d50	fs/built-in.o
>
> FWIW, this is my compiler, I built it using gentoo's crossdev tool.
>    gcc version 4.4.5 (Gentoo 4.4.5 p1.3, pie-0.4.5)
>
> So, what's your opinion?

I see the same warning on my gcc-4.5.1 based toolchain as well.
No surprise really given none of the m68k macros use the persistent arg.

If you want to move that signed-off-by and put the warning text in
the message description I will carry this in the m68knommu git tree.

Thanks
Greg


------------------------------------------------------------------------
Greg Ungerer  --  Principal Engineer        EMAIL:     gerg@snapgear.com
SnapGear Group, McAfee                      PHONE:       +61 7 3435 2888
8 Gardner Close                             FAX:         +61 7 3217 5323
Milton, QLD, 4064, Australia                WEB: http://www.SnapGear.com