From mboxrd@z Thu Jan 1 00:00:00 1970 From: Christoph Egger Subject: Re: tools/libxl: fix compilation and link errors on NetBSD Date: Wed, 15 May 2013 12:31:26 +0200 Message-ID: <519363FE.9040201@amazon.de> References: <51924F79.4000204@amazon.de> <1368549255.14264.59.camel@zakaz.uk.xensource.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1368549255.14264.59.camel@zakaz.uk.xensource.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Ian Campbell Cc: Stefano Stabellini , Ian Jackson , xen-devel List-Id: xen-devel@lists.xenproject.org On 14.05.13 18:34, Ian Campbell wrote: > On Tue, 2013-05-14 at 15:51 +0100, Christoph Egger wrote: >> commit cad172d7b88bd443c81d865051297875ce2551bc >> Author: Christoph Egger >> Date: Thu Feb 7 14:42:29 2013 +0000 >> >> tools/libxl: fix compilation and link errors on NetBSD >> >> - Fix testidl link error that libyajl is not found >> - Make linking of xl and testidl consistent >> - fix error: array subscript has type 'char' >> >> Signed-off-by: Christoph Egger >> Reviewed-by: Matthew Wilson >> >> diff --git a/tools/Rules.mk b/tools/Rules.mk >> index 3f03a31..4067955 100644 >> --- a/tools/Rules.mk >> +++ b/tools/Rules.mk >> @@ -56,7 +56,7 @@ SHLIB_libblktapctl = >> endif >> >> CFLAGS_libxenlight = -I$(XEN_XENLIGHT) $(CFLAGS_libxenctrl) $(CFLAGS_xeninclude) >> -LDLIBS_libxenlight = $(XEN_XENLIGHT)/libxenlight.so $(SHLIB_libxenctrl) $(SHLIB_libxenstore) $(SHLIB_libblktapctl) >> +LDLIBS_libxenlight = $(XEN_XENLIGHT)/libxenlight.so $(APPEND_LDFLAGS) -lyajl $(SHLIB_libxenctrl) $(SHLIB_libxenstore) $(SHLIB_libblktapctl) > > I don't know what compile/link failure you are seeing on NetBSD but I'm > afraid I don't agree this is a correct fix. I see the following link failure: gcc -pthread -o testidl testidl.o libxlutil.so /home/chegger/xen.git/tools/libxl/../../tools/libxl/libxenlight.so -Wl,-rpath-link=/home/chegger/xen.git/tools/libxl/../../tools/libxc -Wl,-rpath-link=/home/chegger/xen.git/tools/libxl/../../tools/xenstore /home/chegger/xen.git/tools/libxl/../../tools/libxc/libxenctrl.so -L/usr/pkg/lib ld: warning: libyajl.so.2, needed by /home/chegger/xen.git/tools/libxl/../../tools/libxl/libxenlight.so, not found (try using -rpath or -rpath-link) /home/chegger/xen.git/tools/libxl/../../tools/libxl/libxenlight.so: undefined reference to `yajl_parse' /home/chegger/xen.git/tools/libxl/../../tools/libxl/libxenlight.so: undefined reference to `yajl_complete_parse' /home/chegger/xen.git/tools/libxl/../../tools/libxl/libxenlight.so: undefined reference to `yajl_gen_null' /home/chegger/xen.git/tools/libxl/../../tools/libxl/libxenlight.so: undefined reference to `yajl_gen_array_open' /home/chegger/xen.git/tools/libxl/../../tools/libxl/libxenlight.so: undefined reference to `yajl_gen_number' /home/chegger/xen.git/tools/libxl/../../tools/libxl/libxenlight.so: undefined reference to `yajl_gen_string' /home/chegger/xen.git/tools/libxl/../../tools/libxl/libxenlight.so: undefined reference to `yajl_gen_map_close' /home/chegger/xen.git/tools/libxl/../../tools/libxl/libxenlight.so: undefined reference to `yajl_gen_get_buf' /home/chegger/xen.git/tools/libxl/../../tools/libxl/libxenlight.so: undefined reference to `yajl_gen_double' /home/chegger/xen.git/tools/libxl/../../tools/libxl/libxenlight.so: undefined reference to `yajl_gen_config' /home/chegger/xen.git/tools/libxl/../../tools/libxl/libxenlight.so: undefined reference to `yajl_free' /home/chegger/xen.git/tools/libxl/../../tools/libxl/libxenlight.so: undefined reference to `yajl_gen_alloc' /home/chegger/xen.git/tools/libxl/../../tools/libxl/libxenlight.so: undefined reference to `yajl_gen_array_close' /home/chegger/xen.git/tools/libxl/../../tools/libxl/libxenlight.so: undefined reference to `yajl_gen_map_open' /home/chegger/xen.git/tools/libxl/../../tools/libxl/libxenlight.so: undefined reference to `yajl_get_error' /home/chegger/xen.git/tools/libxl/../../tools/libxl/libxenlight.so: undefined reference to `yajl_free_error' /home/chegger/xen.git/tools/libxl/../../tools/libxl/libxenlight.so: undefined reference to `yajl_gen_integer' /home/chegger/xen.git/tools/libxl/../../tools/libxl/libxenlight.so: undefined reference to `yajl_alloc' /home/chegger/xen.git/tools/libxl/../../tools/libxl/libxenlight.so: undefined reference to `yajl_gen_free' /home/chegger/xen.git/tools/libxl/../../tools/libxl/libxenlight.so: undefined reference to `yajl_gen_bool' gmake[3]: *** [testidl] Error 1 gmake[3]: Leaving directory `/home/chegger/xen.git/tools/libxl' gmake[2]: *** [subdir-install-libxl] Error 2 gmake[2]: Leaving directory `/home/chegger/xen.git/tools' gmake[1]: *** [subdirs-install] Error 2 gmake[1]: Leaving directory `/home/chegger/xen.git/tools' gmake: *** [install-tools] Error 2 > > This variable specifies the linker line which an application linking > libxl is required to use. Unless that application is using yajl itself > then it doesn't need -lyajl since the library is correctly linked to the > libyajl itself (via ELF DT_NEEDED). > > If an application does use yajl itself (as xl does) then it should also > link against the library itself, which is what xl does (or did before > you changed it below), but it should not get this from > LDLIBS_libxenlight. > > testidl.c doesn't use yajl directly, so it shouldn't need to link > against libjyajl itself directly AFAICT. The fact that testidl and xl > use different libraries is exactly the reason why their link lines are > "inconsistent". > > With GNU ld this relates the the --as-needed option, perhaps you need to > find similar functionality for your linker? > > Also APPEND_LDFLAGS should be in the final app link only, no need to add > it here. > >> SHLIB_libxenlight = -Wl,-rpath-link=$(XEN_XENLIGHT) >> >> CFLAGS += -D__XEN_TOOLS__ >> diff --git a/tools/libxl/Makefile b/tools/libxl/Makefile >> index cf214bb..c65c11e 100644 >> --- a/tools/libxl/Makefile >> +++ b/tools/libxl/Makefile >> @@ -189,7 +189,7 @@ libxlutil.a: $(LIBXLU_OBJS) >> $(AR) rcs libxlutil.a $^ >> >> xl: $(XL_OBJS) libxlutil.so libxenlight.so >> - $(CC) $(LDFLAGS) -o $@ $(XL_OBJS) libxlutil.so $(LDLIBS_libxenlight) $(LDLIBS_libxenctrl) -lyajl $(APPEND_LDFLAGS) >> + $(CC) $(LDFLAGS) -o $@ $(XL_OBJS) libxlutil.so $(LDLIBS_libxenlight) $(LDLIBS_libxenctrl) $(APPEND_LDFLAGS) > > As explained above you shouldn't be changing this. I suppose it might be > possible that reordering the yajl here is a correct fix for your issue, > but having not seen it I can't really say. This change is just that -lyajl is not redundant here. Adding -lyajl to testidl like to xl w/o my patch doesn't work for me. I see the same error as above. > >> libxl-save-helper: $(SAVE_HELPER_OBJS) libxenlight.so >> $(CC) $(LDFLAGS) -o $@ $(SAVE_HELPER_OBJS) $(LDLIBS_libxenctrl) $(LDLIBS_libxenguest) $(APPEND_LDFLAGS) >> diff --git a/tools/libxl/libxl_utils.c b/tools/libxl/libxl_utils.c >> index 35da71c..0c039b1 100644 >> --- a/tools/libxl/libxl_utils.c >> +++ b/tools/libxl/libxl_utils.c >> @@ -95,7 +95,7 @@ int libxl_domain_qualifier_to_domid(libxl_ctx *ctx, const char *name, >> { >> int i, rv; >> for (i=0; name[i]; i++) { >> - if (!isdigit(name[i])) { >> + if (!isdigit((unigned char)name[i])) { > > You very obviously never even compile tested this. Bah. I need to find a better way to transfer patches from my development machine through the internal review process to my mail client. On my development machine I have the missing 's'. I just checked. Christoph