Message ID | 20190919221829.6137-1-alistair.francis@wdc.com |
---|---|
State | New |
Headers | show |
Series | [RFC] time: Create a endian_t.h headerfile | expand |
On Thu, 19 Sep 2019, Alistair Francis wrote: > To allow struct __timespec64 and struct timespec to be the same on a > 32-bit architecture with a 64-bit time_t we need to add padding to the > struct timespec. This padding requires knowledge of the machines > endianess, which requires us exporting the endianness in > time/bits/types/struct_timespec.h. > > To reduce the number of macros that we are exporting let's split out an > endian_t.h header file that can be used. An installed header has to be appropriately listed in "headers" in some directory's Makefile to cause it to be installed; otherwise you get code that builds file in the glibc source tree but breaks with an installed compiler and headers. The bits/types/ namespace is *only* for headers defining a single type (that is, bits/types/endian_t.h would be a header that defines the endian_t typedef and nothing else). So it's not appropriate for this header. You can't use "#include <time/...>" in an installed header because that would also break when using an installed compiler and headers. #includes in installed headers need to use the <bits/*> path the included header would have when installed. Then you add an include/bits/ wrapper in the glibc source tree to allow the uninstalled header to be found when building glibc. (And time/ is not the right location for a header about endianness. Either string/, or use the top-level bits/ directory and so avoid the need for any wrapper at all.) If anything, I think the cleanest naming would be to have <bits/endian.h> be the architecture-independent header that defines these three macros and includes <bits/endian-arch.h>, where the existing <bits/endian.h> headers that define __BYTE_ORDER all get renamed to <bits/endian-arch.h>. So you'd need to add bits/endian-arch.h to the "headers" setting in string/Makefile.
On Thu, Sep 19, 2019 at 7:34 PM Joseph Myers <joseph@codesourcery.com> wrote: > > If anything, I think the cleanest naming would be to have <bits/endian.h> > be the architecture-independent header that defines these three macros and > includes <bits/endian-arch.h>, where the existing <bits/endian.h> headers > that define __BYTE_ORDER all get renamed to <bits/endian-arch.h>. So > you'd need to add bits/endian-arch.h to the "headers" setting in > string/Makefile. I believe I wrote that patch already, in fact, as part of my installed-headers cleanups series: https://sourceware.org/ml/libc-alpha/2019-06/msg00783.html Unlike a lot of those patches, it should be low-risk for application breakage and applicable independently. zw
On Fri, 20 Sep 2019, Zack Weinberg wrote: > On Thu, Sep 19, 2019 at 7:34 PM Joseph Myers <joseph@codesourcery.com> wrote: > > > > If anything, I think the cleanest naming would be to have <bits/endian.h> > > be the architecture-independent header that defines these three macros and > > includes <bits/endian-arch.h>, where the existing <bits/endian.h> headers > > that define __BYTE_ORDER all get renamed to <bits/endian-arch.h>. So > > you'd need to add bits/endian-arch.h to the "headers" setting in > > string/Makefile. > > I believe I wrote that patch already, in fact, as part of my > installed-headers cleanups series: > https://sourceware.org/ml/libc-alpha/2019-06/msg00783.html Unlike a > lot of those patches, it should be low-risk for application breakage > and applicable independently. Yes, that's the sort of thing I'd expect (and I had a comment on it in <https://sourceware.org/ml/libc-alpha/2019-07/msg00561.html>). (This is not a review. When retesting this patch it would be necessary to check if any new includes of <endian.h> have been added to installed headers that need updating as well.) I should explicitly note that, while it's easy to think of possible followup cleanups in this area, it's best *not* to combine such cleanups with this patch in order to keep it of a reasonable size and complexity.
On Fri, Sep 20, 2019 at 9:51 AM Joseph Myers <joseph@codesourcery.com> wrote: > > On Fri, 20 Sep 2019, Zack Weinberg wrote: > > > On Thu, Sep 19, 2019 at 7:34 PM Joseph Myers <joseph@codesourcery.com> wrote: > > > > > > If anything, I think the cleanest naming would be to have <bits/endian.h> > > > be the architecture-independent header that defines these three macros and > > > includes <bits/endian-arch.h>, where the existing <bits/endian.h> headers > > > that define __BYTE_ORDER all get renamed to <bits/endian-arch.h>. So > > > you'd need to add bits/endian-arch.h to the "headers" setting in > > > string/Makefile. > > > > I believe I wrote that patch already, in fact, as part of my > > installed-headers cleanups series: > > https://sourceware.org/ml/libc-alpha/2019-06/msg00783.html Unlike a > > lot of those patches, it should be low-risk for application breakage > > and applicable independently. That patch doesn't apply cleanly. What happened to this series? Should I wait for it to go in or should I look at manually applying this patch. > > Yes, that's the sort of thing I'd expect (and I had a comment on it in > <https://sourceware.org/ml/libc-alpha/2019-07/msg00561.html>). (This is > not a review. When retesting this patch it would be necessary to check if > any new includes of <endian.h> have been added to installed headers that > need updating as well.) > > I should explicitly note that, while it's easy to think of possible > followup cleanups in this area, it's best *not* to combine such cleanups > with this patch in order to keep it of a reasonable size and complexity. Ok Alistair > > -- > Joseph S. Myers > joseph@codesourcery.com
On Fri, Sep 20, 2019 at 2:05 PM Alistair Francis <alistair23@gmail.com> wrote: > > On Fri, Sep 20, 2019 at 9:51 AM Joseph Myers <joseph@codesourcery.com> wrote: > > > > On Fri, 20 Sep 2019, Zack Weinberg wrote: > > > > > On Thu, Sep 19, 2019 at 7:34 PM Joseph Myers <joseph@codesourcery.com> wrote: > > > > > > > > If anything, I think the cleanest naming would be to have <bits/endian.h> > > > > be the architecture-independent header that defines these three macros and > > > > includes <bits/endian-arch.h>, where the existing <bits/endian.h> headers > > > > that define __BYTE_ORDER all get renamed to <bits/endian-arch.h>. So > > > > you'd need to add bits/endian-arch.h to the "headers" setting in > > > > string/Makefile. > > > > > > I believe I wrote that patch already, in fact, as part of my > > > installed-headers cleanups series: > > > https://sourceware.org/ml/libc-alpha/2019-06/msg00783.html Unlike a > > > lot of those patches, it should be low-risk for application breakage > > > and applicable independently. > > That patch doesn't apply cleanly. What happened to this series? Should > I wait for it to go in or should I look at manually applying this > patch. It didn't turn out to be too bad to manually apply the patch. I think I have something that is all working now. I haven't had a chance to test it though. Alistair > > > > > Yes, that's the sort of thing I'd expect (and I had a comment on it in > > <https://sourceware.org/ml/libc-alpha/2019-07/msg00561.html>). (This is > > not a review. When retesting this patch it would be necessary to check if > > any new includes of <endian.h> have been added to installed headers that > > need updating as well.) > > > > I should explicitly note that, while it's easy to think of possible > > followup cleanups in this area, it's best *not* to combine such cleanups > > with this patch in order to keep it of a reasonable size and complexity. > > Ok > > Alistair > > > > > -- > > Joseph S. Myers > > joseph@codesourcery.com
diff --git a/bits/endian.h b/bits/endian.h index 45afd4ae477..29838739049 100644 --- a/bits/endian.h +++ b/bits/endian.h @@ -6,7 +6,7 @@ So if cross-compiling to a machine with a different byte order, the bits/endian.h file for that machine must exist. */ -#ifndef _ENDIAN_H +#ifndef _ENDIAN_T_H # error "Never use <bits/endian.h> directly; include <endian.h> instead." #endif diff --git a/string/endian.h b/string/endian.h index 17e4e893fb5..e23484c1f0f 100644 --- a/string/endian.h +++ b/string/endian.h @@ -19,21 +19,7 @@ #define _ENDIAN_H 1 #include <features.h> - -/* Definitions for byte order, according to significance of bytes, - from low addresses to high addresses. The value is what you get by - putting '4' in the most significant byte, '3' in the second most - significant byte, '2' in the second least significant byte, and '1' - in the least significant byte, and then writing down one digit for - each byte, starting with the byte at the lowest address at the left, - and proceeding to the byte with the highest address at the right. */ - -#define __LITTLE_ENDIAN 1234 -#define __BIG_ENDIAN 4321 -#define __PDP_ENDIAN 3412 - -/* This file defines `__BYTE_ORDER' for the particular machine. */ -#include <bits/endian.h> +#include <time/bits/types/endian_t.h> /* Some machines may need to use a different endianness for floating point values. */ diff --git a/sysdeps/aarch64/bits/endian.h b/sysdeps/aarch64/bits/endian.h index 03801d312a7..26ab0f556e4 100644 --- a/sysdeps/aarch64/bits/endian.h +++ b/sysdeps/aarch64/bits/endian.h @@ -16,7 +16,7 @@ License along with the GNU C Library. If not, see <https://www.gnu.org/licenses/>. */ -#ifndef _ENDIAN_H +#ifndef _ENDIAN_T_H # error "Never use <bits/endian.h> directly; include <endian.h> instead." #endif diff --git a/sysdeps/alpha/bits/endian.h b/sysdeps/alpha/bits/endian.h index 8a16e14e24e..df2e6380726 100644 --- a/sysdeps/alpha/bits/endian.h +++ b/sysdeps/alpha/bits/endian.h @@ -1,6 +1,6 @@ /* Alpha is little-endian. */ -#ifndef _ENDIAN_H +#ifndef _ENDIAN_T_H # error "Never use <bits/endian.h> directly; include <endian.h> instead." #endif diff --git a/sysdeps/arm/bits/endian.h b/sysdeps/arm/bits/endian.h index f49f6ab1c9b..f3d17a449a3 100644 --- a/sysdeps/arm/bits/endian.h +++ b/sysdeps/arm/bits/endian.h @@ -1,4 +1,4 @@ -#ifndef _ENDIAN_H +#ifndef _ENDIAN_T_H # error "Never use <bits/endian.h> directly; include <endian.h> instead." #endif diff --git a/sysdeps/csky/bits/endian.h b/sysdeps/csky/bits/endian.h index 51df38d8f91..bfd4eaedcd0 100644 --- a/sysdeps/csky/bits/endian.h +++ b/sysdeps/csky/bits/endian.h @@ -1,4 +1,4 @@ -#ifndef _ENDIAN_H +#ifndef _ENDIAN_T_H # error "Never use <bits/endian.h> directly; include <endian.h> instead." #endif diff --git a/sysdeps/hppa/bits/endian.h b/sysdeps/hppa/bits/endian.h index 585db0c0faa..c1c6b23aff4 100644 --- a/sysdeps/hppa/bits/endian.h +++ b/sysdeps/hppa/bits/endian.h @@ -1,6 +1,6 @@ /* hppa1.1 big-endian. */ -#ifndef _ENDIAN_H +#ifndef _ENDIAN_T_H # error "Never use <bits/endian.h> directly; include <endian.h> instead." #endif diff --git a/sysdeps/m68k/bits/endian.h b/sysdeps/m68k/bits/endian.h index bf4ecb60a4c..f3edd102a28 100644 --- a/sysdeps/m68k/bits/endian.h +++ b/sysdeps/m68k/bits/endian.h @@ -1,6 +1,6 @@ /* m68k is big-endian. */ -#ifndef _ENDIAN_H +#ifndef _ENDIAN_T_H # error "Never use <bits/endian.h> directly; include <endian.h> instead." #endif diff --git a/sysdeps/microblaze/bits/endian.h b/sysdeps/microblaze/bits/endian.h index 6e961a55474..6e19d2c5dff 100644 --- a/sysdeps/microblaze/bits/endian.h +++ b/sysdeps/microblaze/bits/endian.h @@ -16,7 +16,7 @@ License along with the GNU C Library. If not, see <https://www.gnu.org/licenses/>. */ -#ifndef _ENDIAN_H +#ifndef _ENDIAN_T_H # error "Never use <bits/endian.h> directly; include <endian.h> instead." #endif diff --git a/sysdeps/mips/bits/endian.h b/sysdeps/mips/bits/endian.h index 126059799d7..12b14bc5aae 100644 --- a/sysdeps/mips/bits/endian.h +++ b/sysdeps/mips/bits/endian.h @@ -3,7 +3,7 @@ want to be able to share the installed header files between both, so we define __BYTE_ORDER based on GCC's predefines. */ -#ifndef _ENDIAN_H +#ifndef _ENDIAN_T_H # error "Never use <bits/endian.h> directly; include <endian.h> instead." #endif diff --git a/sysdeps/nios2/bits/endian.h b/sysdeps/nios2/bits/endian.h index 164f9e4d785..f83a74d0d8a 100644 --- a/sysdeps/nios2/bits/endian.h +++ b/sysdeps/nios2/bits/endian.h @@ -1,6 +1,6 @@ /* The Nios II architecture has selectable endianness. */ -#ifndef _ENDIAN_H +#ifndef _ENDIAN_T_H # error "Never use <bits/endian.h> directly; include <endian.h> instead." #endif diff --git a/sysdeps/powerpc/bits/endian.h b/sysdeps/powerpc/bits/endian.h index c82f7ad4de6..8463d17fd66 100644 --- a/sysdeps/powerpc/bits/endian.h +++ b/sysdeps/powerpc/bits/endian.h @@ -17,7 +17,7 @@ /* PowerPC can be little or big endian. Hopefully gcc will know... */ -#ifndef _ENDIAN_H +#ifndef _ENDIAN_T_H # error "Never use <bits/endian.h> directly; include <endian.h> instead." #endif diff --git a/sysdeps/riscv/bits/endian.h b/sysdeps/riscv/bits/endian.h index 4aaf559d4f4..c07ea5f748f 100644 --- a/sysdeps/riscv/bits/endian.h +++ b/sysdeps/riscv/bits/endian.h @@ -1,4 +1,4 @@ -#ifndef _ENDIAN_H +#ifndef _ENDIAN_T_H # error "Never use <bits/endian.h> directly; include <endian.h> instead." #endif diff --git a/sysdeps/s390/bits/endian.h b/sysdeps/s390/bits/endian.h index ac27f0119a8..a2b93390d5a 100644 --- a/sysdeps/s390/bits/endian.h +++ b/sysdeps/s390/bits/endian.h @@ -1,6 +1,6 @@ /* s390 is big-endian */ -#ifndef _ENDIAN_H +#ifndef _ENDIAN_T_H # error "Never use <bits/endian.h> directly; include <endian.h> instead." #endif diff --git a/sysdeps/sh/bits/endian.h b/sysdeps/sh/bits/endian.h index 1fef1ff938f..46f8fd3d3ec 100644 --- a/sysdeps/sh/bits/endian.h +++ b/sysdeps/sh/bits/endian.h @@ -1,6 +1,6 @@ /* SH is bi-endian but with a big-endian FPU. */ -#ifndef _ENDIAN_H +#ifndef _ENDIAN_T_H # error "Never use <bits/endian.h> directly; include <endian.h> instead." #endif diff --git a/sysdeps/sparc/bits/endian.h b/sysdeps/sparc/bits/endian.h index 8acfdf5df6f..60394ea43aa 100644 --- a/sysdeps/sparc/bits/endian.h +++ b/sysdeps/sparc/bits/endian.h @@ -1,7 +1,7 @@ /* Sparc is big-endian, but v9 supports endian conversion on loads/stores and GCC supports such a mode. Be prepared. */ -#ifndef _ENDIAN_H +#ifndef _ENDIAN_T_H # error "Never use <bits/endian.h> directly; include <endian.h> instead." #endif diff --git a/sysdeps/unix/sysv/linux/ia64/bits/endian.h b/sysdeps/unix/sysv/linux/ia64/bits/endian.h index 98a5e239916..8bb227ecd53 100644 --- a/sysdeps/unix/sysv/linux/ia64/bits/endian.h +++ b/sysdeps/unix/sysv/linux/ia64/bits/endian.h @@ -1,6 +1,6 @@ /* Linux/ia64 is little-endian. */ -#ifndef _ENDIAN_H +#ifndef _ENDIAN_T_H # error "Never use <bits/endian.h> directly; include <endian.h> instead." #endif diff --git a/sysdeps/x86/bits/endian.h b/sysdeps/x86/bits/endian.h index 5a56c726f71..c159872ae64 100644 --- a/sysdeps/x86/bits/endian.h +++ b/sysdeps/x86/bits/endian.h @@ -1,6 +1,6 @@ /* i386/x86_64 are little-endian. */ -#ifndef _ENDIAN_H +#ifndef _ENDIAN_T_H # error "Never use <bits/endian.h> directly; include <endian.h> instead." #endif diff --git a/time/bits/types/endian_t.h b/time/bits/types/endian_t.h new file mode 100644 index 00000000000..d68f5ba2f8b --- /dev/null +++ b/time/bits/types/endian_t.h @@ -0,0 +1,36 @@ +/* Copyright (C) 1992-2019 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 + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + <https://www.gnu.org/licenses/>. */ + +#ifndef _ENDIAN_T_H +#define _ENDIAN_T_H 1 + +/* Definitions for byte order, according to significance of bytes, + from low addresses to high addresses. The value is what you get by + putting '4' in the most significant byte, '3' in the second most + significant byte, '2' in the second least significant byte, and '1' + in the least significant byte, and then writing down one digit for + each byte, starting with the byte at the lowest address at the left, + and proceeding to the byte with the highest address at the right. */ + +#define __LITTLE_ENDIAN 1234 +#define __BIG_ENDIAN 4321 +#define __PDP_ENDIAN 3412 + +/* This file defines `__BYTE_ORDER' for the particular machine. */ +#include <bits/endian.h> + +#endif