From: Tomi Ollila <tomi.ollila@iki.fi>
To: david@tethera.net, notmuch@notmuchmail.org
Subject: Re: [PATCH 1/2] lib: fix byte order test in libsha1.c
In-Reply-To: <1385328583-24602-1-git-send-email-david@tethera.net>
References: <m2ob593g55.fsf@guru.guru-group.fi>
	<1385328583-24602-1-git-send-email-david@tethera.net>
User-Agent: Notmuch/0.16+175~g19e97d6 (http://notmuchmail.org) Emacs/24.3.1
	(x86_64-unknown-linux-gnu)
Date: Mon, 25 Nov 2013 11:43:37 +0200
Message-ID: <m2mwkserzq.fsf@guru.guru-group.fi>
MIME-Version: 1.0
Content-Type: text/plain
Precedence: list

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