[RFC] time: Create a endian_t.h headerfile
Commit Message
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.
2019-09-19 Alistair Francis <alistair.francis@wdc.com>
* bits/endian.h: Create a endian_t.h headerfile.
* string/endian.h: Create a endian_t.h headerfile.
* sysdeps/aarch64/bits/endian.h: Allow including from
endian_t.h.
* sysdeps/alpha/bits/endian.h: Likewise.
* sysdeps/arm/bits/endian.h: Likewise.
* sysdeps/csky/bits/endian.h: Likewise.
* sysdeps/hppa/bits/endian.h: Likewise.
* sysdeps/m68k/bits/endian.h: Likewise.
* sysdeps/microblaze/bits/endian.h: Likewise.
* sysdeps/mips/bits/endian.h: Likewise.
* sysdeps/nios2/bits/endian.h: Likewise.
* sysdeps/powerpc/bits/endian.h: Likewise.
* sysdeps/riscv/bits/endian.h: Likewise.
* sysdeps/s390/bits/endian.h: Likewise.
* sysdeps/sh/bits/endian.h: Likewise.
* sysdeps/sparc/bits/endian.h: Likewise.
* sysdeps/unix/sysv/linux/ia64/bits/endian.h: Likewise.
* sysdeps/x86/bits/endian.h: Likewise.
* time/bits/types/endian_t.h: New file.
---
This is based on the feedback from:
https://sourceware.org/ml/libc-alpha/2019-09/msg00272.html
bits/endian.h | 2 +-
string/endian.h | 16 +---------
sysdeps/aarch64/bits/endian.h | 2 +-
sysdeps/alpha/bits/endian.h | 2 +-
sysdeps/arm/bits/endian.h | 2 +-
sysdeps/csky/bits/endian.h | 2 +-
sysdeps/hppa/bits/endian.h | 2 +-
sysdeps/m68k/bits/endian.h | 2 +-
sysdeps/microblaze/bits/endian.h | 2 +-
sysdeps/mips/bits/endian.h | 2 +-
sysdeps/nios2/bits/endian.h | 2 +-
sysdeps/powerpc/bits/endian.h | 2 +-
sysdeps/riscv/bits/endian.h | 2 +-
sysdeps/s390/bits/endian.h | 2 +-
sysdeps/sh/bits/endian.h | 2 +-
sysdeps/sparc/bits/endian.h | 2 +-
sysdeps/unix/sysv/linux/ia64/bits/endian.h | 2 +-
sysdeps/x86/bits/endian.h | 2 +-
time/bits/types/endian_t.h | 36 ++++++++++++++++++++++
19 files changed, 54 insertions(+), 32 deletions(-)
create mode 100644 time/bits/types/endian_t.h
Comments
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
@@ -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
@@ -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. */
@@ -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
@@ -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
@@ -1,4 +1,4 @@
-#ifndef _ENDIAN_H
+#ifndef _ENDIAN_T_H
# error "Never use <bits/endian.h> directly; include <endian.h> instead."
#endif
@@ -1,4 +1,4 @@
-#ifndef _ENDIAN_H
+#ifndef _ENDIAN_T_H
# error "Never use <bits/endian.h> directly; include <endian.h> instead."
#endif
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -1,4 +1,4 @@
-#ifndef _ENDIAN_H
+#ifndef _ENDIAN_T_H
# error "Never use <bits/endian.h> directly; include <endian.h> instead."
#endif
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
new file mode 100644
@@ -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