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 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
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. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
>>> 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 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
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 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
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. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Jimi Xenidis
2006-Aug-30 11:00 UTC
Re: [XenPPC] Re: [Xen-devel] Re: [patch] make ELF functions static
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 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Gerd Hoffmann
2006-Aug-30 11:12 UTC
Re: [XenPPC] Re: [Xen-devel] Re: [patch] make ELF functions static
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 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Jan Beulich
2006-Aug-30 11:45 UTC
Re: [XenPPC] Re: [Xen-devel] Re: [patch] make ELF functions static
>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 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel