Christoph Egger
2013-May-14 14:51 UTC
tools/libxl: fix compilation and link errors on NetBSD
commit cad172d7b88bd443c81d865051297875ce2551bc
Author: Christoph Egger <chegger@amazon.de>
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 <chegger@amazon.de>
Reviewed-by: Matthew Wilson <msw@amazon.de>
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)
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)
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])) {
goto nondigit_found;
}
}
Ian Campbell
2013-May-14 16:34 UTC
Re: tools/libxl: fix compilation and link errors on NetBSD
On Tue, 2013-05-14 at 15:51 +0100, Christoph Egger wrote:> commit cad172d7b88bd443c81d865051297875ce2551bc > Author: Christoph Egger <chegger@amazon.de> > 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 <chegger@amazon.de> > Reviewed-by: Matthew Wilson <msw@amazon.de> > > 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. 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.> 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. Ian.
Keir Fraser
2013-May-14 16:35 UTC
Re: tools/libxl: fix compilation and link errors on NetBSD
On 14/05/2013 15:51, "Christoph Egger" <chegger@amazon.de> wrote:> commit cad172d7b88bd443c81d865051297875ce2551bc > Author: Christoph Egger <chegger@amazon.de> > 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''The ''unsigned char'' cast is mistyped below. Not very tested! -- Keir> > Signed-off-by: Christoph Egger <chegger@amazon.de> > Reviewed-by: Matthew Wilson <msw@amazon.de> > > 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) > 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) > > 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])) { > goto nondigit_found; > } > } > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel
Christoph Egger
2013-May-15 10:31 UTC
Re: tools/libxl: fix compilation and link errors on NetBSD
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 <chegger@amazon.de> >> 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 <chegger@amazon.de> >> Reviewed-by: Matthew Wilson <msw@amazon.de> >> >> 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
Ian Campbell
2013-May-15 10:46 UTC
Re: tools/libxl: fix compilation and link errors on NetBSD
On Wed, 2013-05-15 at 12:31 +0200, Christoph Egger wrote:> 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)This is exactly as I described, at length, below. The libxenlight library uses yajl and is (or should be) linked against it. testidl does not itself use yajl and should not need to be explicitly linked against it. This failure should not be worked around by needlessly linking yajl into testidl. You likely need to investigate your toolchain to figure out what the correct fix is, which may involve changing how libxenlight.so is linked on your system.> > 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.This change *is* redundant because as I explained you should not be changing LDLIBS_libxenlight.> Adding -lyajl to testidl like to xl w/o my patch doesn''t work for me. I see the > same error as above.Adding -lyajl to the testidl link line would also be wrong, for the same reasons.
Matt Wilson
2013-May-16 08:48 UTC
Re: tools/libxl: fix compilation and link errors on NetBSD
On Wed, May 15, 2013 at 12:31:26PM +0200, Christoph Egger wrote:> > 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.This should be pretty easy if you''re using git repositories and "git send-email". --msw
Reasonably Related Threads
- [PATCH 0 of 2 RESEND] tools: add two new compile flags and perform checks on user defined folders.
- [PATCH] tools/xenbackendd: make 'gmake clean' properly cleaning
- [xen-unstable test] 12988: regressions - FAIL
- tools/misc/xencov.c: build failure on NetBSD
- Bug#812166: [PATCH] x86/mce: fix misleading indentation in init_nonfatal_mce_checker().