Skip to contentSkip to conversation

build: various fixes for building with uclibc and libiconv

PATCH Superseded Roger Pau Monne 4 participants 10 messages Dec 20, 2011 – Dec 22, 2011

v2 of 3 Superseded Latest is v3 →

lore
Share & tools
lore
#0 lore raw

PATCH v2 1/1 build: various fixes for building with uclibc and libiconv

This patch contains various fixes to the build system to allow building xen under uclibc with libiconv. Has been tested with uclibc 0.9.32, gcc 4.6.2 and libiconv 1.12, from Alpine Linux 2.3.2.

Since the series is completely different from the previous one, I don't think it's worth to add a "Changes since v1" section, becasue everything is different.

Please review, Roger.

#1 lore raw

PATCH v2 1/1 blktap: remove local definitions and include byteswap.h

# HG changeset patch
# User Roger Pau Monne <[email protected]> # Date 1324344433 -3600
# Node ID 29f5b0afe971335eea4f547efed00b132ade5c64 # Parent d23344a87ad7d3d3acae83171586358b081e3f4e
blktap: remove local definitions and include byteswap.h

Use the same approach as tools/blktap2/include/libvhd.h, remove local definitions of bswap* and include byteswap.h. Also remove the HAVE_BYTESWAP_H ifdef, since it was not defined in this context (it's defined by QEMU).

tools/blktap/drivers/bswap.h +0 -36
diff -r d23344a87ad7 -r 29f5b0afe971 tools/blktap/drivers/bswap.h
@@ -15,43 +15,7 @@
 #define bswap_64(x) swap64(x)
 #else
 
-#ifdef HAVE_BYTESWAP_H
 #include <byteswap.h>
-#else
-
-#define bswap_16(x) \
-({ \
-	uint16_t __x = (x); \
-	((uint16_t)( \
-		(((uint16_t)(__x) & (uint16_t)0x00ffU) << 8) | \
-		(((uint16_t)(__x) & (uint16_t)0xff00U) >> 8) )); \
-})
-
-#define bswap_32(x) \
-({ \
-	uint32_t __x = (x); \
-	((uint32_t)( \
-		(((uint32_t)(__x) & (uint32_t)0x000000ffUL) << 24) | \
-		(((uint32_t)(__x) & (uint32_t)0x0000ff00UL) <<  8) | \
-		(((uint32_t)(__x) & (uint32_t)0x00ff0000UL) >>  8) | \
-		(((uint32_t)(__x) & (uint32_t)0xff000000UL) >> 24) )); \
-})
-
-#define bswap_64(x) \
-({ \
-	uint64_t __x = (x); \
-	((uint64_t)( \
-		(uint64_t)(((uint64_t)(__x) & (uint64_t)0x00000000000000ffULL) << 56) | \
-		(uint64_t)(((uint64_t)(__x) & (uint64_t)0x000000000000ff00ULL) << 40) | \
-		(uint64_t)(((uint64_t)(__x) & (uint64_t)0x0000000000ff0000ULL) << 24) | \
-		(uint64_t)(((uint64_t)(__x) & (uint64_t)0x00000000ff000000ULL) <<  8) | \
-	        (uint64_t)(((uint64_t)(__x) & (uint64_t)0x000000ff00000000ULL) >>  8) | \
-		(uint64_t)(((uint64_t)(__x) & (uint64_t)0x0000ff0000000000ULL) >> 24) | \
-		(uint64_t)(((uint64_t)(__x) & (uint64_t)0x00ff000000000000ULL) >> 40) | \
-		(uint64_t)(((uint64_t)(__x) & (uint64_t)0xff00000000000000ULL) >> 56) )); \
-})
-
-#endif /* !HAVE_BYTESWAP_H */
 
 static inline uint16_t bswap16(uint16_t x)
 {
#2 lore raw

Re: [PATCH 1 of 4 v2] blktap: remove local definitions and include byteswap.h

Quoted 9 lines · Roger Pau Monne

# HG changeset patch
# User Roger Pau Monne <[email protected]> # Date 1324344433 -3600
# Node ID 29f5b0afe971335eea4f547efed00b132ade5c64 # Parent d23344a87ad7d3d3acae83171586358b081e3f4e
blktap: remove local definitions and include byteswap.h

Use the same approach as tools/blktap2/include/libvhd.h, remove local definitions of bswap* and include byteswap.h.

The end result of this patch isn't quite the same as libvhd.h. There it does:

        #if defined(__linux__)
        #include <endian.h>
        #include <byteswap.h>
        #elif defined(__NetBSD__)
        #include <sys/endian.h>
        #include <sys/bswap.h>
        #endif

while here we end up with:

        #if defined(__NetBSD__)
        #include <sys/endian.h>
        #include <sys/types.h>
        #elif defined(__OpenBSD__)
        #include <machine/endian.h>
        #define bswap_16(x) swap16(x)
        #define bswap_32(x) swap32(x)
        #define bswap_64(x) swap64(x)
        #else
        #include <byteswap.h>
        #endif

I think that last #else should be an explicit #elif defined(__linux__) for consistency and so that future porters don't wander around looking for byteswap.h before realising they need a new #elif clause.

Quoted 58 lines

Also remove the
HAVE_BYTESWAP_H ifdef, since it was not defined in this context (it's defined by QEMU).

diff -r d23344a87ad7 -r 29f5b0afe971 tools/blktap/drivers/bswap.h

tools/blktap/drivers/bswap.h +0 -36
@@ -15,43 +15,7 @@
 #define bswap_64(x) swap64(x)
 #else
 
-#ifdef HAVE_BYTESWAP_H
 #include <byteswap.h>
-#else
-
-#define bswap_16(x) \
-({ \
-	uint16_t __x = (x); \
-	((uint16_t)( \
-		(((uint16_t)(__x) & (uint16_t)0x00ffU) << 8) | \
-		(((uint16_t)(__x) & (uint16_t)0xff00U) >> 8) )); \
-})
-
-#define bswap_32(x) \
-({ \
-	uint32_t __x = (x); \
-	((uint32_t)( \
-		(((uint32_t)(__x) & (uint32_t)0x000000ffUL) << 24) | \
-		(((uint32_t)(__x) & (uint32_t)0x0000ff00UL) <<  8) | \
-		(((uint32_t)(__x) & (uint32_t)0x00ff0000UL) >>  8) | \
-		(((uint32_t)(__x) & (uint32_t)0xff000000UL) >> 24) )); \
-})
-
-#define bswap_64(x) \
-({ \
-	uint64_t __x = (x); \
-	((uint64_t)( \
-		(uint64_t)(((uint64_t)(__x) & (uint64_t)0x00000000000000ffULL) << 56) | \
-		(uint64_t)(((uint64_t)(__x) & (uint64_t)0x000000000000ff00ULL) << 40) | \
-		(uint64_t)(((uint64_t)(__x) & (uint64_t)0x0000000000ff0000ULL) << 24) | \
-		(uint64_t)(((uint64_t)(__x) & (uint64_t)0x00000000ff000000ULL) <<  8) | \
-	        (uint64_t)(((uint64_t)(__x) & (uint64_t)0x000000ff00000000ULL) >>  8) | \
-		(uint64_t)(((uint64_t)(__x) & (uint64_t)0x0000ff0000000000ULL) >> 24) | \
-		(uint64_t)(((uint64_t)(__x) & (uint64_t)0x00ff000000000000ULL) >> 40) | \
-		(uint64_t)(((uint64_t)(__x) & (uint64_t)0xff00000000000000ULL) >> 56) )); \
-})
-
-#endif /* !HAVE_BYTESWAP_H */
 
 static inline uint16_t bswap16(uint16_t x)
 {

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel

#3 lore raw

PATCH v2 1/1 blktap2: remove local definitions and include byteswap.h

In reply to Roger Pau Monne

# HG changeset patch
# User Roger Pau Monne <[email protected]> # Date 1324344514 -3600
# Node ID 23e1abbd203ca2619103f3e95c6b1ff36be60e38 # Parent 29f5b0afe971335eea4f547efed00b132ade5c64
blktap2: remove local definitions and include byteswap.h

Use the same approach as tools/blktap2/include/libvhd.h, remove local definitions of bswap* and include byteswap.h. Also remove the HAVE_BYTESWAP_H ifdef, since it was not defined in this context (it's defined by QEMU).

tools/blktap2/drivers/bswap.h +1 -36
diff -r 29f5b0afe971 -r 23e1abbd203c tools/blktap2/drivers/bswap.h
@@ -15,43 +15,8 @@
 #define bswap_64(x) swap64(x)
 #else
 
-#ifdef HAVE_BYTESWAP_H
+#include <endian.h>
 #include <byteswap.h>
-#else
-
-#define bswap_16(x) \
-({ \
-	uint16_t __x = (x); \
-	((uint16_t)( \
-		(((uint16_t)(__x) & (uint16_t)0x00ffU) << 8) | \
-		(((uint16_t)(__x) & (uint16_t)0xff00U) >> 8) )); \
-})
-
-#define bswap_32(x) \
-({ \
-	uint32_t __x = (x); \
-	((uint32_t)( \
-		(((uint32_t)(__x) & (uint32_t)0x000000ffUL) << 24) | \
-		(((uint32_t)(__x) & (uint32_t)0x0000ff00UL) <<  8) | \
-		(((uint32_t)(__x) & (uint32_t)0x00ff0000UL) >>  8) | \
-		(((uint32_t)(__x) & (uint32_t)0xff000000UL) >> 24) )); \
-})
-
-#define bswap_64(x) \
-({ \
-	uint64_t __x = (x); \
-	((uint64_t)( \
-		(uint64_t)(((uint64_t)(__x) & (uint64_t)0x00000000000000ffULL) << 56) | \
-		(uint64_t)(((uint64_t)(__x) & (uint64_t)0x000000000000ff00ULL) << 40) | \
-		(uint64_t)(((uint64_t)(__x) & (uint64_t)0x0000000000ff0000ULL) << 24) | \
-		(uint64_t)(((uint64_t)(__x) & (uint64_t)0x00000000ff000000ULL) <<  8) | \
-	        (uint64_t)(((uint64_t)(__x) & (uint64_t)0x000000ff00000000ULL) >>  8) | \
-		(uint64_t)(((uint64_t)(__x) & (uint64_t)0x0000ff0000000000ULL) >> 24) | \
-		(uint64_t)(((uint64_t)(__x) & (uint64_t)0x00ff000000000000ULL) >> 40) | \
-		(uint64_t)(((uint64_t)(__x) & (uint64_t)0xff00000000000000ULL) >> 56) )); \
-})
-
-#endif /* !HAVE_BYTESWAP_H */
 
 static inline uint16_t bswap16(uint16_t x)
 {
#4 lore raw

PATCH v2 1/1 build: detect is libiconv is present

In reply to Roger Pau Monne

# HG changeset patch
# User Roger Pau Monne <[email protected]> # Date 1324344612 -3600
# Node ID 14e911353a91702b439bc06e2a77d67e8bd5f661 # Parent 23e1abbd203ca2619103f3e95c6b1ff36be60e38
build: detect is libiconv is present

Detect if libiconv is present in the system, since we will have to link against it when using iconv.

Config.mk +2 -0
diff -r 23e1abbd203c -r 14e911353a91 Config.mk
@@ -18,6 +18,8 @@ XEN_TARGET_ARCH     ?= $(XEN_COMPILE_ARC
 XEN_OS              ?= $(shell uname -s)
 
 CONFIG_$(XEN_OS) := y
+CONFIG_LIBICONV  := $(shell grep -q LIBICONV_PLUG /usr/include/iconv.h \
+                      && echo 'y' || echo 'n')
 
 SHELL     ?= /bin/sh
#5 lore raw

Re: [PATCH 3 of 4 v2] build: detect is libiconv is present

Hi,

Quoted 18 lines · Roger Pau Monne

# HG changeset patch
# User Roger Pau Monne <[email protected]> # Date 1324344612 -3600
# Node ID 14e911353a91702b439bc06e2a77d67e8bd5f661 # Parent  23e1abbd203ca2619103f3e95c6b1ff36be60e38
build: detect is libiconv is present

Detect if libiconv is present in the system, since we will have to link against it when using iconv.

diff -r 23e1abbd203c -r 14e911353a91 Config.mk

Config.mk Tue Dec 20 02:30:12 2011 +0100 renamed from Config.mk Tue Dec 20 02:28:34 2011 +0100 +0 -0
@@ -18,6 +18,8 @@ XEN_TARGET_ARCH     ?= $(XEN_COMPILE_ARC
  XEN_OS              ?= $(shell uname -s)
 
Quoted 4 lines

 CONFIG_$(XEN_OS) := y
+CONFIG_LIBICONV  := $(shell grep -q LIBICONV_PLUG /usr/include/iconv.h \
+                      && echo 'y' || echo 'n')

Please don't check this way, it is not guaranteed to work. Check instead for presence of libiconv.so somehow, perhaps using a dummy program with -liconv or something, or using a construct like:

test -f /usr/lib/libiconv.so -o -f /lib/libiconv.so -o -f
/usr/local/lib/libiconv.so && echo 'y' || echo 'n'

If upstream removes LIBICONV_PLUG, this check will break.

Otherwise, everything else looks OK to me.

William

#6 lore raw

Re: [PATCH 3 of 4 v2] build: detect is libiconv is present

Quoted 31 lines · William Pitcock

Hi,

Quoted 18 lines · Roger Pau Monne

# HG changeset patch
# User Roger Pau Monne <[email protected]> # Date 1324344612 -3600
# Node ID 14e911353a91702b439bc06e2a77d67e8bd5f661 # Parent  23e1abbd203ca2619103f3e95c6b1ff36be60e38
build: detect is libiconv is present

Detect if libiconv is present in the system, since we will have to link against it when using iconv.

diff -r 23e1abbd203c -r 14e911353a91 Config.mk

Config.mk Tue Dec 20 02:30:12 2011 +0100 renamed from Config.mk Tue Dec 20 02:28:34 2011 +0100 +0 -0
@@ -18,6 +18,8 @@ XEN_TARGET_ARCH     ?= $(XEN_COMPILE_ARC
  XEN_OS              ?= $(shell uname -s)
 
Quoted 4 lines

 CONFIG_$(XEN_OS) := y
+CONFIG_LIBICONV  := $(shell grep -q LIBICONV_PLUG /usr/include/iconv.h \
+                      && echo 'y' || echo 'n')

Please don't check this way, it is not guaranteed to work.  Check instead for presence of libiconv.so somehow, perhaps using a dummy program with -liconv or something, or using a construct like:

Quoted 7 lines

 test -f /usr/lib/libiconv.so -o -f /lib/libiconv.so -o -f /usr/local/lib/libiconv.so && echo 'y' || echo 'n'

If upstream removes LIBICONV_PLUG, this check will break.

Otherwise, everything else looks OK to me.

That doesn't work if you do cross compilation, your libraries could located in a completely different path.

The best way to check for the presence of a library (and this is how the autotools do it) is to generate a very simple C program that uses some iconv functions and try to compile it with the currently configured compiler. If the compilation succeeded that means the library is present.

Jean

#7 lore raw

Re: [PATCH 3 of 4 v2] build: detect is libiconv is present

2011/12/21 Jean Guyader <[email protected]>:

Quoted 8 lines

That doesn't work if you do cross compilation, your libraries could located in a completely different path.

The best way to check for the presence of a library (and this is how the autotools do it) is to generate a very simple C program that uses some iconv functions and try to compile it with the currently configured compiler. If the compilation succeeded that means the library is present.

I'm not sure this is easy to implement without much fuss, given the way Xen checks for configuration options. What about this:

CONFIG_LIBICONV   := $(shell export OS="`uname -s`"; \
                       export CHECK_LIB="$(CHECK_LIB)"; \
                       . $(XEN_ROOT)/tools/check/funcs.sh; \
                       has_lib libiconv.so && echo 'y' || echo 'n')
#8 lore raw

Re: [PATCH 3 of 4 v2] build: detect is libiconv is present

Quoted 10 lines · Roger Pau Monné

2011/12/21 Jean Guyader <[email protected]>:

Quoted 8 lines

That doesn't work if you do cross compilation, your libraries could located in a completely different path.

The best way to check for the presence of a library (and this is how the autotools do it) is to generate a very simple C program that uses some iconv functions and try to compile it with the currently configured compiler. If the compilation succeeded that means the library is present.

Quoted 7 lines

I'm not sure this is easy to implement without much fuss, given the way Xen checks for configuration options. What about this:

CONFIG_LIBICONV   := $(shell export OS="uname -s"; \
                      export CHECK_LIB="$(CHECK_LIB)"; \
                      . $(XEN_ROOT)/tools/check/funcs.sh; \
                      has_lib libiconv.so && echo 'y' || echo 'n')

I think that does the right thing.

Jean

#9 lore raw

PATCH v2 1/1 blktap2/vhd: add -liconv when linking if using libiconv

In reply to Roger Pau Monne

# HG changeset patch
# User Roger Pau Monne <[email protected]> # Date 1324344791 -3600
# Node ID 993c86e4ecbc85a95f27ba299e8e9285d76c56b2 # Parent 14e911353a91702b439bc06e2a77d67e8bd5f661 blktap2/vhd: add -liconv when linking if using libiconv

If libiconv is detected on the system add -liconv when linking the libvhd library.

If -liconv is not added when compiling libvhd with libiconv the following error occours when linking vhd-util and vhd-update:

gcc -o vhd-util vhd-util.o -Llib -lvhd
lib/libvhd.so: undefined reference to libiconv_open' lib/libvhd.so: undefined reference to libiconv_close' lib/libvhd.so: undefined reference to `libiconv'

tools/blktap2/vhd/lib/Makefile +4 -0
diff -r 14e911353a91 -r 993c86e4ecbc tools/blktap2/vhd/lib/Makefile
@@ -23,6 +23,10 @@ ifeq ($(CONFIG_Linux),y)
 LIBS            := -luuid
 endif
 
+ifeq ($(CONFIG_LIBICONV),y)
+LIBS            += -liconv
+endif
+
 LIB-SRCS        := libvhd.c
 LIB-SRCS        += libvhd-journal.c
 LIB-SRCS        += vhd-util-coalesce.c