[Mesa-dev] [PATCH 1/3] util: Fix SHA1 implementation on big endian

[Matt Turner]

On Fri, Nov 24, 2017 at 2:25 AM, Eric Engestrom
<eric.engestrom at imgtec.com> wrote:
> On Thursday, 2017-11-23 11:08:04 -0800, Matt Turner wrote:
>> The code defines a macro blk0(i) based on the preprocessor condition
>> BYTE_ORDER == LITTLE_ENDIAN. If true, blk0(i) is defined as a byte swap
>> operation. Unfortunately, if the preprocessor macros used in the test
>> are no defined, then the comparison becomes 0 == 0 and it evaluates as
>> true.
>> ---
>>  src/util/sha1/sha1.c | 9 +++++++++
>>  1 file changed, 9 insertions(+)
>>
>> diff --git a/src/util/sha1/sha1.c b/src/util/sha1/sha1.c
>> index ef59ea1dfc..2c629520c3 100644
>> --- a/src/util/sha1/sha1.c
>> +++ b/src/util/sha1/sha1.c
>> @@ -16,8 +16,17 @@
>>
>>  #include <stdint.h>
>>  #include <string.h>
>> +#include "u_endian.h"
>
> If you're including this header, why not use
> `#ifdef PIPE_ARCH_LITTLE_ENDIAN` instead of
> `#if BYTE_ORDER == LITTLE_ENDIAN`?
>
> `#error` becomes unnecessary

In response to Andres' report that the Windows build fails with this
patch, I was going to take your suggestion... but it doesn't seem like
u_endian.h has any code to handle Windows.

I see this crap in p_config.h:

/*
 * Endian detection.
 */

#include "util/u_endian.h"
#if !defined(PIPE_ARCH_LITTLE_ENDIAN) && !defined(PIPE_ARCH_BIG_ENDIAN)

#if defined(PIPE_ARCH_X86) || defined(PIPE_ARCH_X86_64) ||
defined(PIPE_ARCH_ARM) || defined(PIPE_ARCH_AARCH64)
#define PIPE_ARCH_LITTLE_ENDIAN
#elif defined(PIPE_ARCH_PPC) || defined(PIPE_ARCH_PPC_64) ||
defined(PIPE_ARCH_S390)
#define PIPE_ARCH_BIG_ENDIAN
#endif

#endif

#if !defined(PIPE_ARCH_LITTLE_ENDIAN) && !defined(PIPE_ARCH_BIG_ENDIAN)
#error Unknown Endianness
#endif

I don't think *any* of that would be necessary if we just handled
Windows (in the appropriate place -- u_endian.h)!

José, Roland: I think I'm going to commit a patch to u_endian.h to
define PIPE_ARCH_LITTLE_ENDIAN in the absense of any platform-specific
handling, so that the rest of my series can land:

diff --git a/src/util/u_endian.h b/src/util/u_endian.h
index 7bbd7dc215..3d5c006f35 100644
--- a/src/util/u_endian.h
+++ b/src/util/u_endian.h
@@ -67,4 +67,7 @@

 #endif

+#warn Unknown Endianness for this platform. Assuming little endian
+#define PIPE_ARCH_LITTLE_ENDIAN
+
 #endif

Presumably Windows only runs on little endian platforms, so that
should be fine for your purposes.