* Re: [PATCH] Fix up bswap_* types
[not found] ` <20120822164747.90AD42C0F4@topped-with-meat.com>
@ 2012-08-22 20:06 ` Marek Polacek
2012-08-22 20:08 ` Joseph S. Myers
2012-08-22 20:12 ` Roland McGrath
0 siblings, 2 replies; 9+ messages in thread
From: Marek Polacek @ 2012-08-22 20:06 UTC (permalink / raw)
To: Roland McGrath; +Cc: Joseph S. Myers, libc-alpha, Andreas Krebbel, libc-ports
On Wed, Aug 22, 2012 at 09:47:47AM -0700, Roland McGrath wrote:
> Some of your ChangeLog lines are too long (wrap at column 79 or
> before). Otherwise it looks fine to me. But please test an i686
I've fixed the CL. But unfortunately I hadn't noticed that my patch
actually breaks stdlib/isomac test. It fails with:
Tested files:
assert.h
ctype.h
#define INT16_C(c) c
#define UINTMAX_C(c) c ## UL
#define UINT16_C(c) c
#define INT64_C(c) c ## L
#define UINT64_C(c) c ## UL
#define INT8_C(c) c
#define INT32_C(c) c
#define UINT32_C(c) c ## U
#define UINT8_C(c) c
#define INTMAX_C(c) c ## L
errno.h
float.h
iso646.h
limits.h
locale.h
math.h
setjmp.h
signal.h
stdarg.h
stddef.h
stdio.h
stdlib.h
string.h
time.h
wchar.h
wctype.h
#define INT16_C(c) c
#define UINTMAX_C(c) c ## UL
#define UINT16_C(c) c
#define INT64_C(c) c ## L
#define UINT64_C(c) c ## UL
#define INT8_C(c) c
#define INT32_C(c) c
#define UINT32_C(c) c ## U
#define UINT8_C(c) c
#define INTMAX_C(c) c ## L
because {,w}ctype.h contains <endian.h> and
we end up including sysdeps/generic/stdint.h which defines macros above.
I've circumvent that by using #ifndef _ISOMAC guard around them.
Now tested both i?86 and x86_64. Isomac doesn't yell this time around.
Ok for trunk? CC'ing Andreas, ok for s390, too? Thanks,
2012-08-22 Marek Polacek <polacek@redhat.com>
[BZ #14508]
* sysdeps/generic/stdint.h: Exclude some macros if _ISOMAC is defined.
* 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 19:52:49.218953619 +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 19:52:49.223953637 +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 19:52:49.266953776 +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 19:52:49.275953805 +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 19:52:49.353954060 +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 19:52:49.378954140 +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 19:52:49.429954305 +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 19:52:49.437954331 +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);
}
--- libc/sysdeps/generic/stdint.h.mp 2012-08-22 21:00:46.203062052 +0200
+++ libc/sysdeps/generic/stdint.h 2012-08-22 21:04:55.463896478 +0200
@@ -283,7 +283,8 @@ typedef unsigned long long int uintmax_t
/* The ISO C99 standard specifies that in C++ implementations these
should only be defined if explicitly requested. */
-#if !defined __cplusplus || defined __STDC_CONSTANT_MACROS
+#if (!defined __cplusplus || defined __STDC_CONSTANT_MACROS) \
+ && !defined _ISOMAC
/* Signed. */
# define INT8_C(c) c
Marek
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] Fix up bswap_* types
2012-08-22 20:06 ` [PATCH] Fix up bswap_* types Marek Polacek
@ 2012-08-22 20:08 ` Joseph S. Myers
2012-08-22 20:18 ` Marek Polacek
2012-08-22 20:12 ` Roland McGrath
1 sibling, 1 reply; 9+ messages in thread
From: Joseph S. Myers @ 2012-08-22 20:08 UTC (permalink / raw)
To: Marek Polacek; +Cc: Roland McGrath, libc-alpha, Andreas Krebbel, libc-ports
On Wed, 22 Aug 2012, Marek Polacek wrote:
> because {,w}ctype.h contains <endian.h> and
> we end up including sysdeps/generic/stdint.h which defines macros above.
> I've circumvent that by using #ifndef _ISOMAC guard around them.
That sounds wrong. _ISOMAC guards are appropriate in the *internal*
include/ headers, but the installed headers must conform to the ISO C
requirements without any such special macros being defined - which means
not exporting stdint.h symbols in C90 mode.
--
Joseph S. Myers
joseph@codesourcery.com
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] Fix up bswap_* types
2012-08-22 20:06 ` [PATCH] Fix up bswap_* types Marek Polacek
2012-08-22 20:08 ` Joseph S. Myers
@ 2012-08-22 20:12 ` Roland McGrath
2012-08-22 23:05 ` Marek Polacek
1 sibling, 1 reply; 9+ messages in thread
From: Roland McGrath @ 2012-08-22 20:12 UTC (permalink / raw)
To: Marek Polacek; +Cc: Joseph S. Myers, libc-alpha, Andreas Krebbel, libc-ports
> because {,w}ctype.h contains <endian.h> and
> we end up including sysdeps/generic/stdint.h which defines macros above.
> I've circumvent that by using #ifndef _ISOMAC guard around them.
They are wrong to include <endian.h>, which declares public symbols.
They should use only <bits/endian.h> instead. This means all the various
bits/endian.h files need their #ifndef _ENDIAN_H sanity checks adjusted.
Thanks,
Roland
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] Fix up bswap_* types
2012-08-22 20:08 ` Joseph S. Myers
@ 2012-08-22 20:18 ` Marek Polacek
0 siblings, 0 replies; 9+ messages in thread
From: Marek Polacek @ 2012-08-22 20:18 UTC (permalink / raw)
To: Joseph S. Myers; +Cc: Roland McGrath, libc-alpha, Andreas Krebbel, libc-ports
On Wed, Aug 22, 2012 at 08:08:38PM +0000, Joseph S. Myers wrote:
> That sounds wrong. _ISOMAC guards are appropriate in the *internal*
> include/ headers, but the installed headers must conform to the ISO C
> requirements without any such special macros being defined - which means
> not exporting stdint.h symbols in C90 mode.
Aha, haven't noticed. I'm going to try another approach then...
Thanks,
Marek
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] Fix up bswap_* types
2012-08-22 20:12 ` Roland McGrath
@ 2012-08-22 23:05 ` Marek Polacek
2012-08-22 23:14 ` Roland McGrath
2012-08-22 23:33 ` Joseph S. Myers
0 siblings, 2 replies; 9+ messages in thread
From: Marek Polacek @ 2012-08-22 23:05 UTC (permalink / raw)
To: Roland McGrath; +Cc: Joseph S. Myers, libc-alpha, Andreas Krebbel, libc-ports
On Wed, Aug 22, 2012 at 01:12:24PM -0700, Roland McGrath wrote:
> They are wrong to include <endian.h>, which declares public symbols.
> They should use only <bits/endian.h> instead. This means all the various
> bits/endian.h files need their #ifndef _ENDIAN_H sanity checks adjusted.
In that case I think I could in {w,}ctype.h before including
<bits/endian.h> define say `__need_byteorder' and then tweak guards
in all the bits/endian.h so that they look like
#if !defined _ENDIAN_H && !defined __need_byteorder
# error blah...
#endif
And it also looks that I have to move following definitions:
#define __LITTLE_ENDIAN 1234
#define __BIG_ENDIAN 4321
#define __PDP_ENDIAN 3412
from string/endian.h to ctype/ctype.h, otherwise charset test
when running check just blow up.
What do ya say?
Marek
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] Fix up bswap_* types
2012-08-22 23:05 ` Marek Polacek
@ 2012-08-22 23:14 ` Roland McGrath
2012-08-22 23:33 ` Joseph S. Myers
1 sibling, 0 replies; 9+ messages in thread
From: Roland McGrath @ 2012-08-22 23:14 UTC (permalink / raw)
To: Marek Polacek; +Cc: Joseph S. Myers, libc-alpha, Andreas Krebbel, libc-ports
> In that case I think I could in {w,}ctype.h before including
> <bits/endian.h> define say `__need_byteorder' and then tweak guards
> in all the bits/endian.h so that they look like
>
> #if !defined _ENDIAN_H && !defined __need_byteorder
> # error blah...
> #endif
>
> And it also looks that I have to move following definitions:
> #define __LITTLE_ENDIAN 1234
> #define __BIG_ENDIAN 4321
> #define __PDP_ENDIAN 3412
> from string/endian.h to ctype/ctype.h, otherwise charset test
> when running check just blow up.
>
> What do ya say?
Oh, yeah. So I think what you actually want in ctype.h et al is:
#define __need_byteorder
#include <endian.h>
Then make <endian.h> just define __*_ENDIAN and include <bits/endian.h>
if included with __need_byteorder defined, and only do its other
declarations (and _ENDIAN_H) if included without __need_byteorder defined.
Thanks,
Roland
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] Fix up bswap_* types
2012-08-22 23:05 ` Marek Polacek
2012-08-22 23:14 ` Roland McGrath
@ 2012-08-22 23:33 ` Joseph S. Myers
2012-08-22 23:46 ` Roland McGrath
1 sibling, 1 reply; 9+ messages in thread
From: Joseph S. Myers @ 2012-08-22 23:33 UTC (permalink / raw)
To: Marek Polacek; +Cc: Roland McGrath, libc-alpha, Andreas Krebbel, libc-ports
On Thu, 23 Aug 2012, Marek Polacek wrote:
> On Wed, Aug 22, 2012 at 01:12:24PM -0700, Roland McGrath wrote:
> > They are wrong to include <endian.h>, which declares public symbols.
> > They should use only <bits/endian.h> instead. This means all the various
> > bits/endian.h files need their #ifndef _ENDIAN_H sanity checks adjusted.
>
> In that case I think I could in {w,}ctype.h before including
> <bits/endian.h> define say `__need_byteorder' and then tweak guards
> in all the bits/endian.h so that they look like
Personally I find the __need_* scheme pretty fragile, and it's also been
reported as causing problems for such things as attempts at pre-parsed
headers in GCC - it's generally nicer if a header does something
well-defined that doesn't depend on whether such a macro is defined before
including it.
Thus, whenever you are tempted to give a header a special __need_*
interface, I'd rather split out the relevant bits into a new internal
(bits/*) header and include that header in both places.
--
Joseph S. Myers
joseph@codesourcery.com
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] Fix up bswap_* types
2012-08-22 23:33 ` Joseph S. Myers
@ 2012-08-22 23:46 ` Roland McGrath
2012-08-22 23:59 ` Marek Polacek
0 siblings, 1 reply; 9+ messages in thread
From: Roland McGrath @ 2012-08-22 23:46 UTC (permalink / raw)
To: Joseph S. Myers; +Cc: Marek Polacek, libc-alpha, Andreas Krebbel, libc-ports
> Personally I find the __need_* scheme pretty fragile, and it's also been
> reported as causing problems for such things as attempts at pre-parsed
> headers in GCC - it's generally nicer if a header does something
> well-defined that doesn't depend on whether such a macro is defined before
> including it.
It's true. I was suggesting something that follows the prevailing scheme
for such cases, but that doesn't mean it's the ideal scheme.
> Thus, whenever you are tempted to give a header a special __need_*
> interface, I'd rather split out the relevant bits into a new internal
> (bits/*) header and include that header in both places.
Yes, we could instead move __*_ENDIAN into another file. The cleanest
arrangement would be:
* bits/endian.h becomes a generic header that defines __*_ENDIAN and does
#include <bits/byteorder.h>. Only this file would need complex
guards to allow inclusion via <endian.h> or <ctype.h> or <wctype.h>.
* sysdeps bits/endian.h files all renamed to bits/byteorder.h and have
a guard for _BITS_BYTEORDER_H.
Thanks,
Roland
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] Fix up bswap_* types
2012-08-22 23:46 ` Roland McGrath
@ 2012-08-22 23:59 ` Marek Polacek
0 siblings, 0 replies; 9+ messages in thread
From: Marek Polacek @ 2012-08-22 23:59 UTC (permalink / raw)
To: Roland McGrath; +Cc: Joseph S. Myers, libc-alpha, Andreas Krebbel, libc-ports
On Wed, Aug 22, 2012 at 04:46:10PM -0700, Roland McGrath wrote:
> Yes, we could instead move __*_ENDIAN into another file. The cleanest
> arrangement would be:
>
> * bits/endian.h becomes a generic header that defines __*_ENDIAN and does
> #include <bits/byteorder.h>. Only this file would need complex
> guards to allow inclusion via <endian.h> or <ctype.h> or <wctype.h>.
> * sysdeps bits/endian.h files all renamed to bits/byteorder.h and have
> a guard for _BITS_BYTEORDER_H.
That sounds good, thank both of you. I'll try hack something up
over the next few days.
Marek
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2012-08-22 23:59 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <20120819160958.GB3156@redhat.com>
[not found] ` <Pine.LNX.4.64.1208192243590.24239@digraph.polyomino.org.uk>
[not found] ` <20120820143509.GG3156@redhat.com>
[not found] ` <20120821210132.C13992C0D5@topped-with-meat.com>
[not found] ` <20120822154645.GB16963@redhat.com>
[not found] ` <20120822164747.90AD42C0F4@topped-with-meat.com>
2012-08-22 20:06 ` [PATCH] Fix up bswap_* types Marek Polacek
2012-08-22 20:08 ` Joseph S. Myers
2012-08-22 20:18 ` Marek Polacek
2012-08-22 20:12 ` Roland McGrath
2012-08-22 23:05 ` Marek Polacek
2012-08-22 23:14 ` Roland McGrath
2012-08-22 23:33 ` Joseph S. Myers
2012-08-22 23:46 ` Roland McGrath
2012-08-22 23:59 ` Marek Polacek
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).