Libguestfs - Oct 2013 - [Hivex] [PATCH] lib: Promote byte_conversions.h #include to hivex-internal.h

This patch addresses a build failure in OS X.  Running git-bisect on a
straightforward build (bootstrap, autogen.sh, configure, make, make
install) showed this as the "Bad commit:"
3e7c039799cddc45517350cc917eb10715f33fec

The issue is that hivex-internal.h uses le32toh in a static inline
function.  In case `configure` doesn't find le32toh, byte_conversions.h
defines it.  But hivex-internal.h doesn't include the safety definition.

OS X demonstrates this a problem.  Neither endian.h nor byteswap.h are
found with `configure` in OS X 10.8.5 (XCode 5), but the headers are
both found in Fedora 19 and Ubuntu 13.04.  This patch to configure.ac
further logs that only ntohl is available for byte swaps:

    @@ -153,6 +153,8 @@ AC_REPLACE_FUNCS([mmap])
     dnl Functions.
     AC_CHECK_FUNCS([bindtextdomain])

    +AC_CHECK_FUNCS([le32toh ntohl bswap_32 __bswap_32])
    +

(As an aside, it's curious that a missing byteswap.h didn't cause
hivex-internal.h to fail to build.)
(As another aside, this is an interesting example of lazy symbol
binding failing with an inline function.)

The problem is resolved by having hivex-internal.h include
byte_conversions.h.  This obviates most of the other direct inclusions
of byte_conversions.h.  (One persists under sh/.)

This patch builds and runs hivexml on images/large fine in Ubuntu 13.04
and Fedora 19.  It also allows builds to succeed in OS X, but doesn't
run hivexml for an unrelated reason.  The _iconv_open
symbol-not-found issue, that I thought was previously resolved
(491ba0f7a761c7ffd50e0eaa4d892f78d538eb2b), resurfaced.

Signed-off-by: Alex Nelson <a.nelson@prometheuscomputing.com>
---
 lib/handle.c         | 1 -
 lib/hivex-internal.h | 2 ++
 lib/node.c           | 1 -
 lib/utf16.c          | 1 -
 lib/value.c          | 1 -
 lib/visit.c          | 1 -
 lib/write.c          | 1 -
 7 files changed, 2 insertions(+), 6 deletions(-)

diff --git a/lib/handle.c b/lib/handle.c
index 982a6fb..62a8644 100644
--- a/lib/handle.c
+++ b/lib/handle.c
@@ -44,7 +44,6 @@
 
 #include "hivex.h"
 #include "hivex-internal.h"
-#include "byte_conversions.h"
 
 static uint32_t
 header_checksum (const hive_h *h)
diff --git a/lib/hivex-internal.h b/lib/hivex-internal.h
index 53499cb..f391b98 100644
--- a/lib/hivex-internal.h
+++ b/lib/hivex-internal.h
@@ -22,6 +22,8 @@
 #include <stdarg.h>
 #include <stddef.h>
 
+#include "byte_conversions.h"
+
 struct hive_h {
   char *filename;
   int fd;
diff --git a/lib/node.c b/lib/node.c
index ed72b51..5090dbe 100644
--- a/lib/node.c
+++ b/lib/node.c
@@ -34,7 +34,6 @@
 
 #include "hivex.h"
 #include "hivex-internal.h"
-#include "byte_conversions.h"
 
 hive_node_h
 hivex_root (hive_h *h)
diff --git a/lib/utf16.c b/lib/utf16.c
index d0f2e45..844715c 100644
--- a/lib/utf16.c
+++ b/lib/utf16.c
@@ -27,7 +27,6 @@
 
 #include "hivex.h"
 #include "hivex-internal.h"
-#include "byte_conversions.h"
 
 char *
 _hivex_windows_utf16_to_utf8 (/* const */ char *input, size_t len)
diff --git a/lib/value.c b/lib/value.c
index d713cdc..6d0f266 100644
--- a/lib/value.c
+++ b/lib/value.c
@@ -32,7 +32,6 @@
 
 #include "hivex.h"
 #include "hivex-internal.h"
-#include "byte_conversions.h"
 
 int
 _hivex_get_values (hive_h *h, hive_node_h node,
diff --git a/lib/visit.c b/lib/visit.c
index bd95b6c..87c755f 100644
--- a/lib/visit.c
+++ b/lib/visit.c
@@ -32,7 +32,6 @@
 
 #include "hivex.h"
 #include "hivex-internal.h"
-#include "byte_conversions.h"
 
 int
 hivex_visit (hive_h *h, const struct hivex_visitor *visitor, size_t len,
diff --git a/lib/write.c b/lib/write.c
index bc2251c..dbb8292 100644
--- a/lib/write.c
+++ b/lib/write.c
@@ -41,7 +41,6 @@
 
 #include "hivex.h"
 #include "hivex-internal.h"
-#include "byte_conversions.h"
 
 /*----------------------------------------------------------------------
  * Writing.
-- 
1.8.3.4 (Apple Git-47)

On Tue, Oct 15, 2013 at 09:28:13PM -0400, Alex Nelson
wrote:
> This patch addresses a build failure in OS X.  Running git-bisect on a
> straightforward build (bootstrap, autogen.sh, configure, make, make
> install) showed this as the "Bad commit:"
> 3e7c039799cddc45517350cc917eb10715f33fec
> 
> The issue is that hivex-internal.h uses le32toh in a static inline
> function.  In case `configure` doesn't find le32toh, byte_conversions.h
> defines it.  But hivex-internal.h doesn't include the safety
definition.
> 
> OS X demonstrates this a problem.  Neither endian.h nor byteswap.h are
> found with `configure` in OS X 10.8.5 (XCode 5), but the headers are
> both found in Fedora 19 and Ubuntu 13.04.  This patch to configure.ac
> further logs that only ntohl is available for byte swaps:
> 
>     @@ -153,6 +153,8 @@ AC_REPLACE_FUNCS([mmap])
>      dnl Functions.
>      AC_CHECK_FUNCS([bindtextdomain])
> 
>     +AC_CHECK_FUNCS([le32toh ntohl bswap_32 __bswap_32])
>     +
> 
> (As an aside, it's curious that a missing byteswap.h didn't cause
> hivex-internal.h to fail to build.)
> (As another aside, this is an interesting example of lazy symbol
> binding failing with an inline function.)
> 
> The problem is resolved by having hivex-internal.h include
> byte_conversions.h.  This obviates most of the other direct inclusions
> of byte_conversions.h.  (One persists under sh/.)
> 
> This patch builds and runs hivexml on images/large fine in Ubuntu 13.04
> and Fedora 19.  It also allows builds to succeed in OS X, but doesn't
> run hivexml for an unrelated reason.  The _iconv_open
> symbol-not-found issue, that I thought was previously resolved
> (491ba0f7a761c7ffd50e0eaa4d892f78d538eb2b), resurfaced.
> 
> Signed-off-by: Alex Nelson <a.nelson@prometheuscomputing.com>
Hi Alex,

This all looks good to me.  I've ACKed and pushed it.

Thanks,

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Fedora Windows cross-compiler. Compile Windows programs, test, and
build Windows installers. Over 100 libraries supported.
http://fedoraproject.org/wiki/MinGW