[patch] make ELF functions static

[patch] make ELF functions static

[Hollis Blanchard]

Hi Ian, these functions should be static. It would only be a style issue
except PowerPC actually #includes elf.c twice, to support both 32- and
64-bit ELF binaries. Please apply.


Make xen_elfnote_string() and xen_elfnote_numeric() static.
Signed-off-by: Hollis Blanchard <hollisb@us.ibm.com>

--- a/xen/common/elf.c  Tue Aug 29 06:53:58 2006 -0400
+++ b/xen/common/elf.c  Tue Aug 29 15:52:40 2006 -0500
@@ -145,7 +145,7 @@ static Elf_Note *xen_elfnote_lookup(stru
     return NULL;
 }

-const char *xen_elfnote_string(struct domain_setup_info *dsi, int type)
+static const char *xen_elfnote_string(struct domain_setup_info *dsi, int type)
 {
     Elf_Note *note;

@@ -159,8 +159,8 @@ const char *xen_elfnote_string(struct do
     return (const char *)ELFNOTE_DESC(note);
 }

-unsigned long long xen_elfnote_numeric(struct domain_setup_info *dsi,
-                                       int type, int *defined)
+static unsigned long long xen_elfnote_numeric(struct domain_setup_info *dsi,
+  int type, int *defined)
 {
     Elf_Note *note;
 
--- a/xen/include/xen/elf.h     Tue Aug 29 06:53:58 2006 -0400
+++ b/xen/include/xen/elf.h     Tue Aug 29 16:06:08 2006 -0500
@@ -529,10 +529,6 @@ extern int loadelfimage(struct domain_se
 extern int loadelfimage(struct domain_setup_info *);
 extern int parseelfimage(struct domain_setup_info *);

-extern unsigned long long xen_elfnote_numeric(struct domain_setup_info *dsi,
-                                             int type, int *defined);
-extern const char *xen_elfnote_string(struct domain_setup_info *dsi, int type);-
 #ifdef Elf_Ehdr
 extern int elf_sanity_check(Elf_Ehdr *ehdr);
 #endif



-- 
Hollis Blanchard
IBM Linux Technology Center

Re: [patch] make ELF functions static

[Ian Campbell]

On Tue, 2006-08-29 at 16:15 -0500, Hollis Blanchard wrote:
> Hi Ian, these functions should be static. It would only be a style issue
> except PowerPC actually #includes elf.c twice, to support both 32- and
> 64-bit ELF binaries. Please apply.

Unfortunately they are referenced from outside this file
(xen/arch/x86/domain_build.c). 

I'm not sure what a good short term fix for you would be. Perhaps some
preprocessor/CFLAGS magic to name them xen_elfnote32_foo and
xen_elfnote64_foo when compiling powerpc?

Hopefully long term the 32-on-64 work that is going on will lead to ELF
code which doesn't need to be multiply compiled.

Ian.

Re: [patch] make ELF functions static

[Jan Beulich]

>>> Ian Campbell <Ian.Campbell@XenSource.com> 30.08.06 09:03 >>>
>On Tue, 2006-08-29 at 16:15 -0500, Hollis Blanchard wrote:
>> Hi Ian, these functions should be static. It would only be a style issue
>> except PowerPC actually #includes elf.c twice, to support both 32- and
>> 64-bit ELF binaries. Please apply.
>
>Unfortunately they are referenced from outside this file
>(xen/arch/x86/domain_build.c). 
>
>I'm not sure what a good short term fix for you would be. Perhaps some
>preprocessor/CFLAGS magic to name them xen_elfnote32_foo and
>xen_elfnote64_foo when compiling powerpc?
>
>Hopefully long term the 32-on-64 work that is going on will lead to ELF
>code which doesn't need to be multiply compiled.

Why? It's simpler to compile it twice. I already posted draft patches to
do this, simply introducing an elf32.c that #define-s the relevant
symbols to alternative names and that only gets compiled when 64-bit
arches need it for supporting 32-bit binaries.

Jan

Re: Re: [patch] make ELF functions static

[Gerd Hoffmann]

Jan Beulich wrote:
>> Hopefully long term the 32-on-64 work that is going on will lead to ELF
>> code which doesn't need to be multiply compiled.
> 
> Why? It's simpler to compile it twice. I already posted draft patches to
> do this, simply introducing an elf32.c that #define-s the relevant
> symbols to alternative names and that only gets compiled when 64-bit
> arches need it for supporting 32-bit binaries.

My rewritten libxc domain builder takes a simliar approach ...

cheers,

  Gerd

-- 
Gerd Hoffmann <kraxel@suse.de>
http://www.suse.de/~kraxel/julika-dora.jpeg

Re: Re: [patch] make ELF functions static

[Ian Campbell]

On Wed, 2006-08-30 at 09:38 +0100, Jan Beulich wrote:
> >>> Ian Campbell <Ian.Campbell@XenSource.com> 30.08.06 09:03 >>>
> >On Tue, 2006-08-29 at 16:15 -0500, Hollis Blanchard wrote:
> >> Hi Ian, these functions should be static. It would only be a style issue
> >> except PowerPC actually #includes elf.c twice, to support both 32- and
> >> 64-bit ELF binaries. Please apply.
> >
> >Unfortunately they are referenced from outside this file
> >(xen/arch/x86/domain_build.c). 
> >
> >I'm not sure what a good short term fix for you would be. Perhaps some
> >preprocessor/CFLAGS magic to name them xen_elfnote32_foo and
> >xen_elfnote64_foo when compiling powerpc?
> >
> >Hopefully long term the 32-on-64 work that is going on will lead to ELF
> >code which doesn't need to be multiply compiled.
> 
> Why? It's simpler to compile it twice. I already posted draft patches to
> do this, simply introducing an elf32.c that #define-s the relevant
> symbols to alternative names and that only gets compiled when 64-bit
> arches need it for supporting 32-bit binaries.

Fair enough. I should probably have said something like "solves the
problem cleanly for everyone" rather than speculating about how we would
go about it ;-)

Ian.

Re: [XenPPC] Re: Re: [patch] make ELF functions static

[Jimi Xenidis]

On Aug 30, 2006, at 5:06 AM, Ian Campbell wrote:

> On Wed, 2006-08-30 at 09:38 +0100, Jan Beulich wrote:
>>>>> Ian Campbell <Ian.Campbell@XenSource.com> 30.08.06 09:03 >>>
>>> On Tue, 2006-08-29 at 16:15 -0500, Hollis Blanchard wrote:
>>>> Hi Ian, these functions should be static. It would only be a  
>>>> style issue
>>>> except PowerPC actually #includes elf.c twice, to support both  
>>>> 32- and
>>>> 64-bit ELF binaries. Please apply.
>>>
>>> Unfortunately they are referenced from outside this file
>>> (xen/arch/x86/domain_build.c).
>>>
>>> I'm not sure what a good short term fix for you would be. Perhaps  
>>> some
>>> preprocessor/CFLAGS magic to name them xen_elfnote32_foo and
>>> xen_elfnote64_foo when compiling powerpc?
>>>
>>> Hopefully long term the 32-on-64 work that is going on will lead  
>>> to ELF
>>> code which doesn't need to be multiply compiled.
>>
>> Why? It's simpler to compile it twice. I already posted draft  
>> patches to
>> do this, simply introducing an elf32.c that #define-s the relevant
>> symbols to alternative names and that only gets compiled when 64-bit
>> arches need it for supporting 32-bit binaries.
>
> Fair enough. I should probably have said something like "solves the
> problem cleanly for everyone" rather than speculating about how we  
> would
> go about it ;-)

Ok now that we have established the "compile twice" strategy, then  
why don't we stick them in a new file called elfnote.c and keep them  
global?

Can we do the same for tools/libxc/xc_load_elf.c as well since its on  
my todo list to get that building twice too :)

-JX

Re: [XenPPC] Re: Re: [patch] make ELF functions static

[Gerd Hoffmann]

Jimi Xenidis wrote:

> Can we do the same for tools/libxc/xc_load_elf.c as well since its on my
> todo list to get that building twice too :)

My domain builder rewrite does exactly that ;)

http://www.suse.de/~kraxel/patches/domain-builder-kexec-master-xen-hg11286-quilt/tools-domain-builder.diff

cheers,

  Gerd

-- 
Gerd Hoffmann <kraxel@suse.de>
http://www.suse.de/~kraxel/julika-dora.jpeg

Re: [XenPPC] Re: Re: [patch] make ELF functions static

[Jan Beulich]

>Ok now that we have established the "compile twice" strategy, then  
>why don't we stick them in a new file called elfnote.c and keep them  
>global?

Not validly - despite Elf32_Note and Elf64_Note having identical layout
they're distinct structures, and hence for type correctness they should
be compiled twice (and the correct flavor be used) if we don't want to
risk breaking if there ever get additions to these structures defined
that make them different for 32- and 64-bits.
If otoh we don't care about that, then yes, these functions could easily
go in their own file.

Jan