Re: endian: New module.

[Collin Funk]

On 5/18/24 7:45 AM, Bruno Haible wrote:
>> Can you check the attached patch? I think that it should work or
>> at least be in the correct direction...
> 
> In the endian.in.h file: The comments on the 3 last lines are not
> consistent with the '#endif' scopes.

Oops, good catch.

> In the Makefile.am snippet:
> A few lines are indented with spaces, not with a tab. It would
> not cause an error here, since these are only continuation lines,
> I think. But nevertheless better be consistent with how other
> modules do it.

In my Emacs configuration I have a hook that makes makefile-mode use
tabs instead of spaces. For nearly all other modes I use spaces. In
the case of modules/* they are interpreted as text files, so the tab
key inserts spaces.

Perhaps I'll figure out a good way to deal with that if/when it starts
annoying me. Until then I can just insert regular tabs.

> Other than that, it looks correct.

I've applied this patch. Same as before but with those two things
fixed.

CollinFrom 83dd4db866cc5dde2fddcf1944f3b5cc3732e48e Mon Sep 17 00:00:00 2001
From: Collin Funk 
Date: Sat, 18 May 2024 06:36:55 -0700
Subject: [PATCH] endian: Make sure system headers can be included.

Reported by Bruno Haible in
.

* lib/endian.in.h (be16toh, be32toh, be64toh, htobe16, htobe32, htobe64)
(le16toh, le32toh, le64toh, htole16, htole32, htole64): Don't define
functions if the system has working versions.
* m4/endian_h.m4 (gl_ENDIAN_H): Separate checks for stdint types and
proper macro/function definitions.
* modules/endian (Depends-on): Add include_next. Update module
dependency conditions.
(Makefile.am): Perform sed replacements on the header substitute.
---
 ChangeLog   | 14 ++
 lib/endian.in.h | 46 +---
 m4/endian_h.m4  | 70 +++--
 modules/endian  | 15 ---
 4 files changed, 120 insertions(+), 25 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 7de9b90f6d..3ab2dc021a 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,17 @@
+2024-05-18  Collin Funk  
+
+	endian: Make sure system headers can be included.
+	Reported by Bruno Haible in
+	.
+	* lib/endian.in.h (be16toh, be32toh, be64toh, htobe16, htobe32, htobe64)
+	(le16toh, le32toh, le64toh, htole16, htole32, htole64): Don't define
+	functions if the system has working versions.
+	* m4/endian_h.m4 (gl_ENDIAN_H): Separate checks for stdint types and
+	proper macro/function definitions.
+	* modules/endian (Depends-on): Add include_next. Update module
+	dependency conditions.
+	(Makefile.am): Perform sed replacements on the header substitute.
+
 2024-05-18  Bruno Haible  
 
 	abort-debug: Integrate with CONTINUE_AFTER_ASSERT.
diff --git a/lib/endian.in.h b/lib/endian.in.h
index 5d755fd7cf..bd65ae8aab 100644
--- a/lib/endian.in.h
+++ b/lib/endian.in.h
@@ -17,8 +17,30 @@
 
 /* Written by Collin Funk.  */
 
-#ifndef _GL_ENDIAN_H
-#define _GL_ENDIAN_H 1
+#ifndef _@GUARD_PREFIX@_ENDIAN_H
+
+#if __GNUC__ >= 3
+@PRAGMA_SYSTEM_HEADER@
+#endif
+@PRAGMA_COLUMNS@
+
+#if @HAVE_ENDIAN_H@
+
+/* The include_next requires a split double-inclusion guard.  */
+# @INCLUDE_NEXT@ @NEXT_ENDIAN_H@
+
+#endif
+
+
+/* glibc defines all macros and functions but is missing types from
+   stdint.h.  */
+#if @ENDIAN_H_JUST_MISSING_STDINT@
+# include 
+#else
+
+/* Others platforms.  */
+#ifndef _@GUARD_PREFIX@_ENDIAN_H
+#define _@GUARD_PREFIX@_ENDIAN_H 1
 
 /* This file uses _GL_INLINE, WORDS_BIGENDIAN.  */
 #if !_GL_CONFIG_H_INCLUDED
@@ -47,6 +69,22 @@ _GL_INLINE_HEADER_BEGIN
 # define BYTE_ORDER LITTLE_ENDIAN
 #endif
 
+/* Make sure function-like macros get undefined.  */
+#if @HAVE_ENDIAN_H@
+# undef be16toh
+# undef be32toh
+# undef be64toh
+# undef htobe16
+# undef htobe32
+# undef htobe64
+# undef le16toh
+# undef le32toh
+# undef le64toh
+# undef htole16
+# undef htole32
+# undef htole64
+#endif
+
 #ifdef __cplusplus
 extern "C" {
 #endif
@@ -193,4 +231,6 @@ htole64 (uint64_t x)
 
 _GL_INLINE_HEADER_END
 
-#endif /* _GL_ENDIAN_H */
+#endif /* @ENDIAN_H_JUST_MISSING_STDINT@ */
+#endif /* _@GUARD_PREFIX@_ENDIAN_H */
+#endif /* _@GUARD_PREFIX@_ENDIAN_H */
diff --git a/m4/endian_h.m4 b/m4/endian_h.m4
index ec0d111ae3..29dab603e3 100644
--- a/m4/endian_h.m4
+++ b/m4/endian_h.m4
@@ -1,5 +1,5 @@
 # endian_h.m4
-# serial 1
+# serial 2
 dnl Copyright 2024 Free Software Foundation, Inc.
 dnl This file is free software; the Free Software Foundation
 dnl gives unlimited permission to copy and/or distribute it,
@@ -12,8 +12,25 @@ AC_DEFUN_ONCE([gl_ENDIAN_H]
   AC_REQUIRE([gl_BIGENDIAN])
 
   AC_CHECK_HEADERS_ONCE([endian.h])
+  gl_CHECK_NEXT_HEADERS([endian.h])
   if test $ac_cv_header_endian_h = yes; then
-AC_CACHE_CHECK([if endian.h conforms to POSIX],
+HAVE_ENDIAN_H=1
+dnl Check if endian.h defines uint16_t, uint32_t, and uint64_t.
+AC_CACHE_CHECK([if endian.h 

Re: endian: New module.

[Bruno Haible]

Hi Collin,

> Can you check the attached patch? I think that it should work or
> at least be in the correct direction...

In the endian.in.h file: The comments on the 3 last lines are not
consistent with the '#endif' scopes.

In the Makefile.am snippet:
A few lines are indented with spaces, not with a tab. It would
not cause an error here, since these are only continuation lines,
I think. But nevertheless better be consistent with how other
modules do it.

Other than that, it looks correct.

> I've split the configure checks into one checking for uint16_t, etc.
> and the other checking for macros/function definitions. The reason for
> this is that glibc defines macros/functions properly, so we just need
> the stdint.h types. I assume this might occur on other systems but
> that is just a guess.

Yes, that's the right way to do this.

> For other platforms the configure checks might fail because
> function-like macros are used. In that case we can include endian.h
> from the system and #undef the macros before our own definition.

OK.

> Also, double checking that I understand the #include_next idioms would
> be appreciated.

You have applied these idioms correctly, yes.

> Feel free to push this patch if it is okay. I don't want this module
> to mess with your CI jobs.

Thanks for the consideration. But I don't think it will mess with my
CI runs, because they run on newer glibc versions.

Bruno

Re: endian: New module.

[Collin Funk]

On 5/18/24 4:46 AM, Bruno Haible wrote:
>> I see. What is the correct solution here?
> 
> See the idioms used by, say, arpa_inet.in.h.
> 
>> Or something like this:
>>
>> #if @HAVE_ENDIAN_H@
>> # @INCLUDE_NEXT@ @NEXT_ENDIAN_H@
>> #endif
> 
> This is part of it. But there's more details needed.

Can you check the attached patch? I think that it should work or
at least be in the correct direction...

On my system in a testdir with all modules I don't get the error, so
it is a bit hard to test. :(

I've split the configure checks into one checking for uint16_t, etc.
and the other checking for macros/function definitions. The reason for
this is that glibc defines macros/functions properly, so we just need
the stdint.h types. I assume this might occur on other systems but
that is just a guess.

For other platforms the configure checks might fail because
function-like macros are used. In that case we can include endian.h
from the system and #undef the macros before our own definition.

Also, double checking that I understand the #include_next idioms would
be appreciated.

Feel free to push this patch if it is okay. I don't want this module
to mess with your CI jobs.

Collin From 670b8be73f4b79d4a6993dafcd12ae81721a85b6 Mon Sep 17 00:00:00 2001
From: Collin Funk 
Date: Sat, 18 May 2024 06:36:55 -0700
Subject: [PATCH] endian: Make sure system headers can be included.

Reported by Bruno Haible in
.

* lib/endian.in.h (be16toh, be32toh, be64toh, htobe16, htobe32, htobe64)
(le16toh, le32toh, le64toh, htole16, htole32, htole64): Don't define
functions if the system has working versions.
* m4/endian_h.m4 (gl_ENDIAN_H): Separate checks for stdint types and
proper macro/function definitions.
* modules/endian (Depends-on): Add include_next. Update module
dependency conditions.
(Makefile.am): Perform sed replacements on the header substitute.
---
 ChangeLog   | 14 ++
 lib/endian.in.h | 46 +---
 m4/endian_h.m4  | 70 +++--
 modules/endian  | 15 ---
 4 files changed, 120 insertions(+), 25 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 29adf56ab7..90953430be 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,17 @@
+2024-05-18  Collin Funk  
+
+	endian: Make sure system headers can be included.
+	Reported by Bruno Haible in
+	.
+	* lib/endian.in.h (be16toh, be32toh, be64toh, htobe16, htobe32, htobe64)
+	(le16toh, le32toh, le64toh, htole16, htole32, htole64): Don't define
+	functions if the system has working versions.
+	* m4/endian_h.m4 (gl_ENDIAN_H): Separate checks for stdint types and
+	proper macro/function definitions.
+	* modules/endian (Depends-on): Add include_next. Update module
+	dependency conditions.
+	(Makefile.am): Perform sed replacements on the header substitute.
+
 2024-05-18  Bruno Haible  
 
 	endian tests: Verify that it can be used from C++.
diff --git a/lib/endian.in.h b/lib/endian.in.h
index 5d755fd7cf..65473201ea 100644
--- a/lib/endian.in.h
+++ b/lib/endian.in.h
@@ -17,8 +17,30 @@
 
 /* Written by Collin Funk.  */
 
-#ifndef _GL_ENDIAN_H
-#define _GL_ENDIAN_H 1
+#ifndef _@GUARD_PREFIX@_ENDIAN_H
+
+#if __GNUC__ >= 3
+@PRAGMA_SYSTEM_HEADER@
+#endif
+@PRAGMA_COLUMNS@
+
+#if @HAVE_ENDIAN_H@
+
+/* The include_next requires a split double-inclusion guard.  */
+# @INCLUDE_NEXT@ @NEXT_ENDIAN_H@
+
+#endif
+
+
+/* glibc defines all macros and functions but is missing types from
+   stdint.h.  */
+#if @ENDIAN_H_JUST_MISSING_STDINT@
+# include 
+#else
+
+/* Others platforms.  */
+#ifndef _@GUARD_PREFIX@_ENDIAN_H
+#define _@GUARD_PREFIX@_ENDIAN_H 1
 
 /* This file uses _GL_INLINE, WORDS_BIGENDIAN.  */
 #if !_GL_CONFIG_H_INCLUDED
@@ -47,6 +69,22 @@ _GL_INLINE_HEADER_BEGIN
 # define BYTE_ORDER LITTLE_ENDIAN
 #endif
 
+/* Make sure function-like macros get undefined.  */
+#if @HAVE_ENDIAN_H@
+# undef be16toh
+# undef be32toh
+# undef be64toh
+# undef htobe16
+# undef htobe32
+# undef htobe64
+# undef le16toh
+# undef le32toh
+# undef le64toh
+# undef htole16
+# undef htole32
+# undef htole64
+#endif
+
 #ifdef __cplusplus
 extern "C" {
 #endif
@@ -193,4 +231,6 @@ htole64 (uint64_t x)
 
 _GL_INLINE_HEADER_END
 
-#endif /* _GL_ENDIAN_H */
+#endif /* _@GUARD_PREFIX@_ENDIAN_H */
+#endif /* _@GUARD_PREFIX@_ENDIAN_H */
+#endif /* @ENDIAN_H_JUST_MISSING_STDINT@ */
diff --git a/m4/endian_h.m4 b/m4/endian_h.m4
index ec0d111ae3..29dab603e3 100644
--- a/m4/endian_h.m4
+++ b/m4/endian_h.m4
@@ -1,5 +1,5 @@
 # endian_h.m4
-# serial 1
+# serial 2
 dnl Copyright 2024 Free Software Foundation, Inc.
 dnl This file is free software; the Free Software Foundation
 dnl gives unlimited permission to copy and/or distribute it,
@@ -12,8 +12,25 @@ AC_DEFUN_ONCE([gl_ENDIAN_H]
   AC_REQUIRE([gl_BIGENDIAN])
 
   AC_CHECK_HEADERS_ONCE([endian.h])
+  gl_CHECK_NEXT_HEADERS([endian.h])
   if test $ac_cv_header_endian_h 

Re: endian: New module.

[Bruno Haible]

Hi Collin,

> I sent a request for a compile farm account today since I realized I
> have no way of testing other architectures (other then VMs I guess).

That's reasonable. In particular, one cannot install macOS or AIX
in a VM, therefore for these two OSes the compilefarm is the only way
to get an interactive ssh access.

> I see. What is the correct solution here?

See the idioms used by, say, arpa_inet.in.h.

> Or something like this:
> 
> #if @HAVE_ENDIAN_H@
> # @INCLUDE_NEXT@ @NEXT_ENDIAN_H@
> #endif

This is part of it. But there's more details needed.

Bruno

Re: endian: New module.

[Collin Funk]

Hi Bruno,

On 5/18/24 4:12 AM, Bruno Haible wrote:
> While testing a testdir for module 'endian' on gcc110.fsffrance.org
> (in the GCC compilefarm), I get a compilation error. This machine has
> a pretty old libc (glibc 2.17), but that ought to work anyway.

Oops, sorry.

I sent a request for a compile farm account today since I realized I
have no way of testing other architectures (other then VMs I guess).

> In other words: When overriding a system header, you must conditionally
> #include_next that system header. (There are exception to this rule,
> such as  or . But in general this rule is valid.)
> The system header can have made any number of additional definitions
> or declarations, and other system headers (and user code) may rely on
> these definitions or declarations.

I see. What is the correct solution here? I guess you could define
__MACROS but that is a bit hacky in my opinion.

Maybe defining a macro for ALMOST_WORKING_ENDIAN_H (i.e. missing types).

Or something like this:

#if @HAVE_ENDIAN_H@
# @INCLUDE_NEXT@ @NEXT_ENDIAN_H@
#endif

/* Undefine macros.  */
#undef betoh16
#undef betoh32
#undef betoh64
[]

/* Override them if they are functions.  */
#define betoh16 __gl_betoh16
#define betoh32 __gl_betoh32
#define betoh64 __gl_betoh64

I haven't ever used the '#include_next' directive so I am just basing
this off of what I have seen in Gnulib.

Collin

Re: endian: New module.

[Bruno Haible]

Hi Collin,

While testing a testdir for module 'endian' on gcc110.fsffrance.org
(in the GCC compilefarm), I get a compilation error. This machine has
a pretty old libc (glibc 2.17), but that ought to work anyway.

gcc -std=gnu11 -DHAVE_CONFIG_H -I. -I../../gltests -I..  
-DGNULIB_STRICT_CHECKING=1 -DIN_GNULIB_TESTS=1 -I. -I../../gltests -I.. 
-I../../gltests/.. -I../gllib -I../../gltests/../gllib 
-I/home/haible/prefix64/include -Wall  -Wno-error -g -O2 -MT test-alignasof.o 
-MD -MP -MF .deps/test-alignasof.Tpo -c -o test-alignasof.o 
../../gltests/test-alignasof.c
In file included from /usr/include/stdlib.h:42:0,
 from ./stdlib.h:36,
 from ../../gltests/macros.h:22,
 from ../../gltests/test-alignasof.c:24:
/usr/include/bits/waitstatus.h:79:15: error: duplicate member ‘__w_retcode’
  unsigned int __w_retcode:8;
   ^
/usr/include/bits/waitstatus.h:80:15: error: duplicate member ‘__w_coredump’
  unsigned int __w_coredump:1;
   ^
/usr/include/bits/waitstatus.h:81:15: error: duplicate member ‘__w_termsig’
  unsigned int __w_termsig:7;
   ^
/usr/include/bits/waitstatus.h:93:15: error: duplicate member ‘__w_stopsig’
  unsigned int __w_stopsig:8; /* Stopping signal.  */
   ^
/usr/include/bits/waitstatus.h:94:15: error: duplicate member ‘__w_stopval’
  unsigned int __w_stopval:8; /* W_STOPPED if stopped.  */
   ^
make[3]: *** [test-alignasof.o] Error 1

Preprocessing with "-E -dD" shows this snippet:

-
# 65 "/usr/include/bits/waitstatus.h" 2 3 4

union wait
  {
int w_status;
struct
  {

 unsigned int __w_termsig:7;
 unsigned int __w_coredump:1;
 unsigned int __w_retcode:8;
 unsigned int:16;


 unsigned int:16;
 unsigned int __w_retcode:8;
 unsigned int __w_coredump:1;
 unsigned int __w_termsig:7;

  } __wait_terminated;
-
This is obviously broken. It originated from this source code:

-
union wait
  {
int w_status;
struct
  {
# if__BYTE_ORDER == __LITTLE_ENDIAN
unsigned int __w_termsig:7; /* Terminating signal.  */
unsigned int __w_coredump:1; /* Set if dumped core.  */
unsigned int __w_retcode:8; /* Return code if exited normally.  */
unsigned int:16;
# endif /* Little endian.  */
# if__BYTE_ORDER == __BIG_ENDIAN
unsigned int:16;
unsigned int __w_retcode:8;
unsigned int __w_coredump:1;
unsigned int __w_termsig:7;
# endif /* Big endian.  */
  } __wait_terminated;
-

which is correct.

The problem is that configure found:

  checking for endian.h... yes
  checking whether byte ordering is bigendian... yes
  checking if endian.h conforms to POSIX... no

and thus a replacement gllib/endian.h was generated. But it does not include
the original :

$ grep '# *include' gllib/endian.h
#include 
#include 

and thus does not define the __BYTE_ORDER, __LITTLE_ENDIAN,
__BIG_ENDIAN macros that the other system header ()
expects.

In other words: When overriding a system header, you must conditionally
#include_next that system header. (There are exception to this rule,
such as  or . But in general this rule is valid.)
The system header can have made any number of additional definitions
or declarations, and other system headers (and user code) may rely on
these definitions or declarations.

Bruno

Re: endian: New module.

[Bruno Haible]

Hi Collin,

> I've pushed the two patches adding a module to substitute endian.h.
> Pretty much the same as last time except using inline functions
> instead of macros and making sure variables are used in the m4 file.

Thanks!

> I've only mentioned the module in the documentation. It might make
> sense to put it in posix-headers/* once the next POSIX revision is
> actually released. Since now it is just a draft and is subject to
> change.

Yes. For now, it's sufficient to point talk about "Future POSIX specification".

> Also, the next POSIX revision seems like it is going to require
> int64_t and uint64_t support [2].

Yes; see also https://www.austingroupbugs.net/view.php?id=1799=2 .

> I've left the 64-bit functions
> #ifdef'd out though. I figured that was better since the module can be
> used as a dependency or in programs who still support old systems
> (assuming the 64-bit versions don't get used of course). I'm not sure
> if it is worth splitting things into separate modules just for that
> though.

I agree, separate modules would be overkill here.

> Also I ran the test cases on GCC, Clang, and Oracle CC, all x86-64. I
> don't have access to any other architectures. Someone running them on
> a big endian system would be very much appreciated, to catch any typos
> I may have made there. :)

Good point. I'm testing it on glibc/powerpc64 (in a VM) and AIX/powerpc64
(in the GCC compilefarm).

In the doc, the platforms list is less up-to-date than what our current
database has. See:

$ cd maint-tools/platforms/various-includes
$ ./show-portability --doc endian.h
macOS 11.1, FreeBSD 13.0, NetBSD 7.1, OpenBSD 3.8, Minix 3.3.0, AIX 7.3.1, 
HP-UX 11.31, IRIX 6.5, Solaris 11.4, mingw, MSVC 14.

Also, let me add a test for "These macros shall be suitable for use in #if
preprocessing directives." And a C++ test.


2024-05-18  Bruno Haible  

endian tests: Verify that it can be used from C++.
* tests/test-endian-c++.cc: New file.
* modules/endian-c++-tests: New file.
* modules/endian-tests (Depends-on): Add endian-c++-tests.

2024-05-18  Bruno Haible  

endian: Update doc and strengthen tests.
* doc/glibc-headers/endian.texi: Reference LSB and future POSIX
specifications. Update platforms list.
* tests/test-endian.c: Verify that BYTE_ORDER, LITTLE_ENDIAN, BIG_ENDIAN
can be used in #if.

>From 1c0b8a75b49ed9d7fcee2b247a6afb585cb19a75 Mon Sep 17 00:00:00 2001
From: Bruno Haible 
Date: Sat, 18 May 2024 12:25:41 +0200
Subject: [PATCH 1/2] endian: Update doc and strengthen tests.

* doc/glibc-headers/endian.texi: Reference LSB and future POSIX
specifications. Update platforms list.
* tests/test-endian.c: Verify that BYTE_ORDER, LITTLE_ENDIAN, BIG_ENDIAN
can be used in #if.
---
 ChangeLog |  8 
 doc/glibc-headers/endian.texi |  8 ++--
 tests/test-endian.c   | 10 ++
 3 files changed, 24 insertions(+), 2 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 217e0dd059..322aaf362d 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,11 @@
+2024-05-18  Bruno Haible  
+
+	endian: Update doc and strengthen tests.
+	* doc/glibc-headers/endian.texi: Reference LSB and future POSIX
+	specifications. Update platforms list.
+	* tests/test-endian.c: Verify that BYTE_ORDER, LITTLE_ENDIAN, BIG_ENDIAN
+	can be used in #if.
+
 2024-05-18  Collin Funk  
 
 	endian: Add tests.
diff --git a/doc/glibc-headers/endian.texi b/doc/glibc-headers/endian.texi
index 5c7cb72429..ebaeb4fe7f 100644
--- a/doc/glibc-headers/endian.texi
+++ b/doc/glibc-headers/endian.texi
@@ -1,7 +1,11 @@
 @node endian.h
 @section @file{endian.h}
 
-Describe's the platform's endianness (byte ordering of words stored in memory).
+LSB specification:@* @url{https://refspecs.linuxbase.org/LSB_5.0.0/LSB-Core-generic/LSB-Core-generic/libc-ddefs.html}
+
+Future POSIX specification:@* @url{https://www.austingroupbugs.net/view.php?id=162}
+
+Describes the platform's endianness (byte ordering of words stored in memory).
 Defines the macros @code{BYTE_ORDER}, @code{LITTLE_ENDIAN}, @code{BIG_ENDIAN},
 @code{PDP_ENDIAN}.
 
@@ -11,7 +15,7 @@
 @itemize
 @item
 This header file is missing on some platforms:
-macOS 11.1, FreeBSD 13.0, NetBSD 7.1, OpenBSD 3.8, Minix 3.1.8, AIX 5.1, HP-UX 11, Solaris 11.4, mingw, MSVC 14.
+macOS 11.1, FreeBSD 13.0, NetBSD 7.1, OpenBSD 3.8, Minix 3.1.8, AIX 7.3.1, HP-UX 11.31, Solaris 11.4, mingw, MSVC 14.
 @end itemize
 
 Portability problems not fixed by Gnulib:
diff --git a/tests/test-endian.c b/tests/test-endian.c
index 919faa56f5..739b18964e 100644
--- a/tests/test-endian.c
+++ b/tests/test-endian.c
@@ -30,7 +30,17 @@ uint32_t t2;
 uint64_t t3;
 #endif
 
+/* "These macros shall be suitable for use in #if preprocessing directives."  */
+#if BYTE_ORDER == LITTLE_ENDIAN
+int a = 17;
+#endif
+#if BYTE_ORDER == BIG_ENDIAN
+int a = 19;
+#endif
+
+/* "The macros BIG_ENDIAN and LITTLE_ENDIAN shall have distinct values."  */
 

endian: New module.

[Collin Funk]

I've pushed the two patches adding a module to substitute endian.h.
Pretty much the same as last time except using inline functions
instead of macros and making sure variables are used in the m4 file.

I've only mentioned the module in the documentation. It might make
sense to put it in posix-headers/* once the next POSIX revision is
actually released. Since now it is just a draft and is subject to
change.

Right now the configure test probably fails on all platforms. POSIX
requires it to define uint16_t, uint32_t, and uint64_t but glibc
doesn't. I've submitted a bug report for that [1].

Also, the next POSIX revision seems like it is going to require
int64_t and uint64_t support [2]. I've left the 64-bit functions
#ifdef'd out though. I figured that was better since the module can be
used as a dependency or in programs who still support old systems
(assuming the 64-bit versions don't get used of course). I'm not sure
if it is worth splitting things into separate modules just for that
though.

Also I ran the test cases on GCC, Clang, and Oracle CC, all x86-64. I
don't have access to any other architectures. Someone running them on
a big endian system would be very much appreciated, to catch any typos
I may have made there. :)

Collin

[1] https://sourceware.org/bugzilla/show_bug.cgi?id=31749
[2] https://austingroupbugs.net/view.php?id=1799From 95f7085c87236bea297f9251ecb58e0bd821552c Mon Sep 17 00:00:00 2001
From: Collin Funk 
Date: Sat, 18 May 2024 00:10:33 -0700
Subject: [PATCH 1/2] endian: New module.

* doc/glibc-headers/endian.texi, doc/gnulib-tool.texi: Mention it.
* lib/endian.c: New file.
* lib/endian.in.h: New file.
* m4/endian_h.m4: New file.
* modules/endian: New file.
---
 ChangeLog |   9 ++
 doc/glibc-headers/endian.texi |   8 +-
 doc/gnulib-tool.texi  |   1 +
 lib/endian.c  |  23 
 lib/endian.in.h   | 196 ++
 m4/endian_h.m4|  71 
 modules/endian|  44 
 7 files changed, 348 insertions(+), 4 deletions(-)
 create mode 100644 lib/endian.c
 create mode 100644 lib/endian.in.h
 create mode 100644 m4/endian_h.m4
 create mode 100644 modules/endian

diff --git a/ChangeLog b/ChangeLog
index 8ddc85b138..4c2d9b516d 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,12 @@
+2024-05-18  Collin Funk  
+
+	endian: New module.
+	* doc/glibc-headers/endian.texi, doc/gnulib-tool.texi: Mention it.
+	* lib/endian.c: New file.
+	* lib/endian.in.h: New file.
+	* m4/endian_h.m4: New file.
+	* modules/endian: New file.
+
 2024-05-17  Bruno Haible  
 
 	tests: Fix link errors (regression today).
diff --git a/doc/glibc-headers/endian.texi b/doc/glibc-headers/endian.texi
index c45b875501..5c7cb72429 100644
--- a/doc/glibc-headers/endian.texi
+++ b/doc/glibc-headers/endian.texi
@@ -5,15 +5,15 @@ @node endian.h
 Defines the macros @code{BYTE_ORDER}, @code{LITTLE_ENDIAN}, @code{BIG_ENDIAN},
 @code{PDP_ENDIAN}.
 
-Gnulib module: ---
+Gnulib module: endian
 
 Portability problems fixed by Gnulib:
 @itemize
+@item
+This header file is missing on some platforms:
+macOS 11.1, FreeBSD 13.0, NetBSD 7.1, OpenBSD 3.8, Minix 3.1.8, AIX 5.1, HP-UX 11, Solaris 11.4, mingw, MSVC 14.
 @end itemize
 
 Portability problems not fixed by Gnulib:
 @itemize
-@item
-This header file is missing on some platforms:
-macOS 11.1, FreeBSD 13.0, NetBSD 7.1, OpenBSD 3.8, Minix 3.1.8, AIX 5.1, HP-UX 11, Solaris 11.4, mingw, MSVC 14.
 @end itemize
diff --git a/doc/gnulib-tool.texi b/doc/gnulib-tool.texi
index 5a2fd19cac..116734f4d7 100644
--- a/doc/gnulib-tool.texi
+++ b/doc/gnulib-tool.texi
@@ -561,6 +561,7 @@ @node Style of #include statements
 @item @code{assert.h}
 @item @code{ctype.h}
 @item @code{dirent.h}
+@item @code{endian.h}
 @item @code{errno.h}
 @item @code{fcntl.h}
 @item @code{fenv.h}
diff --git a/lib/endian.c b/lib/endian.c
new file mode 100644
index 00..3e7e56f523
--- /dev/null
+++ b/lib/endian.c
@@ -0,0 +1,23 @@
+/* Inline functions for .
+
+   Copyright 2024 Free Software Foundation, Inc.
+
+   This file 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.
+
+   This file 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 this program.  If not, see <https://www.gnu.org/licenses/>.  */
+
+/* Written by Collin Funk.  */
+
+#include 
+
+#define _GL_ENDIAN_INLINE _GL_EXTERN_INLINE
+#include 
diff --git a/lib/endian.in.h b/lib/endian.in.h
new file mode 100644
index 00..5d755fd7cf
--- /dev/null
+++ b/lib/endia

Re: endian: New module.

[Collin Funk]

On 5/15/24 12:03 PM, Paul Eggert wrote:
>> If we can ensure byteswap.h functions are defined as functions,
>> wouldn't it make sense to just define these as macros to them?
> 
> Not sure why macros would be helpful. If functions suffice for good 
> performance when compiling with -O2, it's better to use functions than macros.

True. I'll revisit my patches and perfer inline functions. Your stdbit
implementation serves as a good example for me. Thanks!

Collin

Re: endian: New module.

[Paul Eggert]

On 2024-05-13 15:34, Collin Funk wrote:

To fix this, please use _GL_INLINE and implement with inline functions. And add 
please add test cases to catch these issues.

If we can ensure byteswap.h functions are defined as functions,
wouldn't it make sense to just define these as macros to them?


Not sure why macros would be helpful. If functions suffice for good 
performance when compiling with -O2, it's better to use functions than 
macros.

Re: endian: New module.

[Collin Funk]

On 5/13/24 10:40 AM, Paul Eggert wrote:
> Unfortunately this patch suffers from the problem we discussed earlier, e.g., 
> the substitute be16toh (n++) has undefined behavior but it should add 1 to n 
> and otherwise act as if be16toh (n) was called.
[...]
> These problems arise because Gnulib byteswap.h's macros can evaluate their 
> arguments more than once, and are type-generic in a way that is incompatible 
> with what POSIX wants for endian.h functions.

I think that this should be addressed in byteswap.h as well. Since
glibc defines them as inline functions. How about the two attached
patches. The version numbers for __builtin_checks are copied from my
/bits/byteswap.h.

> Also, there's a bizarre compatibility issue, in that some floating-point args 
> have well-defined behavior in the POSIX spec, e.g., be16toh (0.0) yields 0, 
> but this implementation rejects these calls. Although this is lower priority 
> it's easy to fix so we might as well do it.

Ah, my floating point knowledge is not so great...

> To fix this, please use _GL_INLINE and implement with inline functions. And 
> add please add test cases to catch these issues.

If we can ensure byteswap.h functions are defined as functions,
wouldn't it make sense to just define these as macros to them?

CollinFrom f4a34789c8f55161c1cab343a97511206761ff47 Mon Sep 17 00:00:00 2001
From: Collin Funk 
Date: Mon, 13 May 2024 15:05:33 -0700
Subject: [PATCH 1/2] byteswap: Use inline functions instead of macros.

* lib/byteswap.c: New file.
* lib/byteswap.in.h (bswap_16, bswap_32, bswap_64): Use inline functions
so arguments are evaluated once.
* modules/byteswap (Files): Add lib/byteswap.c.
(Depends-on): Add extern-inline. Add stdint as a conditional dependency.
(Makefile.am): Add lib/byteswap.c to lib_SOURCES.
---
 ChangeLog | 10 +++
 lib/byteswap.c| 21 ++
 lib/byteswap.in.h | 74 ++-
 modules/byteswap  |  4 +++
 4 files changed, 95 insertions(+), 14 deletions(-)
 create mode 100644 lib/byteswap.c

diff --git a/ChangeLog b/ChangeLog
index 09cdfb44f9..5a9354da16 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,13 @@
+2024-05-13  Collin Funk  
+
+	byteswap: Use inline functions instead of macros.
+	* lib/byteswap.c: New file.
+	* lib/byteswap.in.h (bswap_16, bswap_32, bswap_64): Use inline functions
+	so arguments are evaluated once.
+	* modules/byteswap (Files): Add lib/byteswap.c.
+	(Depends-on): Add extern-inline. Add stdint as a conditional dependency.
+	(Makefile.am): Add lib/byteswap.c to lib_SOURCES.
+
 2024-05-13  Bruno Haible  
 
 	doc: Document our conventions for *.m4 files.
diff --git a/lib/byteswap.c b/lib/byteswap.c
new file mode 100644
index 00..6e3dad74ea
--- /dev/null
+++ b/lib/byteswap.c
@@ -0,0 +1,21 @@
+/* Inline functions for .
+
+   Copyright 2024 Free Software Foundation, Inc.
+
+   This file 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.
+
+   This file 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 this program.  If not, see .  */
+
+#include 
+
+#define _GL_BYTESWAP_INLINE _GL_EXTERN_INLINE
+#include 
diff --git a/lib/byteswap.in.h b/lib/byteswap.in.h
index 8e49efad05..24070f3240 100644
--- a/lib/byteswap.in.h
+++ b/lib/byteswap.in.h
@@ -18,27 +18,73 @@
 #ifndef _GL_BYTESWAP_H
 #define _GL_BYTESWAP_H
 
+/* This file uses _GL_INLINE.  */
+#if !_GL_CONFIG_H_INCLUDED
+ #error "Please include config.h first."
+#endif
+
+#include 
+
+_GL_INLINE_HEADER_BEGIN
+#ifndef _GL_BYTESWAP_INLINE
+# define _GL_BYTESWAP_INLINE _GL_INLINE
+#endif
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
 /* Given an unsigned 16-bit argument X, return the value corresponding to
X with reversed byte order.  */
-#define bswap_16(x) x) & 0x00FF) << 8) | \
- (((x) & 0xFF00) >> 8))
+_GL_BYTESWAP_INLINE uint16_t
+bswap_16 (uint16_t x)
+{
+#if (defined __GNUC__ && (__GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 8))) \
+ || (defined __has_builtin && __has_builtin (__builtin_bswap16))
+  return __builtin_bswap16 (x);
+#else
+  return (x) >> 8) & 0xff) | (((x) & 0xff) << 8)));
+#endif
+}
 
 /* Given an unsigned 32-bit argument X, return the value corresponding to
X with reversed byte order.  */
-#define bswap_32(x) x) & 0x00FF) << 24) | \
- (((x) & 0xFF00) << 8) | \
- (((x) & 0x00FF) >> 8) | \
- (((x) & 0xFF00) >> 24))
+_GL_BYTESWAP_INLINE uint32_t
+bswap_32 

Re: endian: New module.

[Collin Funk]

Hi Bruno,

On 5/13/24 5:38 AM, Bruno Haible wrote:
>> Let me know if I missed anything. Other than the docs and test module
>> which I can do later today (assuming this patch is decent enough).
> 
> The doc changes belong in the same commit. The tests module should be
> in a different commit, but it's a good idea to develop both together,
> since the tests often uncover problems.

Sounds good. I was working on tests at the same time but left it out
of the patch.

> But glibc defines PDP_ENDIAN in , and Gnulib attempts to be
> compliant to POSIX _and_ source-compatible with glibc.
> 
> Also there is some code that uses PDP_ENDIAN [1].

Ah, okay. I was thinking it was mostly unused.

> Why do you mention the functions in this order?
> +/* Big endian to host.  */
> +/* Host to big endian.  */
> +/* Host to little endian.  */
> +/* Little endian to host.  */
> Doing things backwards in the second half than in the first half is
> quite unusual; it's a particular property of the Morse sequence [2].
> (Note that alphabetical order of items is only to be used as a fallback,
> when items cannot be ordered by some form of logic or heuristic.)

Interesting link. Yes, I guess it is a strange ordering. I didn't
think about it to much. The POSIX draft has function prototypes in
that order, so I just transformed them into macros.

> In the autoconf test, compilation with -Wall would produce warnings about
> the 12 unused variables. If a user sets CC="gcc -Wall -Werror", this will
> make the compilation fail. Although this setting of CC is not supported by
> Gnulib, it does not hurt to make things work in this case. Like done in
> m4/intmax_t.m4: Add a 'return' statement that uses all the variables.
> Such as
>   return !(value16_1 + value32_1 + value64_1 + value16_2 + value32_2 + 
> value64_2
>+ value16_3 + value32_3 + value64_3 + value16_4 + value32_4 + 
> value64_4);

Ah, I forgot that I have to worry about it. Thanks for the tip. I was
using stdint_h.m4 and sys_time_h.m4 to write that m4 file.

Collin

Re: endian: New module.

[Paul Eggert]

Unfortunately this patch suffers from the problem we discussed earlier, 
e.g., the substitute be16toh (n++) has undefined behavior but it should 
add 1 to n and otherwise act as if be16toh (n) was called.


Also, the returned values and types are sometimes wrong. E.g., on x86-64 
be16toh (-1) should return 0x of type uint16_t, but it returns -1 of 
type int.


These problems arise because Gnulib byteswap.h's macros can evaluate 
their arguments more than once, and are type-generic in a way that is 
incompatible with what POSIX wants for endian.h functions.


Also, there's a bizarre compatibility issue, in that some floating-point 
args have well-defined behavior in the POSIX spec, e.g., be16toh (0.0) 
yields 0, but this implementation rejects these calls. Although this is 
lower priority it's easy to fix so we might as well do it.


To fix this, please use _GL_INLINE and implement with inline functions. 
And add please add test cases to catch these issues.

Re: endian: New module.

[Bruno Haible]

Hi Collin,

> Let me know if I missed anything. Other than the docs and test module
> which I can do later today (assuming this patch is decent enough).

The doc changes belong in the same commit. The tests module should be
in a different commit, but it's a good idea to develop both together,
since the tests often uncover problems.

> POSIX
> doesn't require PDP_ENDIAN and I would rather not think about it. :)

But glibc defines PDP_ENDIAN in , and Gnulib attempts to be
compliant to POSIX _and_ source-compatible with glibc.

Also there is some code that uses PDP_ENDIAN [1].

Why do you mention the functions in this order?
+/* Big endian to host.  */
+/* Host to big endian.  */
+/* Host to little endian.  */
+/* Little endian to host.  */
Doing things backwards in the second half than in the first half is
quite unusual; it's a particular property of the Morse sequence [2].
(Note that alphabetical order of items is only to be used as a fallback,
when items cannot be ordered by some form of logic or heuristic.)

In the autoconf test, compilation with -Wall would produce warnings about
the 12 unused variables. If a user sets CC="gcc -Wall -Werror", this will
make the compilation fail. Although this setting of CC is not supported by
Gnulib, it does not hurt to make things work in this case. Like done in
m4/intmax_t.m4: Add a 'return' statement that uses all the variables.
Such as
  return !(value16_1 + value32_1 + value64_1 + value16_2 + value32_2 + value64_2
   + value16_3 + value32_3 + value64_3 + value16_4 + value32_4 + 
value64_4);

Other than that, it looks fine.

Bruno

[1] 
https://codesearch.debian.net/search?q=%5CbPDP_ENDIAN%5Cb+-path%3Aendian.h=0
[2] https://en.wikipedia.org/wiki/Thue%E2%80%93Morse_sequence

endian: New module.

[Collin Funk]

I haven't pushed this since it is my first time attempting the gnulib
header wizardry and doing anything of value in Autoconf.

The configure check should check for unique values of LITTLE_ENDIAN
and BIG_ENDIAN, and that BYTE_ORDER is defined to one of them. POSIX
doesn't require PDP_ENDIAN and I would rather not think about it. :)

Then it checks for uint16_t, uint32_t, and uint64_t being defined and
the existence of the be16toh macros/functions and friends.

Let me know if I missed anything. Other than the docs and test module
which I can do later today (assuming this patch is decent enough).

CollinFrom fe97691b765e57a9c5ea35dca5cb9502d284fb2b Mon Sep 17 00:00:00 2001
From: Collin Funk 
Date: Mon, 13 May 2024 04:43:18 -0700
Subject: [PATCH] endian: New module.

* modules/endian: New file.
* lib/endian.in.h: New file, based on POSIX draft and glibc.
* m4/endian_h.m4: New file, checks for required types and macro/function
definitions.
---
 ChangeLog   |  8 +
 lib/endian.in.h | 89 +
 m4/endian_h.m4  | 68 +
 modules/endian  | 40 ++
 4 files changed, 205 insertions(+)
 create mode 100644 lib/endian.in.h
 create mode 100644 m4/endian_h.m4
 create mode 100644 modules/endian

diff --git a/ChangeLog b/ChangeLog
index b1cc7ffcf0..91acd31590 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,11 @@
+2024-05-13  Collin Funk  
+
+	endian: New module.
+	* modules/endian: New file.
+	* lib/endian.in.h: New file, based on POSIX draft and glibc.
+	* m4/endian_h.m4: New file, checks for required types and macro/function
+	definitions.
+
 2024-05-13  Paul Eggert  
 
 	stdbit: port to theoretical platforms
diff --git a/lib/endian.in.h b/lib/endian.in.h
new file mode 100644
index 00..6b287760a5
--- /dev/null
+++ b/lib/endian.in.h
@@ -0,0 +1,89 @@
+/* endian.h - Byte order macros
+
+   Copyright 2024 Free Software Foundation, Inc.
+
+   This file 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.
+
+   This file 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 this program.  If not, see <https://www.gnu.org/licenses/>.  */
+
+/* Written by Collin Funk.  */
+
+#ifndef _GL_ENDIAN_H
+#define _GL_ENDIAN_H 1
+
+/* This file uses WORDS_BIGENDIAN.  */
+#if !_GL_CONFIG_H_INCLUDED
+ #error "Please include config.h first."
+#endif
+
+/* Byteswap macros.  */
+#include 
+
+/* Define uint16_t, uint32_t, and uint64_t.  */
+#include 
+
+#define LITTLE_ENDIAN 1234
+#define BIG_ENDIAN 4321
+
+#ifdef WORDS_BIGENDIAN
+# define BYTE_ORDER BIG_ENDIAN
+#else
+# define BYTE_ORDER LITTLE_ENDIAN
+#endif
+
+#if BYTE_ORDER == LITTLE_ENDIAN
+
+/* Big endian to host.  */
+# define be16toh(x) bswap_16 (x)
+# define be32toh(x) bswap_32 (x)
+# define be64toh(x) bswap_64 (x)
+
+/* Host to big endian.  */
+# define htobe16(x) bswap_16 (x)
+# define htobe32(x) bswap_32 (x)
+# define htobe64(x) bswap_64 (x)
+
+/* Host to little endian.  */
+# define htole16(x) ((uint16_t) (x))
+# define htole32(x) ((uint32_t) (x))
+# define htole64(x) ((uint64_t) (x))
+
+/* Little endian to host.  */
+# define le16toh(x) ((uint16_t) (x))
+# define le32toh(x) ((uint32_t) (x))
+# define le64toh(x) ((uint64_t) (x))
+
+#else /* BYTE_ORDER == BIG_ENDIAN */
+
+/* Big endian to host.  */
+# define be16toh(x) ((uint16_t) (x))
+# define be32toh(x) ((uint32_t) (x))
+# define be64toh(x) ((uint64_t) (x))
+
+/* Host to big endian.  */
+# define htobe16(x) ((uint16_t) (x))
+# define htobe32(x) ((uint32_t) (x))
+# define htobe64(x) ((uint64_t) (x))
+
+/* Host to little endian.  */
+# define htole16(x) bswap_16 (x)
+# define htole32(x) bswap_32 (x)
+# define htole64(x) bswap_64 (x)
+
+/* Little endian to host.  */
+# define le16toh(x) bswap_16 (x)
+# define le32toh(x) bswap_32 (x)
+# define le64toh(x) bswap_64 (x)
+
+#endif
+
+#endif /* _GL_ENDIAN_H */
diff --git a/m4/endian_h.m4 b/m4/endian_h.m4
new file mode 100644
index 00..2c4a781b73
--- /dev/null
+++ b/m4/endian_h.m4
@@ -0,0 +1,68 @@
+# endian_h.m4
+# serial 1
+dnl Copyright 2024 Free Software Foundation, Inc.
+dnl This file is free software; the Free Software Foundation
+dnl gives unlimited permission to copy and/or distribute it,
+dnl with or without modifications, as long as this notice is preserved.
+
+dnl A placeholder for , for platforms that have issues.
+
+AC_DEFUN_ONCE([gl_ENDIAN_H],
+[
+  AC_REQUIRE([gl_BIGENDIAN])
+
+  dnl Check for the header.
+  AC_CHECK_HEADERS_ONCE([endian.h])
+
+  dnl Check macros and