notmuch sha1 implementation broken on (some) big-endian architectures

notmuch sha1 implementation broken on (some) big-endian architectures

[David Bremner]

The following code, when linked with libnotmuch.a and libutil.a does a
passable imitation of sha1sum on amd64 (and I guess also i386) but
computes a different digest on powerpc and probably sparc and s390x.

In the long run we should maybe outsource hash computations to
e.g. librhash, but I'd like a simpler fix for 0.17, if possible

P.S. I blame Austin for adding the "missing-headers" test which found
this bug ;).

/* 8<----------------------------------------- */

#include <stdio.h>

#include "notmuch.h"
char * notmuch_sha1_of_file(const char* filename);

int
main (int argc, char **argv)
{

    char *digest = notmuch_sha1_of_file (argv[1]);

    printf("%s  %s\n",digest,argv[1]);
    return 0;
}

Re: notmuch sha1 implementation broken on (some) big-endian architectures

[David Bremner]

David Bremner <david@tethera.net> writes:

> The following code, when linked with libnotmuch.a and libutil.a does a
> passable imitation of sha1sum on amd64 (and I guess also i386) but
> computes a different digest on powerpc and probably sparc and s390x.
>
> In the long run we should maybe outsource hash computations to
> e.g. librhash, but I'd like a simpler fix for 0.17, if possible

Out of curiousity, I tried out a similar example with librhash, and it
works fine on powerpc.

#include <errno.h>
#include "rhash.h" /* LibRHash interface */

int main(int argc, char *argv[])
{
  char digest[64];
  char output[130];

  rhash_library_init(); /* initialize static data */

  int res = rhash_file(RHASH_SHA1, argv[1], digest);
  if(res < 0) {
    fprintf(stderr, "LibRHash error: %s: %s\n", argv[1], strerror(errno));
    return 1;
  }

  /* convert binary digest to hexadecimal string */
  rhash_print_bytes(output, digest, rhash_get_digest_size(RHASH_SHA1),RHPR_HEX);

  printf("%s (%s) = %s\n", rhash_get_name(RHASH_SHA1), argv[1], output);
  return 0;
}

Re: notmuch sha1 implementation broken on (some) big-endian architectures

[Tomi Ollila]

On Sun, Nov 24 2013, David Bremner <david@tethera.net> wrote:

> The following code, when linked with libnotmuch.a and libutil.a does a
> passable imitation of sha1sum on amd64 (and I guess also i386) but
> computes a different digest on powerpc and probably sparc and s390x.
>
> In the long run we should maybe outsource hash computations to
> e.g. librhash, but I'd like a simpler fix for 0.17, if possible
>
> P.S. I blame Austin for adding the "missing-headers" test which found
> this bug ;).

This is interesting problem, I would have guessed that this would
fails on LITTLE_ENDIAN machines easier, if ever...

... especially as there is line

lib/libsha1.c:52:#if (PLATFORM_BYTE_ORDER == IS_LITTLE_ENDIAN)

... but...

I could not find any (other) matches for PLATFORM_BYTE_ORDER nor
IS_LITTLE_ENDIAN in source code or in /usr/include/**/*.h or
in /usr/lib/gcc/**/*.h

I did some testing and it seems that 
#if (FOOOO == BBBAAARRR)
<code>
#endif

will have <code> in output file in case neither of the above are
defined... :/

So, this could work:

#if <endian.h>  // for BYTE_ORDER && LITTLE_ENDIAN

and then

#if (BYTE_ORDER == LITTLE_ENDIAN)
...

to replace lib/libsha1.c:53 (53 now after endian.h added)


Please test on BIG_ENDIAN machine...


In case this works, then we'd need to inform users that their
long/missing Message ID:s are now coded differently in their
databases...


Tomi

[PATCH 1/2] lib: fix byte order test in libsha1.c

[david]

From: David Bremner <david@tethera.net>

Previously PLATFORM_BYTE_ORDER and IS_LITTLE_ENDIAN were not defined,
so the little endian code was always compiled in.

This will have the effect that the "SHA1s" on big endian architectures
will change (i.e. become actual sha1s). So someone re-indexing their
database could conceivable lose tags on messages without a message-id
header.
---
 lib/libsha1.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/lib/libsha1.c b/lib/libsha1.c
index 5d16f6a..794854b 100644
--- a/lib/libsha1.c
+++ b/lib/libsha1.c
@@ -49,11 +49,17 @@ extern "C"
 
 #define bswap_32(x) ((rotr32((x), 24) & 0x00ff00ff) | (rotr32((x), 8) & 0xff00ff00))
 
-#if (PLATFORM_BYTE_ORDER == IS_LITTLE_ENDIAN)
-#define bsw_32(p,n) \
-    { int _i = (n); while(_i--) ((uint32_t*)p)[_i] = bswap_32(((uint32_t*)p)[_i]); }
+#ifdef __BYTE_ORDER__
+#  if (__BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__)
+#    define bsw_32(p,n) \
+       { int _i = (n); while(_i--) ((uint32_t*)p)[_i] = bswap_32(((uint32_t*)p)[_i]); }
+#  elif (__BYTE_ORDER__ == __ORDER_BIG_ENDIAN__)
+#    define bsw_32(p,n)
+#  else
+#    error "unknown byte order"
+#  endif
 #else
-#define bsw_32(p,n)
+#    error "macro __BYTE_ORDER__ is not defined"
 #endif
 
 #define SHA1_MASK   (SHA1_BLOCK_SIZE - 1)
-- 
1.8.4.2

[PATCH 2/2] NEWS: News for big endian sha1 bug fix.

[david]

From: David Bremner <david@tethera.net>

We could give more details about how to migrate tags, but I'm not sure
that it's a practical problem, or just a theoretical one.
---
 NEWS | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/NEWS b/NEWS
index 3383ecf..31c6284 100644
--- a/NEWS
+++ b/NEWS
@@ -1,6 +1,18 @@
-Notmuch 0.17~rc1 (2013-11-20)
+Notmuch 0.17~rc2 (2013-xx-yy)
 =============================
 
+Incompatible change in SHA1 computation
+---------------------------------------
+
+Previously on big endian architectures like sparc and powerpc the
+computation of SHA1 hashes was incorrect. This meant that messages
+with overlong or missing message-ids were given different computed
+message-ids than on more common little endian architectures like i386
+and amd64.  If you use notmuch on a big endian architecture, you are
+strongly advised to make a backup of your tags using `notmuch dump`
+before this upgrade. It should be possible to migrate the tags using a
+script.
+
 Command-Line Interface
 ----------------------
 
-- 
1.8.4.2

Re: [PATCH 2/2] NEWS: News for big endian sha1 bug fix.

[Austin Clements]

Quoth david@tethera.net on Nov 24 at  5:29 pm:
> From: David Bremner <david@tethera.net>
> 
> We could give more details about how to migrate tags, but I'm not sure
> that it's a practical problem, or just a theoretical one.
> ---
>  NEWS | 14 +++++++++++++-
>  1 file changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/NEWS b/NEWS
> index 3383ecf..31c6284 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -1,6 +1,18 @@
> -Notmuch 0.17~rc1 (2013-11-20)
> +Notmuch 0.17~rc2 (2013-xx-yy)
>  =============================
>  
> +Incompatible change in SHA1 computation
> +---------------------------------------
> +
> +Previously on big endian architectures like sparc and powerpc the
> +computation of SHA1 hashes was incorrect. This meant that messages
> +with overlong or missing message-ids were given different computed
> +message-ids than on more common little endian architectures like i386
> +and amd64.  If you use notmuch on a big endian architecture, you are
> +strongly advised to make a backup of your tags using `notmuch dump`
> +before this upgrade. It should be possible to migrate the tags using a
> +script.
> +

Should this mention how to find such messages?  Something like

  notmuch dump |
  awk '/^notmuch-sha1-[0-9a-f]{40} / {system("notmuch search id:" $1)}'

Re: [PATCH 1/2] lib: fix byte order test in libsha1.c

[Tomi Ollila]

On Sun, Nov 24 2013, david@tethera.net wrote:

> From: David Bremner <david@tethera.net>
>
> Previously PLATFORM_BYTE_ORDER and IS_LITTLE_ENDIAN were not defined,
> so the little endian code was always compiled in.
>
> This will have the effect that the "SHA1s" on big endian architectures
> will change (i.e. become actual sha1s). So someone re-indexing their
> database could conceivable lose tags on messages without a message-id
> header.
> ---
>  lib/libsha1.c | 14 ++++++++++----
>  1 file changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/lib/libsha1.c b/lib/libsha1.c
> index 5d16f6a..794854b 100644
> --- a/lib/libsha1.c
> +++ b/lib/libsha1.c
> @@ -49,11 +49,17 @@ extern "C"
>  
>  #define bswap_32(x) ((rotr32((x), 24) & 0x00ff00ff) | (rotr32((x), 8) & 0xff00ff00))
>  
> -#if (PLATFORM_BYTE_ORDER == IS_LITTLE_ENDIAN)
> -#define bsw_32(p,n) \
> -    { int _i = (n); while(_i--) ((uint32_t*)p)[_i] = bswap_32(((uint32_t*)p)[_i]); }
> +#ifdef __BYTE_ORDER__
> +#  if (__BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__)
> +#    define bsw_32(p,n) \
> +       { int _i = (n); while(_i--) ((uint32_t*)p)[_i] = bswap_32(((uint32_t*)p)[_i]); }
> +#  elif (__BYTE_ORDER__ == __ORDER_BIG_ENDIAN__)
> +#    define bsw_32(p,n)
> +#  else
> +#    error "unknown byte order"
> +#  endif
>  #else
> -#define bsw_32(p,n)
> +#    error "macro __BYTE_ORDER__ is not defined"
>  #endif

LGTM. 

When you modify te NEWS entry by adding instructions how to find
out those sha1's of message-id:s that are affected (Austin's code)
You could perhaps add note of the origin of these macros
(i.e. something like 
/* These "Common Predefined Macros" below are GNU C extensions */ )

See http://gcc.gnu.org/onlinedocs/cpp/Common-Predefined-Macros.html
for reference.

Tomi

>  
>  #define SHA1_MASK   (SHA1_BLOCK_SIZE - 1)
> -- 
> 1.8.4.2

[PATCH] util: detect byte order

[david]

From: David Bremner <david@tethera.net>

Unfortunately old versions of GCC and clang do not provide byte order
macros, so we re-invent them.

If UTIL_BYTE_ORDER is not defined, we fall back to macros supported by
recent versions of GCC and clang
---

I pushed the series
id:1385328583-24602-1-git-send-email-david@tethera.net; unfortunately
that broke compilation on old versions of GCC, in particular on our
buildbot. Here is a proposed fix for the fix.

 configure          | 26 ++++++++++++++++++++++++--
 lib/libsha1.c      | 19 +++++--------------
 util/endian-util.h | 39 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 68 insertions(+), 16 deletions(-)
 create mode 100644 util/endian-util.h

diff --git a/configure b/configure
index 1a8e939..02d6396 100755
--- a/configure
+++ b/configure
@@ -441,6 +441,21 @@ else
 EOF
 fi
 
+printf "Checking byte order... "
+cat> _byteorder.c <<EOF
+#include <stdio.h>
+#include <stdint.h>
+#include <string.h>
+uint32_t test = 0x31323334;
+char buf[5];
+int main() { memcpy(buf, &test, 4); buf[4] = '\0'; printf("%s\n", buf); return 0; }
+EOF
+${CC} ${CFLAGS} _byteorder.c -o _byteorder > /dev/null 2>&1
+util_byte_order=$(./_byteorder)
+echo $util_byte_order
+
+#rm -f _byteorder _byteorder.c
+
 if [ $errors -gt 0 ]; then
     cat <<EOF
 
@@ -702,6 +717,9 @@ prefix = ${PREFIX}
 # LIBDIR_IN_LDCONFIG value below is still set correctly.
 libdir = ${LIBDIR:=\$(prefix)/lib}
 
+# byte order within a 32 bit word. 4321 = little, 1234 = big, 0 = guess
+UTIL_BYTE_ORDER = ${util_byte_order}
+
 # Whether libdir is in a path configured into ldconfig
 LIBDIR_IN_LDCONFIG = ${libdir_in_ldconfig}
 
@@ -807,7 +825,9 @@ CONFIGURE_CFLAGS = -DHAVE_GETLINE=\$(HAVE_GETLINE) \$(GMIME_CFLAGS)      \\
 		   -DHAVE_STRSEP=\$(HAVE_STRSEP)                         \\
 		   -DSTD_GETPWUID=\$(STD_GETPWUID)                       \\
 		   -DSTD_ASCTIME=\$(STD_ASCTIME)                         \\
-		   -DHAVE_XAPIAN_COMPACT=\$(HAVE_XAPIAN_COMPACT)
+		   -DHAVE_XAPIAN_COMPACT=\$(HAVE_XAPIAN_COMPACT)	 \\
+		   -DUTIL_BYTE_ORDER=\$(UTIL_BYTE_ORDER)
+
 CONFIGURE_CXXFLAGS = -DHAVE_GETLINE=\$(HAVE_GETLINE) \$(GMIME_CFLAGS)    \\
 		     \$(TALLOC_CFLAGS) -DHAVE_VALGRIND=\$(HAVE_VALGRIND) \\
 		     \$(VALGRIND_CFLAGS) \$(XAPIAN_CXXFLAGS)             \\
@@ -815,6 +835,8 @@ CONFIGURE_CXXFLAGS = -DHAVE_GETLINE=\$(HAVE_GETLINE) \$(GMIME_CFLAGS)    \\
 		     -DHAVE_STRSEP=\$(HAVE_STRSEP)                       \\
 		     -DSTD_GETPWUID=\$(STD_GETPWUID)                     \\
 		     -DSTD_ASCTIME=\$(STD_ASCTIME)                       \\
-		     -DHAVE_XAPIAN_COMPACT=\$(HAVE_XAPIAN_COMPACT)
+		     -DHAVE_XAPIAN_COMPACT=\$(HAVE_XAPIAN_COMPACT)       \\
+		     -DUTIL_BYTE_ORDER=\$(UTIL_BYTE_ORDER)
+
 CONFIGURE_LDFLAGS =  \$(GMIME_LDFLAGS) \$(TALLOC_LDFLAGS) \$(XAPIAN_LDFLAGS)
 EOF
diff --git a/lib/libsha1.c b/lib/libsha1.c
index 87c7c52..3566ed7 100644
--- a/lib/libsha1.c
+++ b/lib/libsha1.c
@@ -34,7 +34,7 @@
 */
 
 #include <string.h>     /* for memcpy() etc.        */
-
+#include "endian-util.h"
 #include "libsha1.h"
 
 #if defined(__cplusplus)
@@ -49,20 +49,11 @@ extern "C"
 
 #define bswap_32(x) ((rotr32((x), 24) & 0x00ff00ff) | (rotr32((x), 8) & 0xff00ff00))
 
-/* The macros __BYTE_ORDER__ and __ORDER_*_ENDIAN__ are GNU C
- * extensions. They are also supported by clang as of v3.2 */
-
-#ifdef __BYTE_ORDER__
-#  if (__BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__)
-#    define bsw_32(p,n) \
-       { int _i = (n); while(_i--) ((uint32_t*)p)[_i] = bswap_32(((uint32_t*)p)[_i]); }
-#  elif (__BYTE_ORDER__ == __ORDER_BIG_ENDIAN__)
-#    define bsw_32(p,n)
-#  else
-#    error "unknown byte order"
-#  endif
+#if (UTIL_BYTE_ORDER == UTIL_ORDER_LITTLE_ENDIAN)
+#  define bsw_32(p,n) \
+     { int _i = (n); while(_i--) ((uint32_t*)p)[_i] = bswap_32(((uint32_t*)p)[_i]); }
 #else
-#    error "macro __BYTE_ORDER__ is not defined"
+#  define bsw_32(p,n)
 #endif
 
 #define SHA1_MASK   (SHA1_BLOCK_SIZE - 1)
diff --git a/util/endian-util.h b/util/endian-util.h
new file mode 100644
index 0000000..cbecf66
--- /dev/null
+++ b/util/endian-util.h
@@ -0,0 +1,39 @@
+/* this file mimics the macros present in recent GCC and CLANG */
+
+#ifndef _ENDIAN_UTIL_H
+#define _ENDIAN_UTIL_H
+
+/* This are prefixed with UTIL to avoid collisions
+ *
+ * You can use something like the following to define UTIL_BYTE_ORDER
+ * in a configure script.
+ */
+#if 0
+#include <stdio.h>
+#include <stdint.h>
+#include <string.h>
+uint32_t test = 0x31323334;
+char buf[5];
+int main() { memcpy(buf, &test, 4); buf[4] = '\0'; printf("%s\n", buf); return 0; }
+#endif
+
+#define UTIL_ORDER_LITTLE_ENDIAN 4321
+#define UTIL_ORDER_BIG_ENDIAN    1234
+
+
+#if !defined(UTIL_BYTE_ORDER) || ((UTIL_BYTE_ORDER != UTIL_ORDER_LITTLE_ENDIAN) && \
+				   (UTIL_BYTE_ORDER != UTIL_ORDER_LITTLE_ENDIAN))
+#ifdef __BYTE_ORDER__
+#  if (__BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__)
+#    define UTIL_BYTE_ORDER UTIL_ORDER_LITTLE_ENDIAN
+#  elif (__BYTE_ORDER__ == __ORDER_BIG_ENDIAN__)
+#    define UTIL_BYTE_ORDER UTIL_ORDER_BIG_ENDIAN
+#  else
+#    error "Unsupported __BYTE_ORDER__"
+#  endif
+#else
+#  error "UTIL_BYTE_ORDER not correctly defined and __BYTE_ORDER__ not defined."
+#endif
+#endif
+
+#endif
-- 
1.8.4.2

Re: [PATCH] util: detect byte order

[Tomi Ollila]

On Tue, Nov 26 2013, david@tethera.net wrote:

> From: David Bremner <david@tethera.net>
>
> Unfortunately old versions of GCC and clang do not provide byte order
> macros, so we re-invent them.

Brief comments inline :D

>
> If UTIL_BYTE_ORDER is not defined, we fall back to macros supported by
> recent versions of GCC and clang

This is not entirely accurate as -DUTIL_BYTE_ORDER=... is given to
compiler... maybe If UTIL_BYTE_ORDER is not defined, empty or zero (?)

> ---
>
> I pushed the series
> id:1385328583-24602-1-git-send-email-david@tethera.net; unfortunately
> that broke compilation on old versions of GCC, in particular on our
> buildbot. Here is a proposed fix for the fix.
>
>  configure          | 26 ++++++++++++++++++++++++--
>  lib/libsha1.c      | 19 +++++--------------
>  util/endian-util.h | 39 +++++++++++++++++++++++++++++++++++++++
>  3 files changed, 68 insertions(+), 16 deletions(-)
>  create mode 100644 util/endian-util.h
>
> diff --git a/configure b/configure
> index 1a8e939..02d6396 100755
> --- a/configure
> +++ b/configure
> @@ -441,6 +441,21 @@ else
>  EOF
>  fi
>  
> +printf "Checking byte order... "
> +cat> _byteorder.c <<EOF
> +#include <stdio.h>
> +#include <stdint.h>
> +#include <string.h>
> +uint32_t test = 0x31323334;
> +char buf[5];
> +int main() { memcpy(buf, &test, 4); buf[4] = '\0'; printf("%s\n", buf); return 0; }
> +EOF
> +${CC} ${CFLAGS} _byteorder.c -o _byteorder > /dev/null 2>&1
> +util_byte_order=$(./_byteorder)
> +echo $util_byte_order

Austin version with 0x34333231

> +
> +#rm -f _byteorder _byteorder.c
> +
>  if [ $errors -gt 0 ]; then
>      cat <<EOF
>  
> @@ -702,6 +717,9 @@ prefix = ${PREFIX}
>  # LIBDIR_IN_LDCONFIG value below is still set correctly.
>  libdir = ${LIBDIR:=\$(prefix)/lib}
>  
> +# byte order within a 32 bit word. 4321 = little, 1234 = big, 0 = guess
> +UTIL_BYTE_ORDER = ${util_byte_order}
> +
>  # Whether libdir is in a path configured into ldconfig
>  LIBDIR_IN_LDCONFIG = ${libdir_in_ldconfig}
>  
> @@ -807,7 +825,9 @@ CONFIGURE_CFLAGS = -DHAVE_GETLINE=\$(HAVE_GETLINE) \$(GMIME_CFLAGS)      \\
>  		   -DHAVE_STRSEP=\$(HAVE_STRSEP)                         \\
>  		   -DSTD_GETPWUID=\$(STD_GETPWUID)                       \\
>  		   -DSTD_ASCTIME=\$(STD_ASCTIME)                         \\
> -		   -DHAVE_XAPIAN_COMPACT=\$(HAVE_XAPIAN_COMPACT)
> +		   -DHAVE_XAPIAN_COMPACT=\$(HAVE_XAPIAN_COMPACT)	 \\
> +		   -DUTIL_BYTE_ORDER=\$(UTIL_BYTE_ORDER)
> +
>  CONFIGURE_CXXFLAGS = -DHAVE_GETLINE=\$(HAVE_GETLINE) \$(GMIME_CFLAGS)    \\
>  		     \$(TALLOC_CFLAGS) -DHAVE_VALGRIND=\$(HAVE_VALGRIND) \\
>  		     \$(VALGRIND_CFLAGS) \$(XAPIAN_CXXFLAGS)             \\
> @@ -815,6 +835,8 @@ CONFIGURE_CXXFLAGS = -DHAVE_GETLINE=\$(HAVE_GETLINE) \$(GMIME_CFLAGS)    \\
>  		     -DHAVE_STRSEP=\$(HAVE_STRSEP)                       \\
>  		     -DSTD_GETPWUID=\$(STD_GETPWUID)                     \\
>  		     -DSTD_ASCTIME=\$(STD_ASCTIME)                       \\
> -		     -DHAVE_XAPIAN_COMPACT=\$(HAVE_XAPIAN_COMPACT)
> +		     -DHAVE_XAPIAN_COMPACT=\$(HAVE_XAPIAN_COMPACT)       \\
> +		     -DUTIL_BYTE_ORDER=\$(UTIL_BYTE_ORDER)
> +
>  CONFIGURE_LDFLAGS =  \$(GMIME_LDFLAGS) \$(TALLOC_LDFLAGS) \$(XAPIAN_LDFLAGS)
>  EOF
> diff --git a/lib/libsha1.c b/lib/libsha1.c
> index 87c7c52..3566ed7 100644
> --- a/lib/libsha1.c
> +++ b/lib/libsha1.c
> @@ -34,7 +34,7 @@
>  */
>  
>  #include <string.h>     /* for memcpy() etc.        */
> -
> +#include "endian-util.h"
>  #include "libsha1.h"
>  
>  #if defined(__cplusplus)
> @@ -49,20 +49,11 @@ extern "C"
>  
>  #define bswap_32(x) ((rotr32((x), 24) & 0x00ff00ff) | (rotr32((x), 8) & 0xff00ff00))
>  
> -/* The macros __BYTE_ORDER__ and __ORDER_*_ENDIAN__ are GNU C
> - * extensions. They are also supported by clang as of v3.2 */
> -
> -#ifdef __BYTE_ORDER__
> -#  if (__BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__)
> -#    define bsw_32(p,n) \
> -       { int _i = (n); while(_i--) ((uint32_t*)p)[_i] = bswap_32(((uint32_t*)p)[_i]); }
> -#  elif (__BYTE_ORDER__ == __ORDER_BIG_ENDIAN__)
> -#    define bsw_32(p,n)
> -#  else
> -#    error "unknown byte order"
> -#  endif
> +#if (UTIL_BYTE_ORDER == UTIL_ORDER_LITTLE_ENDIAN)
> +#  define bsw_32(p,n) \
> +     { int _i = (n); while(_i--) ((uint32_t*)p)[_i] = bswap_32(((uint32_t*)p)[_i]); }
>  #else
> -#    error "macro __BYTE_ORDER__ is not defined"
> +#  define bsw_32(p,n)
>  #endif

I'd keep the BIG_ENDIAN check too, as we don't know what would happen w/
PDP_ENDIAN...

>  
>  #define SHA1_MASK   (SHA1_BLOCK_SIZE - 1)
> diff --git a/util/endian-util.h b/util/endian-util.h
> new file mode 100644
> index 0000000..cbecf66
> --- /dev/null
> +++ b/util/endian-util.h
> @@ -0,0 +1,39 @@
> +/* this file mimics the macros present in recent GCC and CLANG */
> +
> +#ifndef _ENDIAN_UTIL_H
> +#define _ENDIAN_UTIL_H
> +
> +/* This are prefixed with UTIL to avoid collisions
> + *
> + * You can use something like the following to define UTIL_BYTE_ORDER
> + * in a configure script.
> + */
> +#if 0
> +#include <stdio.h>
> +#include <stdint.h>
> +#include <string.h>
> +uint32_t test = 0x31323334;
> +char buf[5];
> +int main() { memcpy(buf, &test, 4); buf[4] = '\0'; printf("%s\n", buf); return 0; }
> +#endif

The "fixed simpler version" version, or just reference configure.

> +
> +#define UTIL_ORDER_LITTLE_ENDIAN 4321
> +#define UTIL_ORDER_BIG_ENDIAN    1234

swap :D

> +
> +
> +#if !defined(UTIL_BYTE_ORDER) || ((UTIL_BYTE_ORDER != UTIL_ORDER_LITTLE_ENDIAN) && \
> +				   (UTIL_BYTE_ORDER != UTIL_ORDER_LITTLE_ENDIAN))

for "guess" this needs to be #if !UTIL_BYTE_ORDER || ((UTIL_BYTE...
... as UTIL_BYTE_ORDER could be defined as 0 (or empty?) in configure.

$ gcc -dM -DUTIL_BYTE_ORDER= -E - </dev/null | grep -i util
#define UTIL_BYTE_ORDER 


Tomi

> +#ifdef __BYTE_ORDER__
> +#  if (__BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__)
> +#    define UTIL_BYTE_ORDER UTIL_ORDER_LITTLE_ENDIAN
> +#  elif (__BYTE_ORDER__ == __ORDER_BIG_ENDIAN__)
> +#    define UTIL_BYTE_ORDER UTIL_ORDER_BIG_ENDIAN
> +#  else
> +#    error "Unsupported __BYTE_ORDER__"
> +#  endif
> +#else
> +#  error "UTIL_BYTE_ORDER not correctly defined and __BYTE_ORDER__ not defined."
> +#endif
> +#endif
> +
> +#endif
> -- 
> 1.8.4.2

[PATCH v2] util: detect byte order

[david]

From: David Bremner <david@tethera.net>

Unfortunately old versions of GCC and clang do not provide byte order
macros, so we re-invent them.

If UTIL_BYTE_ORDER is not defined or defined to 0, we fall back to
macros supported by recent versions of GCC and clang
---

I think I got all of Tomi's comments, including the most serious one
about the test in endian-utils.h (LITTLE twice instead of BIG,
LITTLE).

 configure          | 24 ++++++++++++++++++++++--
 lib/libsha1.c      | 21 +++++++--------------
 util/endian-util.h | 38 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 67 insertions(+), 16 deletions(-)
 create mode 100644 util/endian-util.h

diff --git a/configure b/configure
index 1a8e939..a0c6771 100755
--- a/configure
+++ b/configure
@@ -441,6 +441,19 @@ else
 EOF
 fi
 
+printf "Checking byte order... "
+cat> _byteorder.c <<EOF
+#include <stdio.h>
+#include <stdint.h>
+uint32_t test = 0x34333231;
+int main() { printf("%.4s\n", (const char*)&test); return 0; }
+EOF
+${CC} ${CFLAGS} _byteorder.c -o _byteorder > /dev/null 2>&1
+util_byte_order=$(./_byteorder)
+echo $util_byte_order
+
+#rm -f _byteorder _byteorder.c
+
 if [ $errors -gt 0 ]; then
     cat <<EOF
 
@@ -702,6 +715,9 @@ prefix = ${PREFIX}
 # LIBDIR_IN_LDCONFIG value below is still set correctly.
 libdir = ${LIBDIR:=\$(prefix)/lib}
 
+# byte order within a 32 bit word. 1234 = little, 4321 = big, 0 = guess
+UTIL_BYTE_ORDER = ${util_byte_order}
+
 # Whether libdir is in a path configured into ldconfig
 LIBDIR_IN_LDCONFIG = ${libdir_in_ldconfig}
 
@@ -807,7 +823,9 @@ CONFIGURE_CFLAGS = -DHAVE_GETLINE=\$(HAVE_GETLINE) \$(GMIME_CFLAGS)      \\
 		   -DHAVE_STRSEP=\$(HAVE_STRSEP)                         \\
 		   -DSTD_GETPWUID=\$(STD_GETPWUID)                       \\
 		   -DSTD_ASCTIME=\$(STD_ASCTIME)                         \\
-		   -DHAVE_XAPIAN_COMPACT=\$(HAVE_XAPIAN_COMPACT)
+		   -DHAVE_XAPIAN_COMPACT=\$(HAVE_XAPIAN_COMPACT)	 \\
+		   -DUTIL_BYTE_ORDER=\$(UTIL_BYTE_ORDER)
+
 CONFIGURE_CXXFLAGS = -DHAVE_GETLINE=\$(HAVE_GETLINE) \$(GMIME_CFLAGS)    \\
 		     \$(TALLOC_CFLAGS) -DHAVE_VALGRIND=\$(HAVE_VALGRIND) \\
 		     \$(VALGRIND_CFLAGS) \$(XAPIAN_CXXFLAGS)             \\
@@ -815,6 +833,8 @@ CONFIGURE_CXXFLAGS = -DHAVE_GETLINE=\$(HAVE_GETLINE) \$(GMIME_CFLAGS)    \\
 		     -DHAVE_STRSEP=\$(HAVE_STRSEP)                       \\
 		     -DSTD_GETPWUID=\$(STD_GETPWUID)                     \\
 		     -DSTD_ASCTIME=\$(STD_ASCTIME)                       \\
-		     -DHAVE_XAPIAN_COMPACT=\$(HAVE_XAPIAN_COMPACT)
+		     -DHAVE_XAPIAN_COMPACT=\$(HAVE_XAPIAN_COMPACT)       \\
+		     -DUTIL_BYTE_ORDER=\$(UTIL_BYTE_ORDER)
+
 CONFIGURE_LDFLAGS =  \$(GMIME_LDFLAGS) \$(TALLOC_LDFLAGS) \$(XAPIAN_LDFLAGS)
 EOF
diff --git a/lib/libsha1.c b/lib/libsha1.c
index 87c7c52..aaaa4eb 100644
--- a/lib/libsha1.c
+++ b/lib/libsha1.c
@@ -34,7 +34,7 @@
 */
 
 #include <string.h>     /* for memcpy() etc.        */
-
+#include "endian-util.h"
 #include "libsha1.h"
 
 #if defined(__cplusplus)
@@ -49,20 +49,13 @@ extern "C"
 
 #define bswap_32(x) ((rotr32((x), 24) & 0x00ff00ff) | (rotr32((x), 8) & 0xff00ff00))
 
-/* The macros __BYTE_ORDER__ and __ORDER_*_ENDIAN__ are GNU C
- * extensions. They are also supported by clang as of v3.2 */
-
-#ifdef __BYTE_ORDER__
-#  if (__BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__)
-#    define bsw_32(p,n) \
-       { int _i = (n); while(_i--) ((uint32_t*)p)[_i] = bswap_32(((uint32_t*)p)[_i]); }
-#  elif (__BYTE_ORDER__ == __ORDER_BIG_ENDIAN__)
-#    define bsw_32(p,n)
-#  else
-#    error "unknown byte order"
-#  endif
+#if (UTIL_BYTE_ORDER == UTIL_ORDER_LITTLE_ENDIAN)
+#  define bsw_32(p,n) \
+     { int _i = (n); while(_i--) ((uint32_t*)p)[_i] = bswap_32(((uint32_t*)p)[_i]); }
+#elif (UTIL_BYTE_ORDER == UTIL_ORDER_BIG_ENDIAN)
+#  define bsw_32(p,n)
 #else
-#    error "macro __BYTE_ORDER__ is not defined"
+#  error "Unsupported byte order"
 #endif
 
 #define SHA1_MASK   (SHA1_BLOCK_SIZE - 1)
diff --git a/util/endian-util.h b/util/endian-util.h
new file mode 100644
index 0000000..bc80c40
--- /dev/null
+++ b/util/endian-util.h
@@ -0,0 +1,38 @@
+/* this file mimics the macros present in recent GCC and CLANG */
+
+#ifndef _ENDIAN_UTIL_H
+#define _ENDIAN_UTIL_H
+
+/* This are prefixed with UTIL to avoid collisions
+ *
+ * You can use something like the following to define UTIL_BYTE_ORDER
+ * in a configure script.
+ */
+#if 0
+#include <stdio.h>
+#include <stdint.h>
+uint32_t test = 0x34333231;
+int main() { printf("%.4s\n", (const char*)&test); return 0; }
+#endif
+
+#define UTIL_ORDER_BIG_ENDIAN	  4321
+#define UTIL_ORDER_LITTLE_ENDIAN  1234
+
+
+#if !defined(UTIL_BYTE_ORDER) || ((UTIL_BYTE_ORDER != UTIL_ORDER_BIG_ENDIAN) && \
+				  (UTIL_BYTE_ORDER != UTIL_ORDER_LITTLE_ENDIAN))
+#undef UTIL_BYTE_ORDER
+#ifdef __BYTE_ORDER__
+#  if (__BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__)
+#    define UTIL_BYTE_ORDER UTIL_ORDER_LITTLE_ENDIAN
+#  elif (__BYTE_ORDER__ == __ORDER_BIG_ENDIAN__)
+#    define UTIL_BYTE_ORDER UTIL_ORDER_BIG_ENDIAN
+#  else
+#    error "Unsupported __BYTE_ORDER__"
+#  endif
+#else
+#  error "UTIL_BYTE_ORDER not correctly defined and __BYTE_ORDER__ not defined."
+#endif
+#endif
+
+#endif
-- 
1.8.4.2

Re: [PATCH v2] util: detect byte order

[Tomi Ollila]

On Wed, Nov 27 2013, david@tethera.net wrote:

> From: David Bremner <david@tethera.net>
>
> Unfortunately old versions of GCC and clang do not provide byte order
> macros, so we re-invent them.
>
> If UTIL_BYTE_ORDER is not defined or defined to 0, we fall back to
> macros supported by recent versions of GCC and clang
> ---
>
> I think I got all of Tomi's comments, including the most serious one
> about the test in endian-utils.h (LITTLE twice instead of BIG,
> LITTLE).

Agreed. LGTM.

You may want to amend the '#' out of the line

+#rm -f _byteorder _byteorder.c

before pushing ?

Tomi


>
>  configure          | 24 ++++++++++++++++++++++--
>  lib/libsha1.c      | 21 +++++++--------------
>  util/endian-util.h | 38 ++++++++++++++++++++++++++++++++++++++
>  3 files changed, 67 insertions(+), 16 deletions(-)
>  create mode 100644 util/endian-util.h
>
> diff --git a/configure b/configure
> index 1a8e939..a0c6771 100755
> --- a/configure
> +++ b/configure
> @@ -441,6 +441,19 @@ else
>  EOF
>  fi
>  
> +printf "Checking byte order... "
> +cat> _byteorder.c <<EOF
> +#include <stdio.h>
> +#include <stdint.h>
> +uint32_t test = 0x34333231;
> +int main() { printf("%.4s\n", (const char*)&test); return 0; }
> +EOF
> +${CC} ${CFLAGS} _byteorder.c -o _byteorder > /dev/null 2>&1
> +util_byte_order=$(./_byteorder)
> +echo $util_byte_order
> +
> +#rm -f _byteorder _byteorder.c
> +
>  if [ $errors -gt 0 ]; then
>      cat <<EOF
>  
> @@ -702,6 +715,9 @@ prefix = ${PREFIX}
>  # LIBDIR_IN_LDCONFIG value below is still set correctly.
>  libdir = ${LIBDIR:=\$(prefix)/lib}
>  
> +# byte order within a 32 bit word. 1234 = little, 4321 = big, 0 = guess
> +UTIL_BYTE_ORDER = ${util_byte_order}
> +
>  # Whether libdir is in a path configured into ldconfig
>  LIBDIR_IN_LDCONFIG = ${libdir_in_ldconfig}
>  
> @@ -807,7 +823,9 @@ CONFIGURE_CFLAGS = -DHAVE_GETLINE=\$(HAVE_GETLINE) \$(GMIME_CFLAGS)      \\
>  		   -DHAVE_STRSEP=\$(HAVE_STRSEP)                         \\
>  		   -DSTD_GETPWUID=\$(STD_GETPWUID)                       \\
>  		   -DSTD_ASCTIME=\$(STD_ASCTIME)                         \\
> -		   -DHAVE_XAPIAN_COMPACT=\$(HAVE_XAPIAN_COMPACT)
> +		   -DHAVE_XAPIAN_COMPACT=\$(HAVE_XAPIAN_COMPACT)	 \\
> +		   -DUTIL_BYTE_ORDER=\$(UTIL_BYTE_ORDER)
> +
>  CONFIGURE_CXXFLAGS = -DHAVE_GETLINE=\$(HAVE_GETLINE) \$(GMIME_CFLAGS)    \\
>  		     \$(TALLOC_CFLAGS) -DHAVE_VALGRIND=\$(HAVE_VALGRIND) \\
>  		     \$(VALGRIND_CFLAGS) \$(XAPIAN_CXXFLAGS)             \\
> @@ -815,6 +833,8 @@ CONFIGURE_CXXFLAGS = -DHAVE_GETLINE=\$(HAVE_GETLINE) \$(GMIME_CFLAGS)    \\
>  		     -DHAVE_STRSEP=\$(HAVE_STRSEP)                       \\
>  		     -DSTD_GETPWUID=\$(STD_GETPWUID)                     \\
>  		     -DSTD_ASCTIME=\$(STD_ASCTIME)                       \\
> -		     -DHAVE_XAPIAN_COMPACT=\$(HAVE_XAPIAN_COMPACT)
> +		     -DHAVE_XAPIAN_COMPACT=\$(HAVE_XAPIAN_COMPACT)       \\
> +		     -DUTIL_BYTE_ORDER=\$(UTIL_BYTE_ORDER)
> +
>  CONFIGURE_LDFLAGS =  \$(GMIME_LDFLAGS) \$(TALLOC_LDFLAGS) \$(XAPIAN_LDFLAGS)
>  EOF
> diff --git a/lib/libsha1.c b/lib/libsha1.c
> index 87c7c52..aaaa4eb 100644
> --- a/lib/libsha1.c
> +++ b/lib/libsha1.c
> @@ -34,7 +34,7 @@
>  */
>  
>  #include <string.h>     /* for memcpy() etc.        */
> -
> +#include "endian-util.h"
>  #include "libsha1.h"
>  
>  #if defined(__cplusplus)
> @@ -49,20 +49,13 @@ extern "C"
>  
>  #define bswap_32(x) ((rotr32((x), 24) & 0x00ff00ff) | (rotr32((x), 8) & 0xff00ff00))
>  
> -/* The macros __BYTE_ORDER__ and __ORDER_*_ENDIAN__ are GNU C
> - * extensions. They are also supported by clang as of v3.2 */
> -
> -#ifdef __BYTE_ORDER__
> -#  if (__BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__)
> -#    define bsw_32(p,n) \
> -       { int _i = (n); while(_i--) ((uint32_t*)p)[_i] = bswap_32(((uint32_t*)p)[_i]); }
> -#  elif (__BYTE_ORDER__ == __ORDER_BIG_ENDIAN__)
> -#    define bsw_32(p,n)
> -#  else
> -#    error "unknown byte order"
> -#  endif
> +#if (UTIL_BYTE_ORDER == UTIL_ORDER_LITTLE_ENDIAN)
> +#  define bsw_32(p,n) \
> +     { int _i = (n); while(_i--) ((uint32_t*)p)[_i] = bswap_32(((uint32_t*)p)[_i]); }
> +#elif (UTIL_BYTE_ORDER == UTIL_ORDER_BIG_ENDIAN)
> +#  define bsw_32(p,n)
>  #else
> -#    error "macro __BYTE_ORDER__ is not defined"
> +#  error "Unsupported byte order"
>  #endif
>  
>  #define SHA1_MASK   (SHA1_BLOCK_SIZE - 1)
> diff --git a/util/endian-util.h b/util/endian-util.h
> new file mode 100644
> index 0000000..bc80c40
> --- /dev/null
> +++ b/util/endian-util.h
> @@ -0,0 +1,38 @@
> +/* this file mimics the macros present in recent GCC and CLANG */
> +
> +#ifndef _ENDIAN_UTIL_H
> +#define _ENDIAN_UTIL_H
> +
> +/* This are prefixed with UTIL to avoid collisions
> + *
> + * You can use something like the following to define UTIL_BYTE_ORDER
> + * in a configure script.
> + */
> +#if 0
> +#include <stdio.h>
> +#include <stdint.h>
> +uint32_t test = 0x34333231;
> +int main() { printf("%.4s\n", (const char*)&test); return 0; }
> +#endif
> +
> +#define UTIL_ORDER_BIG_ENDIAN	  4321
> +#define UTIL_ORDER_LITTLE_ENDIAN  1234
> +
> +
> +#if !defined(UTIL_BYTE_ORDER) || ((UTIL_BYTE_ORDER != UTIL_ORDER_BIG_ENDIAN) && \
> +				  (UTIL_BYTE_ORDER != UTIL_ORDER_LITTLE_ENDIAN))
> +#undef UTIL_BYTE_ORDER
> +#ifdef __BYTE_ORDER__
> +#  if (__BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__)
> +#    define UTIL_BYTE_ORDER UTIL_ORDER_LITTLE_ENDIAN
> +#  elif (__BYTE_ORDER__ == __ORDER_BIG_ENDIAN__)
> +#    define UTIL_BYTE_ORDER UTIL_ORDER_BIG_ENDIAN
> +#  else
> +#    error "Unsupported __BYTE_ORDER__"
> +#  endif
> +#else
> +#  error "UTIL_BYTE_ORDER not correctly defined and __BYTE_ORDER__ not defined."
> +#endif
> +#endif
> +
> +#endif
> -- 
> 1.8.4.2
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH v2] util: detect byte order

[David Bremner]

Tomi Ollila <tomi.ollila@iki.fi> writes:
>
> You may want to amend the '#' out of the line
>
> +#rm -f _byteorder _byteorder.c
>

done and pushed.

d