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] Fix up bswap_* types


On Tue, Aug 21, 2012 at 02:01:32PM -0700, Roland McGrath wrote:
> There is also the public <byteswap.h> (string/byteswap.h).  
> That is not standardized and I think it's entirely reasonable that it
> include <stdint.h>.

Ok.

> Note that endian.h is not specified by any published standard.
> You cited materials of the POSIX working group that indicate how
> it's expected to be specified in a future edition of POSIX.1, but
> until such an edition is actually finalized, we're still dealing
> with an <endian.h> that is a GNU invention.

Sorry about that.  Although I think that proposal looks fine.

> I think the right approach to the name space issue is to have the headers
> that include <bits/byteswap.h> include <stdint.h> themselves directly but
> not have any <bits/byteswap.h> include any header that defines any symbols
> in the application name space.

I did that, hopefully.  I moved #include <stdint.h> into
string/byteswap.h and into string/endian.h.
Does this look ok?  Tested x86_64.

> If a future need arises to use the <bits/byteswap.h> macros/inlines in
> another header that should not include <stdint.h>, then we can address
> that with a new set of private (__) type names that match uintNN_t.
> 
> I believe the -Wformat warnings from PRIx64 et al vs {u,}int64_t on x86_64
> are a 2.16 regression that we will be fixing with this change.  So it ought
> to have a bug number.

Filed http://sourceware.org/bugzilla/show_bug.cgi?id=14508.  Thanks,

2012-08-22  Marek Polacek  <polacek@redhat.com>

	[BZ #14508]
	* string/byteswap.h: Include <stdint.h>.
	* string/endian.h: Likewise.
	* bits/byteswap-16.h (__bswap_16) [__GNUC__]: Use uint16_t instead
	of unsigned short int.
	(__bswapt_16) [!__GNUC__]: Likewise.
	* bits/byteswap.h (__bswap_constant_16): Use uint16_t instead of unsigned
	 short int.
	(__bswap_32) [__GNUC_PREREQ]: Use uint32_t instead of unsigned int.
	(__bswap_32) [!__GNUC_PREREQ]: Likewise.
	(__bswap_64) [__GNUC_PREREQ]: Use uint64_t instead of unsigned long long
	int.
	(__bswap_64) [!__GNUC_PREREQ]: Use uint64_t instead of unsigned long long
	int and uint32_t instead of unsigned int.
	(__bswap_64) [__GLIBC_HAVE_LONG_LONG]: Use uint64_t instead of
	unsigned long long int.
	* sysdeps/s390/bits/byteswap-16.h (__bswap_16) [WORDSIZE == 64]: Use
	uint16_t instead of unsigned short int.
	(__bswap_16) [!WORDSIZE == 64]: Likewise.
	(__bswap_16) [!__GNUC__]: Likewise.
	* sysdeps/s390/bits/byteswap.h (__bswap_32) [__GNUC_PREREQ]: Use uint32_t
	 instead of unsigned int.
	(__bswap_32) [!__GNUC_PREREQ]: Likewise.
	(__bswap_64) [__GNUC_PREREQ]: Use uint64_t instead of unsigned long long
	int.
	(__bswap_64) [!__GNUC_PREREQ]: Use uint64_t instead of unsigned long long
	int and uint32_t instead of unsigned int.
	(__bswap_64) [__GLIBC_HAVE_LONG_LONG]: Use uint64_t instead of
	unsigned long long int.
	(__bswap_constant_16): Use uint16_t instead of unsigned short int.
	* sysdeps/x86/bits/byteswap-16.h (__bswap_16) [WORDSIZE == 64]: Use
	uint16_t instead of unsigned short int.
	(__bswap_16) [!WORDSIZE == 64]: Likewise.
	(__bswap_16) [!__GNUC__]: Likewise.
	* sysdeps/x86/bits/byteswap.h (__bswap_32) [__GNUC_PREREQ]: Use uint32_t
	 instead of unsigned int.
	(__bswap_32) [!__GNUC_PREREQ]: Likewise.
	(__bswap_64) [__GNUC_PREREQ]: Use uint64_t instead of unsigned long long
	int.
	(__bswap_64) [!__GNUC_PREREQ]: Use uint64_t instead of unsigned long long
	int and uint32_t instead of unsigned int.
	(__bswap_64) [__GLIBC_HAVE_LONG_LONG]: Use uint64_t instead of
	unsigned long long int.

--- libc/bits/byteswap-16.h.mp	2012-08-19 14:50:02.904713109 +0200
+++ libc/bits/byteswap-16.h	2012-08-22 16:18:35.484207664 +0200
@@ -23,11 +23,11 @@
 #ifdef __GNUC__
 # define __bswap_16(x) \
     (__extension__							      \
-     ({ unsigned short int __bsx = (unsigned short int) (x);		      \
+     ({ uint16_t __bsx = (uint16_t) (x);				      \
        __bswap_constant_16 (__bsx); }))
 #else
-static __inline unsigned short int
-__bswap_16 (unsigned short int __bsx)
+static __inline uint16_t
+__bswap_16 (uint16_t __bsx)
 {
   return __bswap_constant_16 (__bsx);
 }
--- libc/bits/byteswap.h.mp	2012-08-19 14:49:59.151703654 +0200
+++ libc/bits/byteswap.h	2012-08-22 16:19:15.681319692 +0200
@@ -1,5 +1,5 @@
 /* Macros to swap the order of bytes in integer values.
-   Copyright (C) 1997-2012  Free Software Foundation, Inc.
+   Copyright (C) 1997-2012 Free Software Foundation, Inc.
    This file is part of the GNU C Library.
 
    The GNU C Library is free software; you can redistribute it and/or
@@ -27,7 +27,7 @@
 
 /* Swap bytes in 16 bit value.  */
 #define __bswap_constant_16(x) \
-	((unsigned short int)((((x) >> 8) & 0xffu) | (((x) & 0xffu) << 8)))
+	((uint16_t) ((((x) >> 8) & 0xffu) | (((x) & 0xffu) << 8)))
 
 /* Get __bswap_16.  */
 #include <bits/byteswap-16.h>
@@ -39,19 +39,19 @@
 
 #ifdef __GNUC__
 # if __GNUC_PREREQ (4, 2)
-static __inline unsigned int
-__bswap_32 (unsigned int __bsx)
+static __inline uint32_t
+__bswap_32 (uint32_t __bsx)
 {
   return __builtin_bswap32 (__bsx);
 }
 # else
 #  define __bswap_32(x) \
   (__extension__							      \
-   ({ register unsigned int __bsx = (x); __bswap_constant_32 (__bsx); }))
+   ({ register uint32_t __bsx = (x); __bswap_constant_32 (__bsx); }))
 # endif
 #else
-static __inline unsigned int
-__bswap_32 (unsigned int __bsx)
+static __inline uint32_t
+__bswap_32 (uint32_t __bsx)
 {
   return __bswap_constant_32 (__bsx);
 }
@@ -70,16 +70,16 @@ __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);
 }
 # else
 #  define __bswap_64(x) \
      (__extension__							      \
-      ({ union { __extension__ unsigned long long int __ll;		      \
-		 unsigned int __l[2]; } __w, __r;			      \
+      ({ union { __extension__ uint64_t __ll;				      \
+		 uint32_t __l[2]; } __w, __r;				      \
 	 if (__builtin_constant_p (x))					      \
 	   __r.__ll = __bswap_constant_64 (x);				      \
 	 else								      \
@@ -101,8 +101,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);
 }
--- libc/string/byteswap.h.mp	2012-08-22 15:39:37.725254448 +0200
+++ libc/string/byteswap.h	2012-08-22 15:39:48.069279343 +0200
@@ -18,6 +18,7 @@
 #ifndef _BYTESWAP_H
 #define _BYTESWAP_H	1
 
+#include <stdint.h>
 #include <features.h>
 
 /* Get the machine specific, optimized definitions.  */
--- libc/string/endian.h.mp	2012-08-22 15:40:11.595335965 +0200
+++ libc/string/endian.h	2012-08-22 15:40:49.306580707 +0200
@@ -18,6 +18,7 @@
 #ifndef	_ENDIAN_H
 #define	_ENDIAN_H	1
 
+#include <stdint.h>
 #include <features.h>
 
 /* Definitions for byte order, according to significance of bytes,
--- libc/sysdeps/s390/bits/byteswap-16.h.mp	2012-08-19 16:34:38.626981852 +0200
+++ libc/sysdeps/s390/bits/byteswap-16.h	2012-08-22 16:18:05.044122826 +0200
@@ -28,11 +28,11 @@
 # if __WORDSIZE == 64
 #  define __bswap_16(x) \
      (__extension__							      \
-      ({ unsigned short int __v, __x = (unsigned short int) (x);	      \
+      ({ uint16_t __v, __x = (uint16_t) (x);				      \
 	 if (__builtin_constant_p (x))					      \
 	   __v = __bswap_constant_16 (__x);				      \
 	 else {								      \
-	   unsigned short int __tmp = (unsigned short int) (__x);             \
+	   uint16_t __tmp = (uint16_t) (__x);				      \
 	   __asm__ __volatile__ (                                             \
 	      "lrvh %0,%1"                                                    \
 	      : "=&d" (__v) : "m" (__tmp) );                                  \
@@ -41,11 +41,11 @@
 # else
 #  define __bswap_16(x) \
      (__extension__							      \
-      ({ unsigned short int __v, __x = (unsigned short int) (x);	      \
+      ({ uint16_t __v, __x = (uint16_t) (x);				      \
 	 if (__builtin_constant_p (x))					      \
 	   __v = __bswap_constant_16 (__x);				      \
 	 else {								      \
-	   unsigned short int __tmp = (unsigned short int) (__x);             \
+	   uint16_t __tmp = (uint16_t) (__x);				      \
 	   __asm__ __volatile__ (                                             \
 	      "sr   %0,%0\n"                                                  \
 	      "la   1,%1\n"                                                   \
@@ -57,8 +57,8 @@
 # endif
 #else
 /* This is better than nothing.  */
-static __inline unsigned short int
-__bswap_16 (unsigned short int __bsx)
+static __inline uint16_t
+__bswap_16 (uint16_t __bsx)
 {
   return __bswap_constant_16 (__bsx);
 }
--- libc/sysdeps/s390/bits/byteswap.h.mp	2012-08-19 16:22:27.367608050 +0200
+++ libc/sysdeps/s390/bits/byteswap.h	2012-08-22 16:22:09.488804094 +0200
@@ -27,7 +27,7 @@
 #define _BITS_BYTESWAP_H 1
 
 #define __bswap_constant_16(x) \
-	((unsigned short int) ((((x) >> 8) & 0xff) | (((x) & 0xff) << 8)))
+	((uint16_t) ((((x) >> 8) & 0xff) | (((x) & 0xff) << 8)))
 
 /* Get __bswap_16.  */
 #include <bits/byteswap-16.h>
@@ -41,11 +41,11 @@
 # if __WORDSIZE == 64
 #  define __bswap_32(x) \
      (__extension__							      \
-      ({ unsigned int __v, __x = (x);					      \
+      ({ uint32_t __v, __x = (x);					      \
 	 if (__builtin_constant_p (x))					      \
 	   __v = __bswap_constant_32 (__x);				      \
 	 else {								      \
-	   unsigned int __tmp = (unsigned int) (__x);                         \
+	   uint32_t __tmp = (uint32_t) (__x);				      \
 	   __asm__ __volatile__ (                                             \
 	      "lrv   %0,%1"                                                   \
 	      : "=&d" (__v) : "m" (__tmp));                                   \
@@ -54,11 +54,11 @@
 # else
 #  define __bswap_32(x) \
      (__extension__							      \
-      ({ unsigned int __v, __x = (x);					      \
+      ({ uint32_t __v, __x = (x);					      \
 	 if (__builtin_constant_p (x))					      \
 	   __v = __bswap_constant_32 (__x);				      \
 	 else {								      \
-	   unsigned int __tmp = (unsigned int) (__x);                         \
+	   uint32_t __tmp = (uint32_t) (__x);				      \
 	   __asm__ __volatile__ (                                             \
 	      "la    1,%1\n"                                                  \
 	      "icm   %0,8,3(1)\n"                                             \
@@ -70,8 +70,8 @@
 	 __v; }))
 # endif
 #else
-static __inline unsigned int
-__bswap_32 (unsigned int __bsx)
+static __inline uint32_t
+__bswap_32 (uint32_t __bsx)
 {
   return __bswap_constant_32 (__bsx);
 }
@@ -92,11 +92,11 @@ __bswap_32 (unsigned int __bsx)
 # if __WORDSIZE == 64
 #  define __bswap_64(x) \
      (__extension__							      \
-      ({ unsigned long __w, __x = (x);					      \
+      ({ uint64_t __w, __x = (x);					      \
 	 if (__builtin_constant_p (x))					      \
 	   __w = __bswap_constant_64 (__x);				      \
 	 else {								      \
-	   unsigned long __tmp = (unsigned long) (__x);                       \
+	   uint64_t __tmp = (uint64_t) (__x);				      \
 	   __asm__ __volatile__ (                                             \
 	      "lrvg  %0,%1"                                                   \
 	      : "=&d" (__w) : "m" (__tmp));                                   \
@@ -105,8 +105,8 @@ __bswap_32 (unsigned int __bsx)
 # else
 #  define __bswap_64(x) \
      __extension__					\
-       ({ union { unsigned long long int __ll;		\
-		  unsigned long int __l[2]; } __w, __r;	\
+       ({ union { uint64_t __ll;			\
+		  uint32_t __l[2]; } __w, __r;		\
 	  __w.__ll = (x);				\
 	  __r.__l[0] = __bswap_32 (__w.__l[1]);		\
 	  __r.__l[1] = __bswap_32 (__w.__l[0]);		\
@@ -123,8 +123,8 @@ __bswap_32 (unsigned 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);
 }
--- libc/sysdeps/x86/bits/byteswap-16.h.mp	2012-08-19 16:34:46.892008679 +0200
+++ libc/sysdeps/x86/bits/byteswap-16.h	2012-08-22 16:22:48.651913242 +0200
@@ -24,7 +24,7 @@
 # if __GNUC__ >= 2
 #  define __bswap_16(x) \
      (__extension__							      \
-      ({ register unsigned short int __v, __x = (unsigned short int) (x);     \
+      ({ register uint16_t __v, __x = (uint16_t) (x);			      \
 	 if (__builtin_constant_p (__x))				      \
 	   __v = __bswap_constant_16 (__x);				      \
 	 else								      \
@@ -37,12 +37,12 @@
 /* This is better than nothing.  */
 #  define __bswap_16(x) \
      (__extension__							      \
-      ({ register unsigned short int __x = (unsigned short int) (x);	      \
+      ({ register uint16_t __x = (uint16_t) (x);			      \
 	 __bswap_constant_16 (__x); }))
 # endif
 #else
-static __inline unsigned short int
-__bswap_16 (unsigned short int __bsx)
+static __inline uint16_t
+__bswap_16 (uint16_t __bsx)
 {
   return __bswap_constant_16 (__bsx);
 }
--- libc/sysdeps/x86/bits/byteswap.h.mp	2012-08-19 15:32:48.711742439 +0200
+++ libc/sysdeps/x86/bits/byteswap.h	2012-08-22 16:20:34.512539392 +0200
@@ -1,5 +1,5 @@
 /* Macros to swap the order of bytes in integer values.
-   Copyright (C) 1997-2012   Free Software Foundation, Inc.
+   Copyright (C) 1997-2012 Free Software Foundation, Inc.
    This file is part of the GNU C Library.
 
    The GNU C Library is free software; you can redistribute it and/or
@@ -28,7 +28,7 @@
 
 /* Swap bytes in 16 bit value.  */
 #define __bswap_constant_16(x) \
-     ((unsigned short int) ((((x) >> 8) & 0xff) | (((x) & 0xff) << 8)))
+     ((uint16_t) ((((x) >> 8) & 0xff) | (((x) & 0xff) << 8)))
 
 /* Get __bswap_16.  */
 #include <bits/byteswap-16.h>
@@ -40,8 +40,8 @@
 
 #ifdef __GNUC__
 # if __GNUC_PREREQ (4, 2)
-static __inline unsigned int
-__bswap_32 (unsigned int __bsx)
+static __inline uint32_t
+__bswap_32 (uint32_t __bsx)
 {
   return __builtin_bswap32 (__bsx);
 }
@@ -56,7 +56,7 @@ __bswap_32 (unsigned int __bsx)
    `bswap' opcode.  On i386 we have to use three instructions.  */
 #   define __bswap_32(x) \
       (__extension__							      \
-       ({ register unsigned int __v, __x = (x);				      \
+       ({ register uint32_t __v, __x = (x);				      \
 	  if (__builtin_constant_p (__x))				      \
 	    __v = __bswap_constant_32 (__x);				      \
 	  else								      \
@@ -65,7 +65,7 @@ __bswap_32 (unsigned int __bsx)
 #  else
 #   define __bswap_32(x)						      \
       (__extension__							      \
-       ({ register unsigned int __v, __x = (x);				      \
+       ({ register uint32_t __v, __x = (x);				      \
 	  if (__builtin_constant_p (__x))				      \
 	    __v = __bswap_constant_32 (__x);				      \
 	  else								      \
@@ -80,11 +80,11 @@ __bswap_32 (unsigned int __bsx)
 # else
 #  define __bswap_32(x) \
      (__extension__							      \
-      ({ register unsigned int __x = (x); __bswap_constant_32 (__x); }))
+      ({ register uint32_t __x = (x); __bswap_constant_32 (__x); }))
 # endif
 #else
-static __inline unsigned int
-__bswap_32 (unsigned int __bsx)
+static __inline uint32_t
+__bswap_32 (uint32_t __bsx)
 {
   return __bswap_constant_32 (__bsx);
 }
@@ -104,15 +104,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,8 +121,8 @@ __bswap_64 (unsigned long long int __bsx
 # else
 #  define __bswap_64(x) \
      (__extension__                                                           \
-      ({ union { __extension__ unsigned long long int __ll;                   \
-		 unsigned int __l[2]; } __w, __r;                             \
+      ({ union { __extension__ uint64_t __ll;				      \
+		 uint32_t __l[2]; } __w, __r;				      \
 	 if (__builtin_constant_p (x))                                        \
 	   __r.__ll = __bswap_constant_64 (x);                                \
 	 else                                                                 \
@@ -144,8 +144,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);
 }

	Marek


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