This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: PATCH: Use unsigned long int in __bswap_64 for __WORDSIZE == 64


On Thu, Oct 11, 2012 at 3:12 PM, Roland McGrath <roland@hack.frob.com> wrote:
>> 2012-10-11  H.J. Lu  <hongjiu.lu@intel.com>
>>
>>       * sysdeps/x86/bits/byteswap.h [__GNUC_PREREQ (4, 2)]
>>       [__WORDSIZE == 64] (__bswap_uint64_t): New macro.
>>       [__GNUC_PREREQ (4, 2)] [__WORDSIZE == 64] (__bswap_64): Use
>>       it.
>
> That says you only defined and only used in under [__WORDSIZE == 64],
> which is not so.
>
>         * sysdeps/x86/bits/byteswap.h
>         [__GNUC_PREREQ (4, 2)] (__bswap_uint64_t): New macro,
>         different definitions for [__WORDSIZE == 64] and not.
>         [__GNUC_PREREQ (4, 2)] (__bswap_64): Use it for return and arg types.
>
>> +#  if __WORDSIZE == 64
>> +#   define __bswap_uint64_t unsigned long int
>> +#  else
>> +#   define __bswap_uint64_t unsigned long long int
>> +#  endif
>
> Add a comment saying this must match the type used for uint64_t.
>
> Hmm.  Actually, I think you can #include <bits/types.h> and use __uint64_t
> without any name space issues.  So that's clearly the right thing to do,
> and is sufficiently inherently obvious that it doesn't require any special
> comments.  And then it's clearly right to use __uint64_t uniformly
> throughout this file, even in the places where there is no variation in
> possible definitions.
>

Like this?

-- 
H.J.
----
	* sysdeps/x86/bits/byteswap.h: Include <bits/types.h>.
	(__bswap_64): __uint64_t for unsigned 64-bit int.

diff --git a/sysdeps/x86/bits/byteswap.h b/sysdeps/x86/bits/byteswap.h
index 4178439..babe567 100644
--- a/sysdeps/x86/bits/byteswap.h
+++ b/sysdeps/x86/bits/byteswap.h
@@ -24,6 +24,7 @@
 #define _BITS_BYTESWAP_H 1

 #include <features.h>
+#include <bits/types.h>
 #include <bits/wordsize.h>

 /* Swap bytes in 16 bit value.  */
@@ -104,15 +105,15 @@ __bswap_32 (unsigned int __bsx)
 		     | (((x) & 0x00000000000000ffull) << 56)))

 # if __GNUC_PREREQ (4, 2)
-static __inline unsigned long long int
-__bswap_64 (unsigned long long int __bsx)
+static __inline __uint64_t
+__bswap_64 (__uint64_t __bsx)
 {
   return __builtin_bswap64 (__bsx);
 }
 # elif __WORDSIZE == 64
 #  define __bswap_64(x) \
      (__extension__							      \
-      ({ register unsigned long __v, __x = (x);				      \
+      ({ register __uint64_t __v, __x = (x);				      \
 	 if (__builtin_constant_p (__x))				      \
 	   __v = __bswap_constant_64 (__x);				      \
 	 else								      \
@@ -121,7 +122,7 @@ __bswap_64 (unsigned long long int __bsx)
 # else
 #  define __bswap_64(x) \
      (__extension__                                                           \
-      ({ union { __extension__ unsigned long long int __ll;                   \
+      ({ union { __extension__ __uint64_t __ll;		                      \
 		 unsigned int __l[2]; } __w, __r;                             \
 	 if (__builtin_constant_p (x))                                        \
 	   __r.__ll = __bswap_constant_64 (x);                                \
@@ -144,8 +145,8 @@ __bswap_64 (unsigned long long int __bsx)
       | (((x) & 0x000000000000ff00ull) << 40)				      \
       | (((x) & 0x00000000000000ffull) << 56))

-static __inline unsigned long long int
-__bswap_64 (unsigned long long int __bsx)
+static __inline __uint64_t
+__bswap_64 (__uint64_t __bsx)
 {
   return __bswap_constant_64 (__bsx);
 }


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]