Patchwork BYTE_ORDER

login
register
mail settings
Submitter Per Ekman
Date June 18, 2012, 9:40 a.m.
Message ID <87d34xufdk.fsf@hd-wireless.se>
Download mbox | patch
Permalink /patch/165439/
State Changes Requested
Headers show

Comments

Per Ekman - June 18, 2012, 9:40 a.m.
Jouni Malinen <j@w1.fi> writes:

> On Tue, Jun 12, 2012 at 03:36:37PM +0200, pek@kth.se wrote:
>> Whats the story with the *BYTE_ORDER macros?
>
> Long history and multiple different places where these could be set..
>
>> crypto/md4-internal.c checks BYTE_ORDER, is this a bug?
>> It is unclear to me if this is ever defined in the general case.
>
> This is defined by endian.h (from C library).. 

Is endian.h guaranteed to define BYTE_ORDER (or even exist)?

> Could be cleaner to change that to use __BYTE_ORDER to match with the
> eap_peer/ikev2.c use or move to WORDS_BIGENDIAN which is used in some
> more places.

The reason I ask is that when porting to a new environment it's not
obvious that endian.h is a requirement (since it is conditionally
included in common.h). Just looking at common.h makes it look like you
just define __BYTE_ORDER and be on your way but grepping the code makes
me think that I should define BYTE_ORDER, _BYTE_ORDER and __BYTE_ORDER,
which feels somewhat ad-hoc (and I guess it is).

>> driver_atheros.c can define _BYTE_ORDER (but doesn't use it). It doesn't
>> feel like this file should diddle around with these macros at all.
>
> Old hack..
>
>> It seems to the uninitiated that __BYTE_ORDER is the macro that all code
>> is intended to use since common.h defines it. Is this correct?
>
> Well, not necessarily.. This is a question of whether to use the
> __BYTE_ORDER from system headers (with possible compatibility code from
> common.h) or to move something specific to hostap.git (like
> WORDS_BIGENDIAN). I'm not aware of places where the current
> implementation would not work, but well, it could certainly be cleaned
> up.

It would clearly work as long as the person doing the porting was
sufficiently enlightened :) 

WORDS_BIGENDIAN sounds much nicer to me.
Jouni Malinen - June 30, 2012, 1:04 p.m.
On Mon, Jun 18, 2012 at 11:40:55AM +0200, Per Ekman wrote:
> Is endian.h guaranteed to define BYTE_ORDER (or even exist)?

Probably not.

> The reason I ask is that when porting to a new environment it's not
> obvious that endian.h is a requirement (since it is conditionally
> included in common.h). Just looking at common.h makes it look like you
> just define __BYTE_ORDER and be on your way but grepping the code makes
> me think that I should define BYTE_ORDER, _BYTE_ORDER and __BYTE_ORDER,
> which feels somewhat ad-hoc (and I guess it is).

endian.h is not a requirement; defining some macros to indicate the byte
order is.

> WORDS_BIGENDIAN sounds much nicer to me.

> From: Per Ekman <pek@boblet.(none)>
> Date: Mon, 18 Jun 2012 11:26:38 +0200
> Subject: [PATCH] Replace references to _*BYTE_ORDER with WORDS_BIGENDIAN

Could you please read the CONTRIBUTIONS file in the top directory and
send the patch with the Signed-hostap: tag?

> -#if BYTE_ORDER == LITTLE_ENDIAN
> -	os_memcpy(in, block, sizeof(in));
> -#else
> +#ifdef WORDS_BIGENDIAN

#ifndef would avoid the need to move the actual code and keep the patch
cleaner.

Patch

From 8f4fe87ec97a8f4501f29885e09008794f15338f Mon Sep 17 00:00:00 2001
From: Per Ekman <pek@boblet.(none)>
Date: Mon, 18 Jun 2012 11:26:38 +0200
Subject: [PATCH] Replace references to _*BYTE_ORDER with WORDS_BIGENDIAN

---
 src/crypto/md4-internal.c |    6 +++---
 src/eap_peer/ikev2.c      |    2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/crypto/md4-internal.c b/src/crypto/md4-internal.c
index cd5e6ca..a3afc29 100644
--- a/src/crypto/md4-internal.c
+++ b/src/crypto/md4-internal.c
@@ -196,9 +196,7 @@  MD4Transform(u32 state[4], const u8 block[MD4_BLOCK_LENGTH])
 {
 	u32 a, b, c, d, in[MD4_BLOCK_LENGTH / 4];
 
-#if BYTE_ORDER == LITTLE_ENDIAN
-	os_memcpy(in, block, sizeof(in));
-#else
+#ifdef WORDS_BIGENDIAN
 	for (a = 0; a < MD4_BLOCK_LENGTH / 4; a++) {
 		in[a] = (u32)(
 		    (u32)(block[a * 4 + 0]) |
@@ -206,6 +204,8 @@  MD4Transform(u32 state[4], const u8 block[MD4_BLOCK_LENGTH])
 		    (u32)(block[a * 4 + 2]) << 16 |
 		    (u32)(block[a * 4 + 3]) << 24);
 	}
+#else
+	os_memcpy(in, block, sizeof(in));
 #endif
 
 	a = state[0];
diff --git a/src/eap_peer/ikev2.c b/src/eap_peer/ikev2.c
index fcf4712..d663d78 100644
--- a/src/eap_peer/ikev2.c
+++ b/src/eap_peer/ikev2.c
@@ -73,7 +73,7 @@  static int ikev2_derive_keys(struct ikev2_responder_data *data)
 	pos += IKEV2_SPI_LEN;
 	os_memcpy(pos, data->r_spi, IKEV2_SPI_LEN);
 #ifdef CCNS_PL
-#if __BYTE_ORDER == __LITTLE_ENDIAN
+#ifndef WORDS_BIGENDIAN
 	{
 		int i;
 		u8 *tmp = pos - IKEV2_SPI_LEN;
-- 
1.7.0.4