diff mbox

[1/4] include/odp_byteorder: endianess compatible with -std=c99

Message ID 1398428077-32199-2-git-send-email-anders.roxell@linaro.org
State Superseded
Headers show

Commit Message

Anders Roxell April 25, 2014, 12:14 p.m. UTC
Signed-off-by: Anders Roxell <anders.roxell@linaro.org>
---
 include/odp_byteorder.h | 42 ++++++++++++++++--------------------------
 1 file changed, 16 insertions(+), 26 deletions(-)

Comments

Maxim Uvarov April 28, 2014, 11:57 a.m. UTC | #1
On 04/25/2014 04:14 PM, Anders Roxell wrote:
> -/** Selected byte order */
> -#if BYTE_ORDER == LITTLE_ENDIAN
> -#define ODP_BYTE_ORDER ODP_LITTLE_ENDIAN
> -#elif BYTE_ORDER == BIG_ENDIAN
> -#define ODP_BYTE_ORDER ODP_BIG_ENDIAN
> +#if __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__
> +#define ODP_LITTLE_ENDIAN __ORDER_LITTLE_ENDIAN__
> +#else
> +#define ODP_BIG_ENDIAN    __ORDER_BIG_ENDIAN__
>   #endif

This might be simple:
#define ODP_BIG_ENDIAN        __ORDER_BIG_ENDIAN__
#define ODP_LITTLE_ENDIAN    __ORDER_LITTLE_ENDIAN__

Thanks,
Maxim.
Taras Kondratiuk April 28, 2014, 2:08 p.m. UTC | #2
On 04/28/2014 02:57 PM, Maxim Uvarov wrote:
> On 04/25/2014 04:14 PM, Anders Roxell wrote:
>> -/** Selected byte order */
>> -#if BYTE_ORDER == LITTLE_ENDIAN
>> -#define ODP_BYTE_ORDER ODP_LITTLE_ENDIAN
>> -#elif BYTE_ORDER == BIG_ENDIAN
>> -#define ODP_BYTE_ORDER ODP_BIG_ENDIAN
>> +#if __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__
>> +#define ODP_LITTLE_ENDIAN __ORDER_LITTLE_ENDIAN__
>> +#else
>> +#define ODP_BIG_ENDIAN    __ORDER_BIG_ENDIAN__
>>   #endif
>
> This might be simple:
> #define ODP_BIG_ENDIAN        __ORDER_BIG_ENDIAN__
> #define ODP_LITTLE_ENDIAN    __ORDER_LITTLE_ENDIAN__

Anders' changes assume that only one of the is defined.
So this won't work.
Taras Kondratiuk April 28, 2014, 2:38 p.m. UTC | #3
On 04/25/2014 03:14 PM, Anders Roxell wrote:
> Signed-off-by: Anders Roxell <anders.roxell@linaro.org>
> ---
>   include/odp_byteorder.h | 42 ++++++++++++++++--------------------------
>   1 file changed, 16 insertions(+), 26 deletions(-)
>
> diff --git a/include/odp_byteorder.h b/include/odp_byteorder.h
> index c573bb6..4b35da9 100644
> --- a/include/odp_byteorder.h
> +++ b/include/odp_byteorder.h
> @@ -22,21 +22,11 @@ extern "C" {
>   #include <odp_std_types.h>
>   #include <odp_compiler.h>
>
> -#ifndef BYTE_ORDER
> -#error BYTE_ORDER not defined!
> -#endif
> -
> -/** Big endian byte order */
> -#define ODP_BIG_ENDIAN    BIG_ENDIAN
>
> -/** Little endian byte order */
> -#define ODP_LITTLE_ENDIAN LITTLE_ENDIAN
> -
> -/** Selected byte order */
> -#if BYTE_ORDER == LITTLE_ENDIAN
> -#define ODP_BYTE_ORDER ODP_LITTLE_ENDIAN
> -#elif BYTE_ORDER == BIG_ENDIAN
> -#define ODP_BYTE_ORDER ODP_BIG_ENDIAN
> +#if __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__
> +#define ODP_LITTLE_ENDIAN __ORDER_LITTLE_ENDIAN__
> +#else
> +#define ODP_BIG_ENDIAN    __ORDER_BIG_ENDIAN__
>   #endif

You are assuming here that __BYTE_ORDER__ can have only two values.
What about __ORDER_PDP_ENDIAN__ ? An error should be thrown if 
__BYTE_ORDER__ is not one of supported endianness types.

> @@ -75,7 +65,7 @@ typedef uint64_t __odp_bitwise	uint64be_t; /**< unsigned 64bit big endian */
>    */
>   static inline uint16_t odp_be_to_cpu_16(uint16be_t be16)
>   {
> -#if ODP_BYTE_ORDER == ODP_LITTLE_ENDIAN
> +#if ODP_LITTLE_ENDIAN
>   	return __odp_builtin_bswap16((__odp_force uint16_t)be16);
>   #else
>   	return (__odp_force uint16_t)be16;

#else may be conditional if we are going to support more then two 
endianness variants. For example

#if ODP_BYTE_ORDER == ODP_LITTLE_ENDIAN
/* little-endian */
#elif ODP_BYTE_ORDER == ODP_BIG_ENDIAN
/* big-endian */
#elif ODP_BYTE_ORDER == ODP_PDP_ENDIAN
/* PDP or some kind of another weird endianness */
#endif
Anders Roxell April 28, 2014, 5:32 p.m. UTC | #4
On 28 April 2014 16:38, Taras Kondratiuk <taras.kondratiuk@linaro.org>wrote:

> On 04/25/2014 03:14 PM, Anders Roxell wrote:
>
>> Signed-off-by: Anders Roxell <anders.roxell@linaro.org>
>> ---
>>   include/odp_byteorder.h | 42 ++++++++++++++++--------------------------
>>   1 file changed, 16 insertions(+), 26 deletions(-)
>>
>> diff --git a/include/odp_byteorder.h b/include/odp_byteorder.h
>> index c573bb6..4b35da9 100644
>> --- a/include/odp_byteorder.h
>> +++ b/include/odp_byteorder.h
>> @@ -22,21 +22,11 @@ extern "C" {
>>   #include <odp_std_types.h>
>>   #include <odp_compiler.h>
>>
>> -#ifndef BYTE_ORDER
>> -#error BYTE_ORDER not defined!
>> -#endif
>> -
>> -/** Big endian byte order */
>> -#define ODP_BIG_ENDIAN    BIG_ENDIAN
>>
>> -/** Little endian byte order */
>> -#define ODP_LITTLE_ENDIAN LITTLE_ENDIAN
>> -
>> -/** Selected byte order */
>> -#if BYTE_ORDER == LITTLE_ENDIAN
>> -#define ODP_BYTE_ORDER ODP_LITTLE_ENDIAN
>> -#elif BYTE_ORDER == BIG_ENDIAN
>> -#define ODP_BYTE_ORDER ODP_BIG_ENDIAN
>> +#if __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__
>> +#define ODP_LITTLE_ENDIAN __ORDER_LITTLE_ENDIAN__
>> +#else
>> +#define ODP_BIG_ENDIAN    __ORDER_BIG_ENDIAN__
>>   #endif
>>
>
> You are assuming here that __BYTE_ORDER__ can have only two values.
> What about __ORDER_PDP_ENDIAN__ ? An error should be thrown if
> __BYTE_ORDER__ is not one of supported endianness types.
>
>
>  @@ -75,7 +65,7 @@ typedef uint64_t __odp_bitwise        uint64be_t; /**<
>> unsigned 64bit big endian */
>>    */
>>   static inline uint16_t odp_be_to_cpu_16(uint16be_t be16)
>>   {
>> -#if ODP_BYTE_ORDER == ODP_LITTLE_ENDIAN
>> +#if ODP_LITTLE_ENDIAN
>>         return __odp_builtin_bswap16((__odp_force uint16_t)be16);
>>   #else
>>         return (__odp_force uint16_t)be16;
>>
>
> #else may be conditional if we are going to support more then two
> endianness variants. For example
>
> #if ODP_BYTE_ORDER == ODP_LITTLE_ENDIAN
> /* little-endian */
> #elif ODP_BYTE_ORDER == ODP_BIG_ENDIAN
> /* big-endian */
> #elif ODP_BYTE_ORDER == ODP_PDP_ENDIAN
> /* PDP or some kind of another weird endianness */
> #endif
>
> --
> Taras Kondratiuk
>


Thank you for reviewing this Taras.

I will send out v2 soon.

Cheers,
Anders
Ola Liljedahl April 28, 2014, 10:31 p.m. UTC | #5
If __BYTE_ORDER__, __ORDER_LITTLE_ENDIAN__ and __ORDER_BIG_ENDIAN__
are standard defines, do we need to define our own in ODP? Feels
redundant to me.

-- Ola


On 28 April 2014 10:32, Anders Roxell <anders.roxell@linaro.org> wrote:
>
>
> On 28 April 2014 16:38, Taras Kondratiuk <taras.kondratiuk@linaro.org>
> wrote:
>>
>> On 04/25/2014 03:14 PM, Anders Roxell wrote:
>>>
>>> Signed-off-by: Anders Roxell <anders.roxell@linaro.org>
>>> ---
>>>   include/odp_byteorder.h | 42 ++++++++++++++++--------------------------
>>>   1 file changed, 16 insertions(+), 26 deletions(-)
>>>
>>> diff --git a/include/odp_byteorder.h b/include/odp_byteorder.h
>>> index c573bb6..4b35da9 100644
>>> --- a/include/odp_byteorder.h
>>> +++ b/include/odp_byteorder.h
>>> @@ -22,21 +22,11 @@ extern "C" {
>>>   #include <odp_std_types.h>
>>>   #include <odp_compiler.h>
>>>
>>> -#ifndef BYTE_ORDER
>>> -#error BYTE_ORDER not defined!
>>> -#endif
>>> -
>>> -/** Big endian byte order */
>>> -#define ODP_BIG_ENDIAN    BIG_ENDIAN
>>>
>>> -/** Little endian byte order */
>>> -#define ODP_LITTLE_ENDIAN LITTLE_ENDIAN
>>> -
>>> -/** Selected byte order */
>>> -#if BYTE_ORDER == LITTLE_ENDIAN
>>> -#define ODP_BYTE_ORDER ODP_LITTLE_ENDIAN
>>> -#elif BYTE_ORDER == BIG_ENDIAN
>>> -#define ODP_BYTE_ORDER ODP_BIG_ENDIAN
>>> +#if __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__
>>> +#define ODP_LITTLE_ENDIAN __ORDER_LITTLE_ENDIAN__
>>> +#else
>>> +#define ODP_BIG_ENDIAN    __ORDER_BIG_ENDIAN__
>>>   #endif
>>
>>
>> You are assuming here that __BYTE_ORDER__ can have only two values.
>> What about __ORDER_PDP_ENDIAN__ ? An error should be thrown if
>> __BYTE_ORDER__ is not one of supported endianness types.
>>
>>
>>> @@ -75,7 +65,7 @@ typedef uint64_t __odp_bitwise        uint64be_t; /**<
>>> unsigned 64bit big endian */
>>>    */
>>>   static inline uint16_t odp_be_to_cpu_16(uint16be_t be16)
>>>   {
>>> -#if ODP_BYTE_ORDER == ODP_LITTLE_ENDIAN
>>> +#if ODP_LITTLE_ENDIAN
>>>         return __odp_builtin_bswap16((__odp_force uint16_t)be16);
>>>   #else
>>>         return (__odp_force uint16_t)be16;
>>
>>
>> #else may be conditional if we are going to support more then two
>> endianness variants. For example
>>
>> #if ODP_BYTE_ORDER == ODP_LITTLE_ENDIAN
>> /* little-endian */
>> #elif ODP_BYTE_ORDER == ODP_BIG_ENDIAN
>> /* big-endian */
>> #elif ODP_BYTE_ORDER == ODP_PDP_ENDIAN
>> /* PDP or some kind of another weird endianness */
>> #endif
>>
>> --
>> Taras Kondratiuk
>
>
>
> Thank you for reviewing this Taras.
>
> I will send out v2 soon.
>
> Cheers,
> Anders
>
> _______________________________________________
> lng-odp mailing list
> lng-odp@lists.linaro.org
> http://lists.linaro.org/mailman/listinfo/lng-odp
>
Taras Kondratiuk April 29, 2014, 8:43 a.m. UTC | #6
On 04/29/2014 01:31 AM, Ola Liljedahl wrote:
> If __BYTE_ORDER__, __ORDER_LITTLE_ENDIAN__ and __ORDER_BIG_ENDIAN__
> are standard defines, do we need to define our own in ODP? Feels
> redundant to me.

They are standard for GCC.
Savolainen, Petri (NSN - FI/Espoo) April 29, 2014, 8:57 a.m. UTC | #7
> -----Original Message-----
> From: lng-odp-bounces@lists.linaro.org [mailto:lng-odp-
> bounces@lists.linaro.org] On Behalf Of ext Taras Kondratiuk
> Sent: Tuesday, April 29, 2014 11:43 AM
> To: Ola Liljedahl; Anders Roxell
> Cc: lng-odp-forward
> Subject: Re: [lng-odp] [PATCH 1/4] include/odp_byteorder: endianess
> compatible with -std=c99
> 
> On 04/29/2014 01:31 AM, Ola Liljedahl wrote:
> > If __BYTE_ORDER__, __ORDER_LITTLE_ENDIAN__ and __ORDER_BIG_ENDIAN__
> > are standard defines, do we need to define our own in ODP? Feels
> > redundant to me.
> 
> They are standard for GCC.
> 
> --
> Taras Kondratiuk
> 


ODP defines are needed, if it's not standard for C99.


-Petri
Mike Holmes April 29, 2014, 3:38 p.m. UTC | #8
I will generate an ARCH doc page stating our rules for defines and we can
capture what we expect for ODP.  If any one has any text for our use of
typedef, etc or other C99 issues etc I will add them to the v1 patch for
the document page.

Mike


On 29 April 2014 01:57, Savolainen, Petri (NSN - FI/Espoo) <
petri.savolainen@nsn.com> wrote:

>
>
> > -----Original Message-----
> > From: lng-odp-bounces@lists.linaro.org [mailto:lng-odp-
> > bounces@lists.linaro.org] On Behalf Of ext Taras Kondratiuk
> > Sent: Tuesday, April 29, 2014 11:43 AM
> > To: Ola Liljedahl; Anders Roxell
> > Cc: lng-odp-forward
> > Subject: Re: [lng-odp] [PATCH 1/4] include/odp_byteorder: endianess
> > compatible with -std=c99
> >
> > On 04/29/2014 01:31 AM, Ola Liljedahl wrote:
> > > If __BYTE_ORDER__, __ORDER_LITTLE_ENDIAN__ and __ORDER_BIG_ENDIAN__
> > > are standard defines, do we need to define our own in ODP? Feels
> > > redundant to me.
> >
> > They are standard for GCC.
> >
> > --
> > Taras Kondratiuk
> >
>
>
> ODP defines are needed, if it's not standard for C99.
>
>
> -Petri
>
> _______________________________________________
> lng-odp mailing list
> lng-odp@lists.linaro.org
> http://lists.linaro.org/mailman/listinfo/lng-odp
>
diff mbox

Patch

diff --git a/include/odp_byteorder.h b/include/odp_byteorder.h
index c573bb6..4b35da9 100644
--- a/include/odp_byteorder.h
+++ b/include/odp_byteorder.h
@@ -22,21 +22,11 @@  extern "C" {
 #include <odp_std_types.h>
 #include <odp_compiler.h>
 
-#ifndef BYTE_ORDER
-#error BYTE_ORDER not defined!
-#endif
-
-/** Big endian byte order */
-#define ODP_BIG_ENDIAN    BIG_ENDIAN
 
-/** Little endian byte order */
-#define ODP_LITTLE_ENDIAN LITTLE_ENDIAN
-
-/** Selected byte order */
-#if BYTE_ORDER == LITTLE_ENDIAN
-#define ODP_BYTE_ORDER ODP_LITTLE_ENDIAN
-#elif BYTE_ORDER == BIG_ENDIAN
-#define ODP_BYTE_ORDER ODP_BIG_ENDIAN
+#if __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__
+#define ODP_LITTLE_ENDIAN __ORDER_LITTLE_ENDIAN__
+#else
+#define ODP_BIG_ENDIAN    __ORDER_BIG_ENDIAN__
 #endif
 
 
@@ -75,7 +65,7 @@  typedef uint64_t __odp_bitwise	uint64be_t; /**< unsigned 64bit big endian */
  */
 static inline uint16_t odp_be_to_cpu_16(uint16be_t be16)
 {
-#if ODP_BYTE_ORDER == ODP_LITTLE_ENDIAN
+#if ODP_LITTLE_ENDIAN
 	return __odp_builtin_bswap16((__odp_force uint16_t)be16);
 #else
 	return (__odp_force uint16_t)be16;
@@ -89,7 +79,7 @@  static inline uint16_t odp_be_to_cpu_16(uint16be_t be16)
  */
 static inline uint32_t odp_be_to_cpu_32(uint32be_t be32)
 {
-#if ODP_BYTE_ORDER == ODP_LITTLE_ENDIAN
+#if ODP_LITTLE_ENDIAN
 	return __builtin_bswap32((__odp_force uint32_t)be32);
 #else
 	return (__odp_force uint32_t)be32;
@@ -103,7 +93,7 @@  static inline uint32_t odp_be_to_cpu_32(uint32be_t be32)
  */
 static inline uint64_t odp_be_to_cpu_64(uint64be_t be64)
 {
-#if ODP_BYTE_ORDER == ODP_LITTLE_ENDIAN
+#if ODP_LITTLE_ENDIAN
 	return __builtin_bswap64((__odp_force uint64_t)be64);
 #else
 	return (__odp_force uint64_t)be64;
@@ -122,7 +112,7 @@  static inline uint64_t odp_be_to_cpu_64(uint64be_t be64)
  */
 static inline uint16be_t odp_cpu_to_be_16(uint16_t cpu16)
 {
-#if ODP_BYTE_ORDER == ODP_LITTLE_ENDIAN
+#if ODP_LITTLE_ENDIAN
 	return (__odp_force uint16be_t)__odp_builtin_bswap16(cpu16);
 #else
 	return (__odp_force uint16be_t)cpu16;
@@ -136,7 +126,7 @@  static inline uint16be_t odp_cpu_to_be_16(uint16_t cpu16)
  */
 static inline uint32be_t odp_cpu_to_be_32(uint32_t cpu32)
 {
-#if ODP_BYTE_ORDER == ODP_LITTLE_ENDIAN
+#if ODP_LITTLE_ENDIAN
 	return (__odp_force uint32be_t)__builtin_bswap32(cpu32);
 #else
 	return (__odp_force uint32be_t)cpu32;
@@ -150,7 +140,7 @@  static inline uint32be_t odp_cpu_to_be_32(uint32_t cpu32)
  */
 static inline uint64be_t odp_cpu_to_be_64(uint64_t cpu64)
 {
-#if ODP_BYTE_ORDER == ODP_LITTLE_ENDIAN
+#if ODP_LITTLE_ENDIAN
 	return (__odp_force uint64be_t)__builtin_bswap64(cpu64);
 #else
 	return (__odp_force uint64be_t)cpu64;
@@ -169,7 +159,7 @@  static inline uint64be_t odp_cpu_to_be_64(uint64_t cpu64)
  */
 static inline uint16_t odp_le_to_cpu_16(uint16le_t le16)
 {
-#if ODP_BYTE_ORDER == ODP_LITTLE_ENDIAN
+#if ODP_LITTLE_ENDIAN
 	return (__odp_force uint16_t)le16;
 #else
 	return __odp_builtin_bswap16((__odp_force uint16_t)le16);
@@ -183,7 +173,7 @@  static inline uint16_t odp_le_to_cpu_16(uint16le_t le16)
  */
 static inline uint32_t odp_le_to_cpu_32(uint32le_t le32)
 {
-#if ODP_BYTE_ORDER == ODP_LITTLE_ENDIAN
+#if ODP_LITTLE_ENDIAN
 	return (__odp_force uint32_t)le32;
 #else
 	return __builtin_bswap32((__odp_force uint32_t)le32);
@@ -197,7 +187,7 @@  static inline uint32_t odp_le_to_cpu_32(uint32le_t le32)
  */
 static inline uint64_t odp_le_to_cpu_64(uint64le_t le64)
 {
-#if ODP_BYTE_ORDER == ODP_LITTLE_ENDIAN
+#if ODP_LITTLE_ENDIAN
 	return (__odp_force uint64_t)le64;
 #else
 	return __builtin_bswap64((__odp_force uint64_t)le64);
@@ -216,7 +206,7 @@  static inline uint64_t odp_le_to_cpu_64(uint64le_t le64)
  */
 static inline uint16le_t odp_cpu_to_le_16(uint16_t cpu16)
 {
-#if ODP_BYTE_ORDER == ODP_LITTLE_ENDIAN
+#if ODP_LITTLE_ENDIAN
 	return (__odp_force uint16le_t)cpu16;
 #else
 	return (__odp_force uint16le_t)__odp_builtin_bswap16(cpu16);
@@ -230,7 +220,7 @@  static inline uint16le_t odp_cpu_to_le_16(uint16_t cpu16)
  */
 static inline uint32le_t odp_cpu_to_le_32(uint32_t cpu32)
 {
-#if ODP_BYTE_ORDER == ODP_LITTLE_ENDIAN
+#if ODP_LITTLE_ENDIAN
 	return (__odp_force uint32le_t)cpu32;
 #else
 	return (__odp_force uint32le_t)__builtin_bswap32(cpu32);
@@ -244,7 +234,7 @@  static inline uint32le_t odp_cpu_to_le_32(uint32_t cpu32)
  */
 static inline uint64le_t odp_cpu_to_le_64(uint64_t cpu64)
 {
-#if ODP_BYTE_ORDER == ODP_LITTLE_ENDIAN
+#if ODP_LITTLE_ENDIAN
 	return (__odp_force uint64le_t)cpu64;
 #else
 	return (__odp_force uint64le_t)__builtin_bswap64(cpu64);