[PATCH 1/3] msg: Remove redundant #include <sys/msg.h> header

[PATCH 1/3] msg: Remove redundant #include <sys/msg.h> header

[Lukasz Majewski]

The #include <sys/msg.h> is redundant as we do not use message specific
types for issuing syscalls to handle msg and shm. Only msgctl requires
this header.

Build tests:
./src/scripts/build-many-glibcs.py glibcs
---
 sysdeps/unix/sysv/linux/msgget.c | 1 -
 sysdeps/unix/sysv/linux/msgrcv.c | 1 -
 sysdeps/unix/sysv/linux/msgsnd.c | 1 -
 sysdeps/unix/sysv/linux/shmat.c  | 1 -
 sysdeps/unix/sysv/linux/shmdt.c  | 1 -
 sysdeps/unix/sysv/linux/shmget.c | 1 -
 6 files changed, 6 deletions(-)

diff --git a/sysdeps/unix/sysv/linux/msgget.c b/sysdeps/unix/sysv/linux/msgget.c
index 7968b738a4..f5ed417a1f 100644
--- a/sysdeps/unix/sysv/linux/msgget.c
+++ b/sysdeps/unix/sysv/linux/msgget.c
@@ -16,7 +16,6 @@
    License along with the GNU C Library; if not, see
    <https://www.gnu.org/licenses/>.  */
 
-#include <sys/msg.h>
 #include <ipc_priv.h>
 #include <sysdep.h>
 #include <errno.h>
diff --git a/sysdeps/unix/sysv/linux/msgrcv.c b/sysdeps/unix/sysv/linux/msgrcv.c
index 95edc7a787..f3567033e2 100644
--- a/sysdeps/unix/sysv/linux/msgrcv.c
+++ b/sysdeps/unix/sysv/linux/msgrcv.c
@@ -16,7 +16,6 @@
    License along with the GNU C Library; if not, see
    <https://www.gnu.org/licenses/>.  */
 
-#include <sys/msg.h>
 #include <ipc_priv.h>
 #include <sysdep-cancel.h>
 
diff --git a/sysdeps/unix/sysv/linux/msgsnd.c b/sysdeps/unix/sysv/linux/msgsnd.c
index 554516f2ca..6369782706 100644
--- a/sysdeps/unix/sysv/linux/msgsnd.c
+++ b/sysdeps/unix/sysv/linux/msgsnd.c
@@ -16,7 +16,6 @@
    License along with the GNU C Library; if not, see
    <https://www.gnu.org/licenses/>.  */
 
-#include <sys/msg.h>
 #include <ipc_priv.h>
 #include <sysdep-cancel.h>
 
diff --git a/sysdeps/unix/sysv/linux/shmat.c b/sysdeps/unix/sysv/linux/shmat.c
index 3bc791c05f..89df350d84 100644
--- a/sysdeps/unix/sysv/linux/shmat.c
+++ b/sysdeps/unix/sysv/linux/shmat.c
@@ -16,7 +16,6 @@
    License along with the GNU C Library; if not, see
    <https://www.gnu.org/licenses/>.  */
 
-#include <sys/msg.h>
 #include <ipc_priv.h>
 #include <sysdep.h>
 #include <errno.h>
diff --git a/sysdeps/unix/sysv/linux/shmdt.c b/sysdeps/unix/sysv/linux/shmdt.c
index c506064a48..48e223e037 100644
--- a/sysdeps/unix/sysv/linux/shmdt.c
+++ b/sysdeps/unix/sysv/linux/shmdt.c
@@ -16,7 +16,6 @@
    License along with the GNU C Library; if not, see
    <https://www.gnu.org/licenses/>.  */
 
-#include <sys/msg.h>
 #include <ipc_priv.h>
 #include <sysdep.h>
 #include <errno.h>
diff --git a/sysdeps/unix/sysv/linux/shmget.c b/sysdeps/unix/sysv/linux/shmget.c
index 6f7ce8c057..71de81c365 100644
--- a/sysdeps/unix/sysv/linux/shmget.c
+++ b/sysdeps/unix/sysv/linux/shmget.c
@@ -16,7 +16,6 @@
    License along with the GNU C Library; if not, see
    <https://www.gnu.org/licenses/>.  */
 
-#include <sys/msg.h>
 #include <ipc_priv.h>
 #include <sysdep.h>
 #include <errno.h>
-- 
2.20.1

[PATCH 2/3] msqid: Provide internal copy of struct __msqid64_ds

[Lukasz Majewski]

After the commit SHA1: 3283f711132eaadc4f04bd8c1d84c910c29ba
"sysv: linux: Add 64-bit time_t variant for msgctl"

The glibc internal, 64 bit supporting version of struct __msqid64_ds has
been exported.

This approach has issue when one would like to add Y2038 support to glibc.
The problem is that both struct msqid_ds and __msqid64_ds rely on exported
implementation of the msg.h header (./sysvipc/sys/msg.h) which
unconditionally includes ./sysdeps/unix/sysv/linux/bits/msq.h which in
turn includes ./sysdeps/unix/sysv/linux/bits/types/struct_msqid_ds.h

As a result the externally exported struct msqid_ds.h is used internally
by glibc. Problem starts when one wants to extend this struct with 64 bit
time support:

| In file included from ../sysdeps/unix/sysv/linux/bits/msq.h:28,
|                  from ../sysvipc/sys/msg.h:30,
|                  from ../sysdeps/unix/sysv/linux/include/sys/msg.h:2,
|                  from ../include/sys/msg.h:1,
|                  from ../sysdeps/unix/sysv/linux/msgctl.c:19:
| ../sysdeps/unix/sysv/linux/bits/types/struct_msqid_ds.h:31:6: error:
            "__USE_TIME_BITS64" is not defined, evaluates to 0 [-Werror=undef]
|  # if __USE_TIME_BITS64
|       ^~~~~~~~~~~~~~~~~


To fix this issue - the internal,for glibc, copy of struct __msqid64_ds
has been re-introduced. This will allow clear separation between exported
and internal glibc types.

Moreover, the __TIMESIZE based alias shall be internal to glibc.

Build tests:
./src/scripts/build-many-glibcs.py glibcs
---
 include/bits/types/struct_msqid64_ds.h        | 36 +++++++++++++++++++
 .../sysv/linux/bits/types/struct_msqid64_ds.h |  4 ---
 2 files changed, 36 insertions(+), 4 deletions(-)
 create mode 100644 include/bits/types/struct_msqid64_ds.h

diff --git a/include/bits/types/struct_msqid64_ds.h b/include/bits/types/struct_msqid64_ds.h
new file mode 100644
index 0000000000..8c39a2a872
--- /dev/null
+++ b/include/bits/types/struct_msqid64_ds.h
@@ -0,0 +1,36 @@
+/* Internal for glibc implementation of the SysV message struct msqid64_ds.
+   Copyright (C) 2020 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/>.  */
+
+#include <time.h>
+
+#if __TIMESIZE == 64
+# define __msqid64_ds msqid_ds
+#else
+struct __msqid64_ds
+{
+  struct ipc_perm msg_perm;	/* structure describing operation permission */
+  __time64_t msg_stime;		/* time of last msgsnd command */
+  __time64_t msg_rtime;		/* time of last msgsnd command */
+  __time64_t msg_ctime;		/* time of last change */
+  __syscall_ulong_t __msg_cbytes; /* current number of bytes on queue */
+  msgqnum_t msg_qnum;		/* number of messages currently on queue */
+  msglen_t msg_qbytes;		/* max number of bytes allowed on queue */
+  __pid_t msg_lspid;		/* pid of last msgsnd() */
+  __pid_t msg_lrpid;		/* pid of last msgrcv() */
+};
+#endif
diff --git a/sysdeps/unix/sysv/linux/bits/types/struct_msqid64_ds.h b/sysdeps/unix/sysv/linux/bits/types/struct_msqid64_ds.h
index 3536c8ea62..07cc1036ab 100644
--- a/sysdeps/unix/sysv/linux/bits/types/struct_msqid64_ds.h
+++ b/sysdeps/unix/sysv/linux/bits/types/struct_msqid64_ds.h
@@ -20,9 +20,6 @@
 # error "Never use <bits/msq.h> directly; include <sys/msg.h> instead."
 #endif
 
-#if __TIMESIZE == 64
-# define __msqid64_ds msqid_ds
-#else
 struct __msqid64_ds
 {
   struct ipc_perm msg_perm;	/* structure describing operation permission */
@@ -35,4 +32,3 @@ struct __msqid64_ds
   __pid_t msg_lspid;		/* pid of last msgsnd() */
   __pid_t msg_lrpid;		/* pid of last msgrcv() */
 };
-#endif
-- 
2.20.1

Re: [PATCH 2/3] msqid: Provide internal copy of struct __msqid64_ds

[Lukasz Majewski]

[-- Attachment #1: Type: text/plain, Size: 4852 bytes --]

Dear Community,

> After the commit SHA1: 3283f711132eaadc4f04bd8c1d84c910c29ba
> "sysv: linux: Add 64-bit time_t variant for msgctl"
> 
> The glibc internal, 64 bit supporting version of struct __msqid64_ds
> has been exported.
> 
> This approach has issue when one would like to add Y2038 support to
> glibc. The problem is that both struct msqid_ds and __msqid64_ds rely
> on exported implementation of the msg.h header (./sysvipc/sys/msg.h)
> which unconditionally includes ./sysdeps/unix/sysv/linux/bits/msq.h
> which in turn includes
> ./sysdeps/unix/sysv/linux/bits/types/struct_msqid_ds.h
> 
> As a result the externally exported struct msqid_ds.h is used
> internally by glibc. Problem starts when one wants to extend this
> struct with 64 bit time support:
> 
> | In file included from ../sysdeps/unix/sysv/linux/bits/msq.h:28,
> |                  from ../sysvipc/sys/msg.h:30,
> |                  from
> ../sysdeps/unix/sysv/linux/include/sys/msg.h:2, |
> from ../include/sys/msg.h:1, |                  from
> ../sysdeps/unix/sysv/linux/msgctl.c:19: |
> ../sysdeps/unix/sysv/linux/bits/types/struct_msqid_ds.h:31:6: error:
> "__USE_TIME_BITS64" is not defined, evaluates to 0 [-Werror=undef] |
> # if __USE_TIME_BITS64 |       ^~~~~~~~~~~~~~~~~
> 
> 
> To fix this issue - the internal,for glibc, copy of struct
> __msqid64_ds has been re-introduced. This will allow clear separation
> between exported and internal glibc types.
> 
> Moreover, the __TIMESIZE based alias shall be internal to glibc.
> 
> Build tests:
> ./src/scripts/build-many-glibcs.py glibcs

Gentle ping on this patch series.

> ---
>  include/bits/types/struct_msqid64_ds.h        | 36
> +++++++++++++++++++ .../sysv/linux/bits/types/struct_msqid64_ds.h |
> 4 --- 2 files changed, 36 insertions(+), 4 deletions(-)
>  create mode 100644 include/bits/types/struct_msqid64_ds.h
> 
> diff --git a/include/bits/types/struct_msqid64_ds.h
> b/include/bits/types/struct_msqid64_ds.h new file mode 100644
> index 0000000000..8c39a2a872
> --- /dev/null
> +++ b/include/bits/types/struct_msqid64_ds.h
> @@ -0,0 +1,36 @@
> +/* Internal for glibc implementation of the SysV message struct
> msqid64_ds.
> +   Copyright (C) 2020 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/>.  */
> +
> +#include <time.h>
> +
> +#if __TIMESIZE == 64
> +# define __msqid64_ds msqid_ds
> +#else
> +struct __msqid64_ds
> +{
> +  struct ipc_perm msg_perm;	/* structure describing operation
> permission */
> +  __time64_t msg_stime;		/* time of last msgsnd
> command */
> +  __time64_t msg_rtime;		/* time of last msgsnd
> command */
> +  __time64_t msg_ctime;		/* time of last change */
> +  __syscall_ulong_t __msg_cbytes; /* current number of bytes on
> queue */
> +  msgqnum_t msg_qnum;		/* number of messages currently
> on queue */
> +  msglen_t msg_qbytes;		/* max number of bytes allowed
> on queue */
> +  __pid_t msg_lspid;		/* pid of last msgsnd() */
> +  __pid_t msg_lrpid;		/* pid of last msgrcv() */
> +};
> +#endif
> diff --git a/sysdeps/unix/sysv/linux/bits/types/struct_msqid64_ds.h
> b/sysdeps/unix/sysv/linux/bits/types/struct_msqid64_ds.h index
> 3536c8ea62..07cc1036ab 100644 ---
> a/sysdeps/unix/sysv/linux/bits/types/struct_msqid64_ds.h +++
> b/sysdeps/unix/sysv/linux/bits/types/struct_msqid64_ds.h @@ -20,9
> +20,6 @@ # error "Never use <bits/msq.h> directly; include
> <sys/msg.h> instead." #endif
>  
> -#if __TIMESIZE == 64
> -# define __msqid64_ds msqid_ds
> -#else
>  struct __msqid64_ds
>  {
>    struct ipc_perm msg_perm;	/* structure describing operation
> permission */ @@ -35,4 +32,3 @@ struct __msqid64_ds
>    __pid_t msg_lspid;		/* pid of last msgsnd() */
>    __pid_t msg_lrpid;		/* pid of last msgrcv() */
>  };
> -#endif




Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 2/3] msqid: Provide internal copy of struct __msqid64_ds

[Adhemerval Zanella]

On 27/10/2020 19:10, Lukasz Majewski wrote:
> After the commit SHA1: 3283f711132eaadc4f04bd8c1d84c910c29ba
> "sysv: linux: Add 64-bit time_t variant for msgctl"
> 
> The glibc internal, 64 bit supporting version of struct __msqid64_ds has
> been exported.
> 
> This approach has issue when one would like to add Y2038 support to glibc.
> The problem is that both struct msqid_ds and __msqid64_ds rely on exported
> implementation of the msg.h header (./sysvipc/sys/msg.h) which
> unconditionally includes ./sysdeps/unix/sysv/linux/bits/msq.h which in
> turn includes ./sysdeps/unix/sysv/linux/bits/types/struct_msqid_ds.h
> 
> As a result the externally exported struct msqid_ds.h is used internally
> by glibc. Problem starts when one wants to extend this struct with 64 bit
> time support:
> 
> | In file included from ../sysdeps/unix/sysv/linux/bits/msq.h:28,
> |                  from ../sysvipc/sys/msg.h:30,
> |                  from ../sysdeps/unix/sysv/linux/include/sys/msg.h:2,
> |                  from ../include/sys/msg.h:1,
> |                  from ../sysdeps/unix/sysv/linux/msgctl.c:19:
> | ../sysdeps/unix/sysv/linux/bits/types/struct_msqid_ds.h:31:6: error:
>             "__USE_TIME_BITS64" is not defined, evaluates to 0 [-Werror=undef]
> |  # if __USE_TIME_BITS64
> |       ^~~~~~~~~~~~~~~~~
> 
> 
> To fix this issue - the internal,for glibc, copy of struct __msqid64_ds
> has been re-introduced. This will allow clear separation between exported
> and internal glibc types.
> 
> Moreover, the __TIMESIZE based alias shall be internal to glibc.

I don think the definition duplication is the correct approach, it is not
required for other ABIs which vary depending of the compiler flags (such
as LFS).  Also, __TIMESIZE is already defined on an installed header 
(timesize.h) and it defines a macro which within the glibc namespace.

The redirection is different than LFS since we don't have the symbol64 
for all ABIs, so we will need a different way to redirect the implementation.  

What I think it should be done it to move the __TIMESIZE definition to
exported header that actually includes and use the redirect macros we
have for LFS support.  Something as:

  1. On sysdeps/unix/sysv/linux/bits/msq.h

    #if __TIMESIZE == 64 || !defined __USE_TIME_BITS64
    # include <bits/types/struct_msqid_ds.h>
    #else
    # include <bits/types/struct_msqid_ds64.h>
    #endif

    This make currently 64-bit time ABIs or 32-bit ABIs using 32-bit flags
    to include the architecture defined struct_msqid_ds.h

    I used this approach instead of moving the logic to bits/msq.h to avoid
    the need to add this boilerplate and on each architecture define
    bits/msq.h.

  2. On the installed sysvipc/sys/msg.h header do a similar thing:

    #if __TIMESIZE == 32 && defined __USE_TIME_BITS64
    # define msqid_ds __msqid64_ds
    #endif

    #if __TIMESIZE == 64 || !defined __USE_TIME_BITS64
    extern int msgctl (int __msqid, int __cmd, struct msqid_ds *__buf) __THROW;
    #else
    # ifdef __REDIRECT_NTH
    extern int __REDIRECT_NTH (msgctl (int __msqid, 
				       int __cmd,
				       struct msqid64_ds *__buf),
				       __msgctl64) __THROW;
    # else
    #  define msgctl __msgctl64
    # endif
    #endif
    
    Different than LFS, I don't think we need to support the symbol64
    usage (enabled by _LARGEFILE64_SOURCE); just the build to use the
    64-bit time_t ABI.

    Unfortunately I don't know a easy way to redirect the 'struct msqid'
    instantiation to the 64-bit time_t type without a '#define'.

  3. Install the struct_msqid_ds64.h header.


You might need to adjust something on the build itself, but I think this
is the general idea of exporting the 64-bit time interfaces.  What I am
not sure if how we gonna export the 64-bit ABI: whether to use current
name with double underscore of use a similar name convention used for
LFS (without the underscore).

> 
> Build tests:
> ./src/scripts/build-many-glibcs.py glibcs
> ---
>  include/bits/types/struct_msqid64_ds.h        | 36 +++++++++++++++++++
>  .../sysv/linux/bits/types/struct_msqid64_ds.h |  4 ---
>  2 files changed, 36 insertions(+), 4 deletions(-)
>  create mode 100644 include/bits/types/struct_msqid64_ds.h
> 
> diff --git a/include/bits/types/struct_msqid64_ds.h b/include/bits/types/struct_msqid64_ds.h
> new file mode 100644
> index 0000000000..8c39a2a872
> --- /dev/null
> +++ b/include/bits/types/struct_msqid64_ds.h
> @@ -0,0 +1,36 @@
> +/* Internal for glibc implementation of the SysV message struct msqid64_ds.
> +   Copyright (C) 2020 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/>.  */
> +
> +#include <time.h>
> +
> +#if __TIMESIZE == 64
> +# define __msqid64_ds msqid_ds
> +#else
> +struct __msqid64_ds
> +{
> +  struct ipc_perm msg_perm;	/* structure describing operation permission */
> +  __time64_t msg_stime;		/* time of last msgsnd command */
> +  __time64_t msg_rtime;		/* time of last msgsnd command */
> +  __time64_t msg_ctime;		/* time of last change */
> +  __syscall_ulong_t __msg_cbytes; /* current number of bytes on queue */
> +  msgqnum_t msg_qnum;		/* number of messages currently on queue */
> +  msglen_t msg_qbytes;		/* max number of bytes allowed on queue */
> +  __pid_t msg_lspid;		/* pid of last msgsnd() */
> +  __pid_t msg_lrpid;		/* pid of last msgrcv() */
> +};
> +#endif
> diff --git a/sysdeps/unix/sysv/linux/bits/types/struct_msqid64_ds.h b/sysdeps/unix/sysv/linux/bits/types/struct_msqid64_ds.h
> index 3536c8ea62..07cc1036ab 100644
> --- a/sysdeps/unix/sysv/linux/bits/types/struct_msqid64_ds.h
> +++ b/sysdeps/unix/sysv/linux/bits/types/struct_msqid64_ds.h
> @@ -20,9 +20,6 @@
>  # error "Never use <bits/msq.h> directly; include <sys/msg.h> instead."
>  #endif
>  
> -#if __TIMESIZE == 64
> -# define __msqid64_ds msqid_ds
> -#else
>  struct __msqid64_ds
>  {
>    struct ipc_perm msg_perm;	/* structure describing operation permission */
> @@ -35,4 +32,3 @@ struct __msqid64_ds
>    __pid_t msg_lspid;		/* pid of last msgsnd() */
>    __pid_t msg_lrpid;		/* pid of last msgrcv() */
>  };
> -#endif
> 

Re: [PATCH 2/3] msqid: Provide internal copy of struct __msqid64_ds

[Joseph Myers]

On Tue, 3 Nov 2020, Adhemerval Zanella via Libc-alpha wrote:

> You might need to adjust something on the build itself, but I think this
> is the general idea of exporting the 64-bit time interfaces.  What I am
> not sure if how we gonna export the 64-bit ABI: whether to use current
> name with double underscore of use a similar name convention used for
> LFS (without the underscore).

The names should (a) have double underscore only, so namespace-clean and 
(b) be ABI only (so the API is to define _TIME_BITS=64, without having an 
API that involves user programs' source code directly calling *64 
functions).

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [PATCH 2/3] msqid: Provide internal copy of struct __msqid64_ds

[Lukasz Majewski]

[-- Attachment #1: Type: text/plain, Size: 10077 bytes --]

Hi Adhemerval,

> On 27/10/2020 19:10, Lukasz Majewski wrote:
> > After the commit SHA1: 3283f711132eaadc4f04bd8c1d84c910c29ba
> > "sysv: linux: Add 64-bit time_t variant for msgctl"
> > 
> > The glibc internal, 64 bit supporting version of struct
> > __msqid64_ds has been exported.
> > 
> > This approach has issue when one would like to add Y2038 support to
> > glibc. The problem is that both struct msqid_ds and __msqid64_ds
> > rely on exported implementation of the msg.h header
> > (./sysvipc/sys/msg.h) which unconditionally includes
> > ./sysdeps/unix/sysv/linux/bits/msq.h which in turn includes
> > ./sysdeps/unix/sysv/linux/bits/types/struct_msqid_ds.h
> > 
> > As a result the externally exported struct msqid_ds.h is used
> > internally by glibc. Problem starts when one wants to extend this
> > struct with 64 bit time support:
> > 
> > | In file included from ../sysdeps/unix/sysv/linux/bits/msq.h:28,
> > |                  from ../sysvipc/sys/msg.h:30,
> > |                  from
> > ../sysdeps/unix/sysv/linux/include/sys/msg.h:2, |
> > from ../include/sys/msg.h:1, |                  from
> > ../sysdeps/unix/sysv/linux/msgctl.c:19: |
> > ../sysdeps/unix/sysv/linux/bits/types/struct_msqid_ds.h:31:6:
> > error: "__USE_TIME_BITS64" is not defined, evaluates to 0
> > [-Werror=undef] |  # if __USE_TIME_BITS64 |       ^~~~~~~~~~~~~~~~~
> > 
> > 
> > To fix this issue - the internal,for glibc, copy of struct
> > __msqid64_ds has been re-introduced. This will allow clear
> > separation between exported and internal glibc types.
> > 
> > Moreover, the __TIMESIZE based alias shall be internal to glibc.  
> 
> I don think the definition duplication is the correct approach,

Up till now my Y2038 supporting code uses duplication of exported and
internal structs [1].

For example in-glibc we do use struct __timespec64, which matches
exported (and properly adjusted when #ifdef __USE_TIME_BITS64) struct
timespec [2].

The same situation is with time_t and struct timeval.

> it is
> not required for other ABIs which vary depending of the compiler
> flags (such as LFS).  Also, __TIMESIZE is already defined on an
> installed header (timesize.h) and it defines a macro which within the
> glibc namespace.

Up till now the aliasing (like in [3]) was done in internal glibc
headers.

> 
> The redirection is different than LFS since we don't have the
> symbol64 for all ABIs, so we will need a different way to redirect
> the implementation.  

Ok.

> 
> What I think it should be done it to move the __TIMESIZE definition to
> exported header that actually includes and use the redirect macros we
> have for LFS support.  Something as:
> 
>   1. On sysdeps/unix/sysv/linux/bits/msq.h
> 
>     #if __TIMESIZE == 64 || !defined __USE_TIME_BITS64
>     # include <bits/types/struct_msqid_ds.h>
>     #else
>     # include <bits/types/struct_msqid_ds64.h>
>     #endif
> 
>     This make currently 64-bit time ABIs or 32-bit ABIs using 32-bit
> flags to include the architecture defined struct_msqid_ds.h

As I've stated before - I've been avoiding using __TIMESIZE in exported
headers. Instead, I was only using __USE_TIME_BITS64 there.

IIRC that stem from the very early discussion with Joseph about the
generic shape of Y2038 support.

I can adjust my WIP patch supporting Y2038 [1], but I would need a
consensus to avoid rewritting the code in the future.

> 
>     I used this approach instead of moving the logic to bits/msq.h to
> avoid the need to add this boilerplate and on each architecture define
>     bits/msq.h.

Isn't the solution from point 1. requiring to have struct msqid64_ds
instead of struct msqid_ds in [***] ?

> 
>   2. On the installed sysvipc/sys/msg.h header do a similar thing:
> 
>     #if __TIMESIZE == 32 && defined __USE_TIME_BITS64
>     # define msqid_ds __msqid64_ds
>     #endif
> 
>     #if __TIMESIZE == 64 || !defined __USE_TIME_BITS64
>     extern int msgctl (int __msqid, int __cmd, struct msqid_ds
> *__buf) __THROW; #else
  ^^^^^^^^^^^^^^ [*]

>     # ifdef __REDIRECT_NTH
  ^^^^^^^^^^^^^^ [**]

>     extern int __REDIRECT_NTH (msgctl (int __msqid, 
> 				       int __cmd,
> 				       struct msqid64_ds *__buf),
				       ^^^^^^^^^^^^^^^^^^^ - [***]

> 				       __msgctl64) __THROW;
>     # else
>     #  define msgctl __msgctl64
>     # endif
>     #endif

Till now I'm only adding redirection [**] to exported files. The part
from [*] is used in internal glibc headers. I've used the [*] pattern in
all already converted (and pulled to -master) syscalls.

>     
>     Different than LFS, I don't think we need to support the symbol64
>     usage (enabled by _LARGEFILE64_SOURCE); just the build to use the
>     64-bit time_t ABI.

As Joseph stated in the other mail - user code shall use "normal"
syscalls and structures - namely e.g. clock_settime() and struct
timespec.

The aliasing shall be done in exported glibc headers when one compiles
the program with -D_TIME_BITS=64 -D_FILE_OFFSET_BITS=64

This is how it is done in [1].

> 
>     Unfortunately I don't know a easy way to redirect the 'struct
> msqid' instantiation to the 64-bit time_t type without a '#define'.
> 
>   3. Install the struct_msqid_ds64.h header.
> 

IIRC it is already exported.

> 
> You might need to adjust something on the build itself, but I think
> this is the general idea of exporting the 64-bit time interfaces.

Having [*] in exported headers duplicates the code which is already
pulled.

> What I am not sure if how we gonna export the 64-bit ABI: whether to
> use current name with double underscore of use a similar name
> convention used for LFS (without the underscore).
> 

I do guess that you refer to for example exported version of in-glibc
struct __timespec64 ?

And wonder if it shall have exported name as struct __timespec, struct
timespec64 or struct __timespec64 ?

Up till now my WIP patch, which adds Y2038 support [1], gets away
with just adjusting the exported struct timespec, so there is no need
to export struct __timespec64.


Links:
[1] -
https://github.com/lmajewski/y2038_glibc/commit/400ea2f0c98ec36666b395b8e7fe822a3dd40565

[2] -
https://github.com/lmajewski/y2038_glibc/commit/400ea2f0c98ec36666b395b8e7fe822a3dd40565#diff-8297a1191cc06a8a7af6fc6ac33af4bef2d7d72efd901d1634d910b56631e0c7R13

[3] -
https://github.com/lmajewski/y2038_glibc/commit/75c4044b9a49faaeec245cc3a79a390dde7c804e#diff-6403fd00660a397fd05b433121f58de44f09b18aed3a3843669c14041ad79d7bR320

> > 
> > Build tests:
> > ./src/scripts/build-many-glibcs.py glibcs
> > ---
> >  include/bits/types/struct_msqid64_ds.h        | 36
> > +++++++++++++++++++ .../sysv/linux/bits/types/struct_msqid64_ds.h |
> >  4 --- 2 files changed, 36 insertions(+), 4 deletions(-)
> >  create mode 100644 include/bits/types/struct_msqid64_ds.h
> > 
> > diff --git a/include/bits/types/struct_msqid64_ds.h
> > b/include/bits/types/struct_msqid64_ds.h new file mode 100644
> > index 0000000000..8c39a2a872
> > --- /dev/null
> > +++ b/include/bits/types/struct_msqid64_ds.h
> > @@ -0,0 +1,36 @@
> > +/* Internal for glibc implementation of the SysV message struct
> > msqid64_ds.
> > +   Copyright (C) 2020 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/>.  */
> > +
> > +#include <time.h>
> > +
> > +#if __TIMESIZE == 64
> > +# define __msqid64_ds msqid_ds
> > +#else
> > +struct __msqid64_ds
> > +{
> > +  struct ipc_perm msg_perm;	/* structure describing
> > operation permission */
> > +  __time64_t msg_stime;		/* time of last msgsnd
> > command */
> > +  __time64_t msg_rtime;		/* time of last msgsnd
> > command */
> > +  __time64_t msg_ctime;		/* time of last change */
> > +  __syscall_ulong_t __msg_cbytes; /* current number of bytes on
> > queue */
> > +  msgqnum_t msg_qnum;		/* number of messages
> > currently on queue */
> > +  msglen_t msg_qbytes;		/* max number of bytes
> > allowed on queue */
> > +  __pid_t msg_lspid;		/* pid of last msgsnd() */
> > +  __pid_t msg_lrpid;		/* pid of last msgrcv() */
> > +};
> > +#endif
> > diff --git a/sysdeps/unix/sysv/linux/bits/types/struct_msqid64_ds.h
> > b/sysdeps/unix/sysv/linux/bits/types/struct_msqid64_ds.h index
> > 3536c8ea62..07cc1036ab 100644 ---
> > a/sysdeps/unix/sysv/linux/bits/types/struct_msqid64_ds.h +++
> > b/sysdeps/unix/sysv/linux/bits/types/struct_msqid64_ds.h @@ -20,9
> > +20,6 @@ # error "Never use <bits/msq.h> directly; include
> > <sys/msg.h> instead." #endif
> >  
> > -#if __TIMESIZE == 64
> > -# define __msqid64_ds msqid_ds
> > -#else
> >  struct __msqid64_ds
> >  {
> >    struct ipc_perm msg_perm;	/* structure describing
> > operation permission */ @@ -35,4 +32,3 @@ struct __msqid64_ds
> >    __pid_t msg_lspid;		/* pid of last msgsnd() */
> >    __pid_t msg_lrpid;		/* pid of last msgrcv() */
> >  };
> > -#endif
> >   




Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 2/3] msqid: Provide internal copy of struct __msqid64_ds

[Adhemerval Zanella]

On 04/11/2020 10:30, Lukasz Majewski wrote:
> Hi Adhemerval,
> 
>> On 27/10/2020 19:10, Lukasz Majewski wrote:
>>> After the commit SHA1: 3283f711132eaadc4f04bd8c1d84c910c29ba
>>> "sysv: linux: Add 64-bit time_t variant for msgctl"
>>>
>>> The glibc internal, 64 bit supporting version of struct
>>> __msqid64_ds has been exported.
>>>
>>> This approach has issue when one would like to add Y2038 support to
>>> glibc. The problem is that both struct msqid_ds and __msqid64_ds
>>> rely on exported implementation of the msg.h header
>>> (./sysvipc/sys/msg.h) which unconditionally includes
>>> ./sysdeps/unix/sysv/linux/bits/msq.h which in turn includes
>>> ./sysdeps/unix/sysv/linux/bits/types/struct_msqid_ds.h
>>>
>>> As a result the externally exported struct msqid_ds.h is used
>>> internally by glibc. Problem starts when one wants to extend this
>>> struct with 64 bit time support:
>>>
>>> | In file included from ../sysdeps/unix/sysv/linux/bits/msq.h:28,
>>> |                  from ../sysvipc/sys/msg.h:30,
>>> |                  from
>>> ../sysdeps/unix/sysv/linux/include/sys/msg.h:2, |
>>> from ../include/sys/msg.h:1, |                  from
>>> ../sysdeps/unix/sysv/linux/msgctl.c:19: |
>>> ../sysdeps/unix/sysv/linux/bits/types/struct_msqid_ds.h:31:6:
>>> error: "__USE_TIME_BITS64" is not defined, evaluates to 0
>>> [-Werror=undef] |  # if __USE_TIME_BITS64 |       ^~~~~~~~~~~~~~~~~
>>>
>>>
>>> To fix this issue - the internal,for glibc, copy of struct
>>> __msqid64_ds has been re-introduced. This will allow clear
>>> separation between exported and internal glibc types.
>>>
>>> Moreover, the __TIMESIZE based alias shall be internal to glibc.  
>>
>> I don think the definition duplication is the correct approach,
> 
> Up till now my Y2038 supporting code uses duplication of exported and
> internal structs [1].
> 
> For example in-glibc we do use struct __timespec64, which matches
> exported (and properly adjusted when #ifdef __USE_TIME_BITS64) struct
> timespec [2].
> 
> The same situation is with time_t and struct timeval.

The drawback of using this strategy is exactly what you pointed out:
the exported ABI is different than the one used internally which
require duplicate definition (which adds extra effort to keep both
in sync), one must take care to avoid use __USE_TIME_BITS64 internally
which might either break the build or generate wrong code, and it
requires to change over multiple files for full y2038 support
(specially in the case for arch-specific structs).

My understanding of y2038 of moving to platform neutral definition
on specific file is just to simplify it: no need to keep multiple
architecture specific definitions and the same definition is used
internally and externally.

> 
>> it is
>> not required for other ABIs which vary depending of the compiler
>> flags (such as LFS).  Also, __TIMESIZE is already defined on an
>> installed header (timesize.h) and it defines a macro which within the
>> glibc namespace.
> 
> Up till now the aliasing (like in [3]) was done in internal glibc
> headers.
> 
>>
>> The redirection is different than LFS since we don't have the
>> symbol64 for all ABIs, so we will need a different way to redirect
>> the implementation.  
> 
> Ok.
> 
>>
>> What I think it should be done it to move the __TIMESIZE definition to
>> exported header that actually includes and use the redirect macros we
>> have for LFS support.  Something as:
>>
>>   1. On sysdeps/unix/sysv/linux/bits/msq.h
>>
>>     #if __TIMESIZE == 64 || !defined __USE_TIME_BITS64
>>     # include <bits/types/struct_msqid_ds.h>
>>     #else
>>     # include <bits/types/struct_msqid_ds64.h>
>>     #endif
>>
>>     This make currently 64-bit time ABIs or 32-bit ABIs using 32-bit
>> flags to include the architecture defined struct_msqid_ds.h
> 
> As I've stated before - I've been avoiding using __TIMESIZE in exported
> headers. Instead, I was only using __USE_TIME_BITS64 there.
> 
> IIRC that stem from the very early discussion with Joseph about the
> generic shape of Y2038 support.
> 
> I can adjust my WIP patch supporting Y2038 [1], but I would need a
> consensus to avoid rewritting the code in the future.

The __TIMESIZE is used now internally to define whether the ABI has
originally support for 32-bit time_t and it is an installed header.
It is also used on *installed* headers to define Linux specific ABI,
for instance on bits/socket-constants.h introduced on riscv32 support.

So I think it is ok to use to decide whether the ABI has 32-bit support
to decide whether __USE_TIME_BITS64 should redirect the symbol to the
y2038 ones.

> 
>>
>>     I used this approach instead of moving the logic to bits/msq.h to
>> avoid the need to add this boilerplate and on each architecture define
>>     bits/msq.h.
> 
> Isn't the solution from point 1. requiring to have struct msqid64_ds
> instead of struct msqid_ds in [***] ?

Indeed on [***] it should be 'struct __msqid64_ds'.

> 
>>
>>   2. On the installed sysvipc/sys/msg.h header do a similar thing:
>>
>>     #if __TIMESIZE == 32 && defined __USE_TIME_BITS64
>>     # define msqid_ds __msqid64_ds
>>     #endif
>>
>>     #if __TIMESIZE == 64 || !defined __USE_TIME_BITS64
>>     extern int msgctl (int __msqid, int __cmd, struct msqid_ds
>> *__buf) __THROW; #else
>   ^^^^^^^^^^^^^^ [*]
> 
>>     # ifdef __REDIRECT_NTH
>   ^^^^^^^^^^^^^^ [**]
> 
>>     extern int __REDIRECT_NTH (msgctl (int __msqid, 
>> 				       int __cmd,
>> 				       struct msqid64_ds *__buf),
> 				       ^^^^^^^^^^^^^^^^^^^ - [***]
> 
>> 				       __msgctl64) __THROW;
>>     # else
>>     #  define msgctl __msgctl64
>>     # endif
>>     #endif
> 
> Till now I'm only adding redirection [**] to exported files. The part
> from [*] is used in internal glibc headers. I've used the [*] pattern in
> all already converted (and pulled to -master) syscalls.
> 
>>     
>>     Different than LFS, I don't think we need to support the symbol64
>>     usage (enabled by _LARGEFILE64_SOURCE); just the build to use the
>>     64-bit time_t ABI.
> 
> As Joseph stated in the other mail - user code shall use "normal"
> syscalls and structures - namely e.g. clock_settime() and struct
> timespec.
> 
> The aliasing shall be done in exported glibc headers when one compiles
> the program with -D_TIME_BITS=64 -D_FILE_OFFSET_BITS=64
> 
> This is how it is done in [1].

That's exactly the idea of __REDIRECT_NTH. For the code:

  struct msqid_ds buf;
  [...]
  msgctl (..., &buf);

With _TIME_BITS=32 it will emit a call to msgctl, while for _TIME_BITS=64 
it will still emit a call to __msgctl64 (due the asm alias).

> 
>>
>>     Unfortunately I don't know a easy way to redirect the 'struct
>> msqid' instantiation to the 64-bit time_t type without a '#define'.
>>
>>   3. Install the struct_msqid_ds64.h header.
>>
> 
> IIRC it is already exported.

I meant to add it on 'sysdep_headers' rule to install with 'make install'.

> 
>>
>> You might need to adjust something on the build itself, but I think
>> this is the general idea of exporting the 64-bit time interfaces.
> 
> Having [*] in exported headers duplicates the code which is already
> pulled.
> 
>> What I am not sure if how we gonna export the 64-bit ABI: whether to
>> use current name with double underscore of use a similar name
>> convention used for LFS (without the underscore).
>>
> 
> I do guess that you refer to for example exported version of in-glibc
> struct __timespec64 ?
> 
> And wonder if it shall have exported name as struct __timespec, struct
> timespec64 or struct __timespec64 ?
> 
> Up till now my WIP patch, which adds Y2038 support [1], gets away
> with just adjusting the exported struct timespec, so there is no need
> to export struct __timespec64.

I think the exported names for the y2038 struct should be different than
the default 32-time one to force mangling mismatch when mixing C++ object
built with different _TIME_BITS values.  The names should also have the
double underscore as indicated by Joseph (and it avoid the issue we have
for LFS, BZ#14106).

> 
> 
> Links:
> [1] -
> https://github.com/lmajewski/y2038_glibc/commit/400ea2f0c98ec36666b395b8e7fe822a3dd40565
> 
> [2] -
> https://github.com/lmajewski/y2038_glibc/commit/400ea2f0c98ec36666b395b8e7fe822a3dd40565#diff-8297a1191cc06a8a7af6fc6ac33af4bef2d7d72efd901d1634d910b56631e0c7R13
> 
> [3] -
> https://github.com/lmajewski/y2038_glibc/commit/75c4044b9a49faaeec245cc3a79a390dde7c804e#diff-6403fd00660a397fd05b433121f58de44f09b18aed3a3843669c14041ad79d7bR320
> 
>>>
>>> Build tests:
>>> ./src/scripts/build-many-glibcs.py glibcs
>>> ---
>>>  include/bits/types/struct_msqid64_ds.h        | 36
>>> +++++++++++++++++++ .../sysv/linux/bits/types/struct_msqid64_ds.h |
>>>  4 --- 2 files changed, 36 insertions(+), 4 deletions(-)
>>>  create mode 100644 include/bits/types/struct_msqid64_ds.h
>>>
>>> diff --git a/include/bits/types/struct_msqid64_ds.h
>>> b/include/bits/types/struct_msqid64_ds.h new file mode 100644
>>> index 0000000000..8c39a2a872
>>> --- /dev/null
>>> +++ b/include/bits/types/struct_msqid64_ds.h
>>> @@ -0,0 +1,36 @@
>>> +/* Internal for glibc implementation of the SysV message struct
>>> msqid64_ds.
>>> +   Copyright (C) 2020 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/>.  */
>>> +
>>> +#include <time.h>
>>> +
>>> +#if __TIMESIZE == 64
>>> +# define __msqid64_ds msqid_ds
>>> +#else
>>> +struct __msqid64_ds
>>> +{
>>> +  struct ipc_perm msg_perm;	/* structure describing
>>> operation permission */
>>> +  __time64_t msg_stime;		/* time of last msgsnd
>>> command */
>>> +  __time64_t msg_rtime;		/* time of last msgsnd
>>> command */
>>> +  __time64_t msg_ctime;		/* time of last change */
>>> +  __syscall_ulong_t __msg_cbytes; /* current number of bytes on
>>> queue */
>>> +  msgqnum_t msg_qnum;		/* number of messages
>>> currently on queue */
>>> +  msglen_t msg_qbytes;		/* max number of bytes
>>> allowed on queue */
>>> +  __pid_t msg_lspid;		/* pid of last msgsnd() */
>>> +  __pid_t msg_lrpid;		/* pid of last msgrcv() */
>>> +};
>>> +#endif
>>> diff --git a/sysdeps/unix/sysv/linux/bits/types/struct_msqid64_ds.h
>>> b/sysdeps/unix/sysv/linux/bits/types/struct_msqid64_ds.h index
>>> 3536c8ea62..07cc1036ab 100644 ---
>>> a/sysdeps/unix/sysv/linux/bits/types/struct_msqid64_ds.h +++
>>> b/sysdeps/unix/sysv/linux/bits/types/struct_msqid64_ds.h @@ -20,9
>>> +20,6 @@ # error "Never use <bits/msq.h> directly; include
>>> <sys/msg.h> instead." #endif
>>>  
>>> -#if __TIMESIZE == 64
>>> -# define __msqid64_ds msqid_ds
>>> -#else
>>>  struct __msqid64_ds
>>>  {
>>>    struct ipc_perm msg_perm;	/* structure describing
>>> operation permission */ @@ -35,4 +32,3 @@ struct __msqid64_ds
>>>    __pid_t msg_lspid;		/* pid of last msgsnd() */
>>>    __pid_t msg_lrpid;		/* pid of last msgrcv() */
>>>  };
>>> -#endif
>>>   
> 
> 
> 
> 
> Best regards,
> 
> Lukasz Majewski
> 
> --
> 
> DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de
> 

Re: [PATCH 2/3] msqid: Provide internal copy of struct __msqid64_ds

[Lukasz Majewski]

[-- Attachment #1: Type: text/plain, Size: 15314 bytes --]

Hi Adhemerval,

> On 04/11/2020 10:30, Lukasz Majewski wrote:
> > Hi Adhemerval,
> >   
> >> On 27/10/2020 19:10, Lukasz Majewski wrote:  
> >>> After the commit SHA1: 3283f711132eaadc4f04bd8c1d84c910c29ba
> >>> "sysv: linux: Add 64-bit time_t variant for msgctl"
> >>>
> >>> The glibc internal, 64 bit supporting version of struct
> >>> __msqid64_ds has been exported.
> >>>
> >>> This approach has issue when one would like to add Y2038 support
> >>> to glibc. The problem is that both struct msqid_ds and
> >>> __msqid64_ds rely on exported implementation of the msg.h header
> >>> (./sysvipc/sys/msg.h) which unconditionally includes
> >>> ./sysdeps/unix/sysv/linux/bits/msq.h which in turn includes
> >>> ./sysdeps/unix/sysv/linux/bits/types/struct_msqid_ds.h
> >>>
> >>> As a result the externally exported struct msqid_ds.h is used
> >>> internally by glibc. Problem starts when one wants to extend this
> >>> struct with 64 bit time support:
> >>>
> >>> | In file included from ../sysdeps/unix/sysv/linux/bits/msq.h:28,
> >>> |                  from ../sysvipc/sys/msg.h:30,
> >>> |                  from
> >>> ../sysdeps/unix/sysv/linux/include/sys/msg.h:2, |
> >>> from ../include/sys/msg.h:1, |                  from
> >>> ../sysdeps/unix/sysv/linux/msgctl.c:19: |
> >>> ../sysdeps/unix/sysv/linux/bits/types/struct_msqid_ds.h:31:6:
> >>> error: "__USE_TIME_BITS64" is not defined, evaluates to 0
> >>> [-Werror=undef] |  # if __USE_TIME_BITS64 |
> >>> ^~~~~~~~~~~~~~~~~
> >>>
> >>>
> >>> To fix this issue - the internal,for glibc, copy of struct
> >>> __msqid64_ds has been re-introduced. This will allow clear
> >>> separation between exported and internal glibc types.
> >>>
> >>> Moreover, the __TIMESIZE based alias shall be internal to glibc.
> >>>   
> >>
> >> I don think the definition duplication is the correct approach,  
> > 
> > Up till now my Y2038 supporting code uses duplication of exported
> > and internal structs [1].
> > 
> > For example in-glibc we do use struct __timespec64, which matches
> > exported (and properly adjusted when #ifdef __USE_TIME_BITS64)
> > struct timespec [2].
> > 
> > The same situation is with time_t and struct timeval.  
> 
> The drawback of using this strategy is exactly what you pointed out:
> the exported ABI is different than the one used internally which
> require duplicate definition (which adds extra effort to keep both
> in sync), one must take care to avoid use __USE_TIME_BITS64 internally
> which might either break the build or generate wrong code, and it
> requires to change over multiple files for full y2038 support
> (specially in the case for arch-specific structs).

Ok.

> 
> My understanding of y2038 of moving to platform neutral definition
> on specific file is just to simplify it: no need to keep multiple
> architecture specific definitions and the same definition is used
> internally and externally.

So if I understood you correctly - lets consider the struct timespec
(exported definition) and struct __timespec64 (internal one).

1. We shall export struct __timespec64:
./time/bits/types/struct___timespec64.h

2. The above header shall be re-included by glibc -
./include/bits/types/struct_timespec.h shall have just #include <....
struct___timespec64.h>

3. The exported header (from point 1) shall have:

#if __TIMESIZE == 32 && defined __USE_TIME_BITS64
# define timespec __timespec64
#endif

So the 'struct timespec' in user program would then be aliased to
'struct __timespec64' (the same, which was used in glib internally).



> 
> >   
> >> it is
> >> not required for other ABIs which vary depending of the compiler
> >> flags (such as LFS).  Also, __TIMESIZE is already defined on an
> >> installed header (timesize.h) and it defines a macro which within
> >> the glibc namespace.  
> > 
> > Up till now the aliasing (like in [3]) was done in internal glibc
> > headers.
> >   
> >>
> >> The redirection is different than LFS since we don't have the
> >> symbol64 for all ABIs, so we will need a different way to redirect
> >> the implementation.    
> > 
> > Ok.
> >   
> >>
> >> What I think it should be done it to move the __TIMESIZE
> >> definition to exported header that actually includes and use the
> >> redirect macros we have for LFS support.  Something as:
> >>
> >>   1. On sysdeps/unix/sysv/linux/bits/msq.h
> >>
> >>     #if __TIMESIZE == 64 || !defined __USE_TIME_BITS64
> >>     # include <bits/types/struct_msqid_ds.h>
> >>     #else
> >>     # include <bits/types/struct_msqid_ds64.h>
> >>     #endif
> >>
> >>     This make currently 64-bit time ABIs or 32-bit ABIs using
> >> 32-bit flags to include the architecture defined struct_msqid_ds.h
> >>  
> > 
> > As I've stated before - I've been avoiding using __TIMESIZE in
> > exported headers. Instead, I was only using __USE_TIME_BITS64 there.
> > 
> > IIRC that stem from the very early discussion with Joseph about the
> > generic shape of Y2038 support.
> > 
> > I can adjust my WIP patch supporting Y2038 [1], but I would need a
> > consensus to avoid rewritting the code in the future.  
> 
> The __TIMESIZE is used now internally to define whether the ABI has
> originally support for 32-bit time_t and it is an installed header.
> It is also used on *installed* headers to define Linux specific ABI,
> for instance on bits/socket-constants.h introduced on riscv32 support.
> 

Ok. I've overlooked the __TIMESIZE ifdefs in exported
sysdeps/unix/sysv/linux/powerpc/bits/types/struct_* files.

If you think that __TIMESIZE check shall be added along the
__USE_TIME_BITS64 to redirections then I'm fine with it:

In my code I do have:

extern int msgctl (int __msqid, int __cmd, struct msqid_ds *__buf)

#ifdef __USE_TIME_BITS64
# ifdef __REDIRECT_NTH
 extern int __REDIRECT_NTH (msgctl (int __msqid, 
			       int __cmd,
			       struct msqid64_ds *__buf),
			       __msgctl64) __THROW;
# else
#  define msgctl __msgctl64
# endif
#endif

And if you think it is a better approach - we can replace it to:

#if __TIMESIZE == 64 || !defined __USE_TIME_BITS64
 extern int msgctl (int __msqid, int __cmd, struct msqid_ds
*__buf)__THROW;
#else
# ifdef __REDIRECT_NTH
 extern int __REDIRECT_NTH (msgctl (int __msqid, 
		       int __cmd,
		       struct msqid64_ds *__buf),
		       __msgctl64) __THROW;
# else
#  define msgctl __msgctl64
# endif
#endif


> So I think it is ok to use to decide whether the ABI has 32-bit
> support to decide whether __USE_TIME_BITS64 should redirect the
> symbol to the y2038 ones.
> 
> >   
> >>
> >>     I used this approach instead of moving the logic to bits/msq.h
> >> to avoid the need to add this boilerplate and on each architecture
> >> define bits/msq.h.  
> > 
> > Isn't the solution from point 1. requiring to have struct msqid64_ds
> > instead of struct msqid_ds in [***] ?  
> 
> Indeed on [***] it should be 'struct __msqid64_ds'.

Ok.

> 
> >   
> >>
> >>   2. On the installed sysvipc/sys/msg.h header do a similar thing:
> >>
> >>     #if __TIMESIZE == 32 && defined __USE_TIME_BITS64
> >>     # define msqid_ds __msqid64_ds
> >>     #endif
> >>
> >>     #if __TIMESIZE == 64 || !defined __USE_TIME_BITS64
> >>     extern int msgctl (int __msqid, int __cmd, struct msqid_ds
> >> *__buf) __THROW; #else  
> >   ^^^^^^^^^^^^^^ [*]
> >   
> >>     # ifdef __REDIRECT_NTH  
> >   ^^^^^^^^^^^^^^ [**]
> >   
> >>     extern int __REDIRECT_NTH (msgctl (int __msqid, 
> >> 				       int __cmd,
> >> 				       struct msqid64_ds *__buf),  
> > 				       ^^^^^^^^^^^^^^^^^^^ - [***]
> >   
> >> 				       __msgctl64) __THROW;
> >>     # else
> >>     #  define msgctl __msgctl64
> >>     # endif
> >>     #endif  
> > 
> > Till now I'm only adding redirection [**] to exported files. The
> > part from [*] is used in internal glibc headers. I've used the [*]
> > pattern in all already converted (and pulled to -master) syscalls.
> >   
> >>     
> >>     Different than LFS, I don't think we need to support the
> >> symbol64 usage (enabled by _LARGEFILE64_SOURCE); just the build to
> >> use the 64-bit time_t ABI.  
> > 
> > As Joseph stated in the other mail - user code shall use "normal"
> > syscalls and structures - namely e.g. clock_settime() and struct
> > timespec.
> > 
> > The aliasing shall be done in exported glibc headers when one
> > compiles the program with -D_TIME_BITS=64 -D_FILE_OFFSET_BITS=64
> > 
> > This is how it is done in [1].  
> 
> That's exactly the idea of __REDIRECT_NTH. For the code:
> 
>   struct msqid_ds buf;
>   [...]
>   msgctl (..., &buf);
> 
> With _TIME_BITS=32 it will emit a call to msgctl, while for
> _TIME_BITS=64 it will still emit a call to __msgctl64 (due the asm
> alias).

Yes, correct.

> 
> >   
> >>
> >>     Unfortunately I don't know a easy way to redirect the 'struct
> >> msqid' instantiation to the 64-bit time_t type without a '#define'.
> >>
> >>   3. Install the struct_msqid_ds64.h header.
> >>  
> > 
> > IIRC it is already exported.  
> 
> I meant to add it on 'sysdep_headers' rule to install with 'make
> install'.

Ok, this is interesting. I've grep'ed for "sysdep_headers" and some
headers have been found. However, I'm wondering how other ones are
exported as the list seems to be reduced when compared to the total
number of installed headers.

> 
> >   
> >>
> >> You might need to adjust something on the build itself, but I think
> >> this is the general idea of exporting the 64-bit time interfaces.  
> > 
> > Having [*] in exported headers duplicates the code which is already
> > pulled.
> >   
> >> What I am not sure if how we gonna export the 64-bit ABI: whether
> >> to use current name with double underscore of use a similar name
> >> convention used for LFS (without the underscore).
> >>  
> > 
> > I do guess that you refer to for example exported version of
> > in-glibc struct __timespec64 ?
> > 
> > And wonder if it shall have exported name as struct __timespec,
> > struct timespec64 or struct __timespec64 ?
> > 
> > Up till now my WIP patch, which adds Y2038 support [1], gets away
> > with just adjusting the exported struct timespec, so there is no
> > need to export struct __timespec64.  
> 
> I think the exported names for the y2038 struct should be different
> than the default 32-time one to force mangling mismatch when mixing
> C++ object built with different _TIME_BITS values.  The names should
> also have the double underscore as indicated by Joseph (and it avoid
> the issue we have for LFS, BZ#14106).

IMHO the struct __timespec64 shall be a good choice for exported name
(the same would be done for __time64 and __timeval64).

> 
> > 
> > 
> > Links:
> > [1] -
> > https://github.com/lmajewski/y2038_glibc/commit/400ea2f0c98ec36666b395b8e7fe822a3dd40565
> > 
> > [2] -
> > https://github.com/lmajewski/y2038_glibc/commit/400ea2f0c98ec36666b395b8e7fe822a3dd40565#diff-8297a1191cc06a8a7af6fc6ac33af4bef2d7d72efd901d1634d910b56631e0c7R13
> > 
> > [3] -
> > https://github.com/lmajewski/y2038_glibc/commit/75c4044b9a49faaeec245cc3a79a390dde7c804e#diff-6403fd00660a397fd05b433121f58de44f09b18aed3a3843669c14041ad79d7bR320
> >   
> >>>
> >>> Build tests:
> >>> ./src/scripts/build-many-glibcs.py glibcs
> >>> ---
> >>>  include/bits/types/struct_msqid64_ds.h        | 36
> >>> +++++++++++++++++++ .../sysv/linux/bits/types/struct_msqid64_ds.h
> >>> | 4 --- 2 files changed, 36 insertions(+), 4 deletions(-)
> >>>  create mode 100644 include/bits/types/struct_msqid64_ds.h
> >>>
> >>> diff --git a/include/bits/types/struct_msqid64_ds.h
> >>> b/include/bits/types/struct_msqid64_ds.h new file mode 100644
> >>> index 0000000000..8c39a2a872
> >>> --- /dev/null
> >>> +++ b/include/bits/types/struct_msqid64_ds.h
> >>> @@ -0,0 +1,36 @@
> >>> +/* Internal for glibc implementation of the SysV message struct
> >>> msqid64_ds.
> >>> +   Copyright (C) 2020 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/>.  */
> >>> +
> >>> +#include <time.h>
> >>> +
> >>> +#if __TIMESIZE == 64
> >>> +# define __msqid64_ds msqid_ds
> >>> +#else
> >>> +struct __msqid64_ds
> >>> +{
> >>> +  struct ipc_perm msg_perm;	/* structure describing
> >>> operation permission */
> >>> +  __time64_t msg_stime;		/* time of last msgsnd
> >>> command */
> >>> +  __time64_t msg_rtime;		/* time of last msgsnd
> >>> command */
> >>> +  __time64_t msg_ctime;		/* time of last change */
> >>> +  __syscall_ulong_t __msg_cbytes; /* current number of bytes on
> >>> queue */
> >>> +  msgqnum_t msg_qnum;		/* number of messages
> >>> currently on queue */
> >>> +  msglen_t msg_qbytes;		/* max number of bytes
> >>> allowed on queue */
> >>> +  __pid_t msg_lspid;		/* pid of last msgsnd() */
> >>> +  __pid_t msg_lrpid;		/* pid of last msgrcv() */
> >>> +};
> >>> +#endif
> >>> diff --git
> >>> a/sysdeps/unix/sysv/linux/bits/types/struct_msqid64_ds.h
> >>> b/sysdeps/unix/sysv/linux/bits/types/struct_msqid64_ds.h index
> >>> 3536c8ea62..07cc1036ab 100644 ---
> >>> a/sysdeps/unix/sysv/linux/bits/types/struct_msqid64_ds.h +++
> >>> b/sysdeps/unix/sysv/linux/bits/types/struct_msqid64_ds.h @@ -20,9
> >>> +20,6 @@ # error "Never use <bits/msq.h> directly; include
> >>> <sys/msg.h> instead." #endif 
> >>> -#if __TIMESIZE == 64
> >>> -# define __msqid64_ds msqid_ds
> >>> -#else
> >>>  struct __msqid64_ds
> >>>  {
> >>>    struct ipc_perm msg_perm;	/* structure describing
> >>> operation permission */ @@ -35,4 +32,3 @@ struct __msqid64_ds
> >>>    __pid_t msg_lspid;		/* pid of last msgsnd() */
> >>>    __pid_t msg_lrpid;		/* pid of last msgrcv() */
> >>>  };
> >>> -#endif
> >>>     
> > 
> > 
> > 
> > 
> > Best regards,
> > 
> > Lukasz Majewski
> > 
> > --
> > 
> > DENX Software Engineering GmbH,      Managing Director: Wolfgang
> > Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell,
> > Germany Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email:
> > lukma@denx.de 




Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 2/3] msqid: Provide internal copy of struct __msqid64_ds

[Joseph Myers]

On Fri, 6 Nov 2020, Lukasz Majewski wrote:

> 3. The exported header (from point 1) shall have:
> 
> #if __TIMESIZE == 32 && defined __USE_TIME_BITS64
> # define timespec __timespec64
> #endif

You'll need to make sure that's permitted under the namespace reservations 
in POSIX.

For the cases where a struct tag has the same name as a function (e.g. 
stat), #undef of the function name is definitely allowed for user 
programs, so relying on defining the struct tag as a macro is probably 
unsafe in that case.

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [PATCH 2/3] msqid: Provide internal copy of struct __msqid64_ds

[Adhemerval Zanella]

On 06/11/2020 14:37, Joseph Myers wrote:
> On Fri, 6 Nov 2020, Lukasz Majewski wrote:
> 
>> 3. The exported header (from point 1) shall have:
>>
>> #if __TIMESIZE == 32 && defined __USE_TIME_BITS64
>> # define timespec __timespec64
>> #endif
> 
> You'll need to make sure that's permitted under the namespace reservations 
> in POSIX.
> 
> For the cases where a struct tag has the same name as a function (e.g. 
> stat), #undef of the function name is definitely allowed for user 
> programs, so relying on defining the struct tag as a macro is probably 
> unsafe in that case.

In these which alternative would be better?  Export __timespec64 as 
different name or use the strategy to redefine timespec to different
member sizes and layout depending of time ABI?

Re: [PATCH 2/3] msqid: Provide internal copy of struct __msqid64_ds

[Joseph Myers]

On Mon, 9 Nov 2020, Adhemerval Zanella via Libc-alpha wrote:

> On 06/11/2020 14:37, Joseph Myers wrote:
> > On Fri, 6 Nov 2020, Lukasz Majewski wrote:
> > 
> >> 3. The exported header (from point 1) shall have:
> >>
> >> #if __TIMESIZE == 32 && defined __USE_TIME_BITS64
> >> # define timespec __timespec64
> >> #endif
> > 
> > You'll need to make sure that's permitted under the namespace reservations 
> > in POSIX.
> > 
> > For the cases where a struct tag has the same name as a function (e.g. 
> > stat), #undef of the function name is definitely allowed for user 
> > programs, so relying on defining the struct tag as a macro is probably 
> > unsafe in that case.
> 
> In these which alternative would be better?  Export __timespec64 as 
> different name or use the strategy to redefine timespec to different
> member sizes and layout depending of time ABI?

If in a particular case defining the struct tag as a macro is unsafe, we 
can't avoid having different definitions of the contents of the same 
struct tag depending on how _TIME_BITS is defined.

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [PATCH 2/3] msqid: Provide internal copy of struct __msqid64_ds

[Lukasz Majewski]

[-- Attachment #1: Type: text/plain, Size: 2479 bytes --]

Hi Joseph, Adhemerval,

> On Mon, 9 Nov 2020, Adhemerval Zanella via Libc-alpha wrote:
> 
> > On 06/11/2020 14:37, Joseph Myers wrote:  
> > > On Fri, 6 Nov 2020, Lukasz Majewski wrote:
> > >   
> > >> 3. The exported header (from point 1) shall have:
> > >>
> > >> #if __TIMESIZE == 32 && defined __USE_TIME_BITS64
> > >> # define timespec __timespec64
> > >> #endif  
> > > 
> > > You'll need to make sure that's permitted under the namespace
> > > reservations in POSIX.
> > > 
> > > For the cases where a struct tag has the same name as a function
> > > (e.g. stat), #undef of the function name is definitely allowed
> > > for user programs, so relying on defining the struct tag as a
> > > macro is probably unsafe in that case.  
> > 
> > In these which alternative would be better?  Export __timespec64 as 
> > different name or use the strategy to redefine timespec to different
> > member sizes and layout depending of time ABI?  
> 
> If in a particular case defining the struct tag as a macro is unsafe,

Could you explain why having construct as:

#define timespec __timespec64 in ./time/bits/types/struct_timespec.c is
unsafe (after having the file ./time/bits/types/struct___timespec64.c
exported)?

What are the problems with such approach? Would it not make the "posix"
exported headers content polluted and make this test fail?

(I had such case when by mistake I've added some extra defines to some
exported headers).

> we can't avoid having different definitions of the contents of the
> same struct tag depending on how _TIME_BITS is defined.

Have I understood correctly that we shall then stick to the approach
to have exported only e.g. struct timespec and have corresponding
internal for glibc structure - __timespec64, which matches the former's
ABI? Such approach is now used in my WIP patch [1].

The chosen approach is quite important in terms of the work necessary
for Y2038 support. I would opt for having the consensus reached sooner
than latter?

> 

Links:

[1] -
https://github.com/lmajewski/y2038_glibc/commit/cc68c0498f82697e6f87418e148a19ded2d5aec7#diff-8297a1191cc06a8a7af6fc6ac33af4bef2d7d72efd901d1634d910b56631e0c7R11


Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 2/3] msqid: Provide internal copy of struct __msqid64_ds

[Joseph Myers]

On Mon, 9 Nov 2020, Lukasz Majewski wrote:

> > If in a particular case defining the struct tag as a macro is unsafe,
> 
> Could you explain why having construct as:
> 
> #define timespec __timespec64 in ./time/bits/types/struct_timespec.c is
> unsafe (after having the file ./time/bits/types/struct___timespec64.c
> exported)?

I think "#define stat __stat64_time64" is unsafe, since people may do 
"#undef stat" to get a function rather than a macro (and thereby suppress 
the macro expansion for the struct tag as well).

I'm less sure there would be any problems with "#define timespec 
__timespec64".  The POSIX namespace rules are clearer on e.g. what's 
reserved with file scope, than on what's reserved as a macro name that the 
user mustn't undefine.

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [PATCH 2/3] msqid: Provide internal copy of struct __msqid64_ds

[Lukasz Majewski]

[-- Attachment #1: Type: text/plain, Size: 1223 bytes --]

Hi Joseph, Adhemerval,

> On Mon, 9 Nov 2020, Lukasz Majewski wrote:
> 
> > > If in a particular case defining the struct tag as a macro is
> > > unsafe,  
> > 
> > Could you explain why having construct as:
> > 
> > #define timespec __timespec64 in
> > ./time/bits/types/struct_timespec.c is unsafe (after having the
> > file ./time/bits/types/struct___timespec64.c exported)?  
> 
> I think "#define stat __stat64_time64" is unsafe, since people may do 
> "#undef stat" to get a function rather than a macro (and thereby
> suppress the macro expansion for the struct tag as well).
> 
> I'm less sure there would be any problems with "#define timespec 
> __timespec64".  The POSIX namespace rules are clearer on e.g. what's 
> reserved with file scope, than on what's reserved as a macro name
> that the user mustn't undefine.
> 

Please correct me if I'm wrong, but the scheme shall be consistent.
Which approach shall be taken then?


Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 2/3] msqid: Provide internal copy of struct __msqid64_ds

[Adhemerval Zanella]

[-- Attachment #1.1: Type: text/plain, Size: 4995 bytes --]



On 09/11/2020 21:02, Lukasz Majewski wrote:
> Hi Joseph, Adhemerval,
> 
>> On Mon, 9 Nov 2020, Lukasz Majewski wrote:
>>
>>>> If in a particular case defining the struct tag as a macro is
>>>> unsafe,  
>>>
>>> Could you explain why having construct as:
>>>
>>> #define timespec __timespec64 in
>>> ./time/bits/types/struct_timespec.c is unsafe (after having the
>>> file ./time/bits/types/struct___timespec64.c exported)?  
>>
>> I think "#define stat __stat64_time64" is unsafe, since people may do 
>> "#undef stat" to get a function rather than a macro (and thereby
>> suppress the macro expansion for the struct tag as well).
>>
>> I'm less sure there would be any problems with "#define timespec 
>> __timespec64".  The POSIX namespace rules are clearer on e.g. what's 
>> reserved with file scope, than on what's reserved as a macro name
>> that the user mustn't undefine.
>>
> 
> Please correct me if I'm wrong, but the scheme shall be consistent.
> Which approach shall be taken then?

As Joseph has put, some header name space allows the use of use 
identifiers prefix by the implementation.  However, there are
restriction as well of what the implementation is allowed to do,
check the 'Name Space' part at the POSIX definition [1].

For instance, for the sys/msg.h interface we can export the
'struct msqid64_ds' type since 'msg' is a reserve prefix. And I
think that doing

    #if __TIMESIZE == 32 && defined __USE_TIME_BITS64
    # define msqid_ds __msqid64_ds
    #endif

For 'sys/msg.h' should be safe as well.

However, doing:

  #if __TIMESIZE == 32 && defined __USE_TIME_BITS64
  # define timespec __timespec64
  #endif

It is not allowed since 'timespec' does not fall on the reserved
namespace of 'time.h' header.   As indicated by Joseph, if in a 
particular case defining the struct tag as a macro is unsafe (as for
timespec), we would need to redefine the struct member itself depending
of the _TIME_BITS.

It means that for timespec, we will need to do something as:

  1. On 'time/bits/types/struct_timespec.h' define 'struct timespec' as:

  struct timespec
  {
  #if __TIMESIZE == 32 && defined __USE_TIME_BITS64
    __tim64_t tv_sec;             /* Seconds.  */
  #else
    __time_t tv_sec;
  #endif
  #if __WORDSIZE == 64 \
    || (defined __SYSCALL_WORDSIZE && __SYSCALL_WORDSIZE == 64) \
    || __TIMESIZE == 32
    __syscall_slong_t tv_nsec;    /* Nanoseconds.  */
  #else
  # if __BYTE_ORDER == __BIG_ENDIAN
    int: 32;           /* Padding.  */
    long int tv_nsec;  /* Nanoseconds.  */
  # else
    long int tv_nsec;  /* Nanoseconds.  */
    int: 32;           /* Padding.  */
  # endif
  #endif
  };

  
  2. And for the interfaces that uses it, on time/time.h:

  [...]
  #ifdef __USE_POSIX199309
  [...]
  #if __TIMESIZE == 64 || !defined __USE_TIME_BITS64
  extern int nanosleep (const struct timespec *__requested_time,
                        struct timespec *__remaining);
  #else
  # ifdef __REDIRECT_NTH
  extern int __REDIRECT_NTH (nanosleep (const struct timespec *__requested_time,
				        struct timespec *__remaining),
                                        __nanosleep64);
  # else
  /* This might not be safe, but I don't see a better way to do it without
     __REDIRECT_NTH support.  */
  #  define nanosleep __nanosleep64
  # endif
 

  3. It would be good to add a internal tests to check if the export
     ABI for timespec does in fact match the internal definition.  A test that
     does something:

     #define _TIME_SIZE 64

     _Static_assert (sizeof (struct timespec)
                     == sizeof (struct __timespec64),
                     ...);
  
     _Static_assert (sizeof ((struct timespec){0}.tv_sec)
                     == sizeof ((struct __timespec64){0}.tv_nsec),
                     ...);
     _Static_assert (offsetof (struct timespec, tv_sec)
                     == offsetof (struct __timespec64, tv_sec),
                     ...); 
     _Static_assert (sizeof ((struct timespec){0}.tv_nsec)
                     == sizeof ((struct __timespec64){0}.tv_nsec),
                     ...);
     _Static_assert (offsetof (struct timespec, tv_nsec)
                     == offsetof (struct __timespec64, tv_nsec),
                     ...); 
      
     I don't think it needs test for the internal fields (unless the implementation
     does use it in some way, which I don't think it is the case).

 
  4. For timespec we should change all symbols that uses it, including compound
     types.


  5. We need to make sure glibc does not build or define _TIME_SIZE internally
     (we already have issues if user tries to enable the LFS support, so it should
     be ok as-is with default values).


I think I have outlined what is required, but I might have overlooked something.


[1] https://pubs.opengroup.org/onlinepubs/9699919799/functions/V2_chap02.html


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 2/3] msqid: Provide internal copy of struct __msqid64_ds

[Lukasz Majewski]

[-- Attachment #1: Type: text/plain, Size: 7365 bytes --]

Hi Adhemerval,

> On 09/11/2020 21:02, Lukasz Majewski wrote:
> > Hi Joseph, Adhemerval,
> >   
> >> On Mon, 9 Nov 2020, Lukasz Majewski wrote:
> >>  
> >>>> If in a particular case defining the struct tag as a macro is
> >>>> unsafe,    
> >>>
> >>> Could you explain why having construct as:
> >>>
> >>> #define timespec __timespec64 in
> >>> ./time/bits/types/struct_timespec.c is unsafe (after having the
> >>> file ./time/bits/types/struct___timespec64.c exported)?    
> >>
> >> I think "#define stat __stat64_time64" is unsafe, since people may
> >> do "#undef stat" to get a function rather than a macro (and thereby
> >> suppress the macro expansion for the struct tag as well).
> >>
> >> I'm less sure there would be any problems with "#define timespec 
> >> __timespec64".  The POSIX namespace rules are clearer on e.g.
> >> what's reserved with file scope, than on what's reserved as a
> >> macro name that the user mustn't undefine.
> >>  
> > 
> > Please correct me if I'm wrong, but the scheme shall be consistent.
> > Which approach shall be taken then?  
> 
> As Joseph has put, some header name space allows the use of use 
> identifiers prefix by the implementation.  However, there are
> restriction as well of what the implementation is allowed to do,
> check the 'Name Space' part at the POSIX definition [1].

Thanks for sharing the link. Now I do understand a bit more.

> 
> For instance, for the sys/msg.h interface we can export the
> 'struct msqid64_ds' type since 'msg' is a reserve prefix. And I
> think that doing
> 
>     #if __TIMESIZE == 32 && defined __USE_TIME_BITS64
>     # define msqid_ds __msqid64_ds
>     #endif
> 
> For 'sys/msg.h' should be safe as well.

According to the link [1] - the "msg" prefix is reserved in the
'sys/msg.h', but we do #define msqid_ds - the msq prefix is not
reserved...

Is this correct?

> 
> However, doing:
> 
>   #if __TIMESIZE == 32 && defined __USE_TIME_BITS64
>   # define timespec __timespec64
>   #endif
> 
> It is not allowed since 'timespec' does not fall on the reserved
> namespace of 'time.h' header. 

Definately - the timespec is not reserved in the time.h name space.

>  As indicated by Joseph, if in a 
> particular case defining the struct tag as a macro is unsafe (as for
> timespec), we would need to redefine the struct member itself
> depending of the _TIME_BITS.
> 
> It means that for timespec, we will need to do something as:
> 
>   1. On 'time/bits/types/struct_timespec.h' define 'struct timespec'
> as:
> 
>   struct timespec
>   {
>   #if __TIMESIZE == 32 && defined __USE_TIME_BITS64
>     __tim64_t tv_sec;             /* Seconds.  */
>   #else
>     __time_t tv_sec;
>   #endif
>   #if __WORDSIZE == 64 \
>     || (defined __SYSCALL_WORDSIZE && __SYSCALL_WORDSIZE == 64) \
>     || __TIMESIZE == 32
>     __syscall_slong_t tv_nsec;    /* Nanoseconds.  */
>   #else
>   # if __BYTE_ORDER == __BIG_ENDIAN
>     int: 32;           /* Padding.  */
>     long int tv_nsec;  /* Nanoseconds.  */
>   # else
>     long int tv_nsec;  /* Nanoseconds.  */
>     int: 32;           /* Padding.  */
>   # endif
>   #endif
>   };
> 

I do follow this approach - such implementation is in [2].

>   
>   2. And for the interfaces that uses it, on time/time.h:
> 
>   [...]
>   #ifdef __USE_POSIX199309
>   [...]
>   #if __TIMESIZE == 64 || !defined __USE_TIME_BITS64
>   extern int nanosleep (const struct timespec *__requested_time,
>                         struct timespec *__remaining);
>   #else
>   # ifdef __REDIRECT_NTH
>   extern int __REDIRECT_NTH (nanosleep (const struct timespec
> *__requested_time, struct timespec *__remaining),
>                                         __nanosleep64);
>   # else
>   /* This might not be safe, but I don't see a better way to do it
> without __REDIRECT_NTH support.  */
>   #  define nanosleep __nanosleep64
>   # endif
>  

This is also done in the code from [2]. However, there is no need to
have 
#if __TIMESIZE == 64 || !defined __USE_TIME_BITS64
as we override it when __USE_TIME_BITS64 is defined

> 
>   3. It would be good to add a internal tests to check if the export
>      ABI for timespec does in fact match the internal definition.  A
> test that does something:
> 
>      #define _TIME_SIZE 64
> 
>      _Static_assert (sizeof (struct timespec)
>                      == sizeof (struct __timespec64),
>                      ...);
>   
>      _Static_assert (sizeof ((struct timespec){0}.tv_sec)
>                      == sizeof ((struct __timespec64){0}.tv_nsec),
>                      ...);
>      _Static_assert (offsetof (struct timespec, tv_sec)
>                      == offsetof (struct __timespec64, tv_sec),
>                      ...); 
>      _Static_assert (sizeof ((struct timespec){0}.tv_nsec)
>                      == sizeof ((struct __timespec64){0}.tv_nsec),
>                      ...);
>      _Static_assert (offsetof (struct timespec, tv_nsec)
>                      == offsetof (struct __timespec64, tv_nsec),
>                      ...); 
>

This would be welcome, I will add such simple check when I add
__USE_TIME_BITS64 support (i.e. Y2038 support).

>      I don't think it needs test for the internal fields (unless the
> implementation does use it in some way, which I don't think it is the
> case).
> 

Ok.

>  
>   4. For timespec we should change all symbols that uses it,
> including compound types.

Yes - this is also done in patch [2]. For example struct itimerspec is
adjusted as well.

> 
> 
>   5. We need to make sure glibc does not build or define _TIME_SIZE
> internally (we already have issues if user tries to enable the LFS
> support, so it should be ok as-is with default values).

Do you think about _TIME_BITS=64 when the code is compiled
(-D_TIME_BITS=64)?
When -D_TIME_BITS=64 is passed during compilation of user program, the
__USE_TIME_BITS64 is set to 1 in exported headers, so aliasing for
Y2038 is enabled.

I'm not sure about the role of _TIME_SIZE. I only recall the __TIMESIZE
which is set depending on __WORDSIZE (or explicitly to 64 on e.g.
RISC-V).

> 
> 
> I think I have outlined what is required, but I might have overlooked
> something.

So to sum up:

- When prefix is reserved in POSIX namespaces [1] it may be allowed to
  export __foo64 struct and make the aliasing in the corresponding
  exported header.

- Otherwise, we need an internal glibc copy of the struct (i.e. struct
  __timespec64) and also corresponding exported struct (i.e. struct
  timespec) needs to be adjusted when __USE_TIME_BITS64 is defined.

The second case seems to be pervasive and is already utilized in my WIP
patch [2].

> 
> 
> [1]
> https://pubs.opengroup.org/onlinepubs/9699919799/functions/V2_chap02.html
> 


Links:

[2] -
https://github.com/lmajewski/y2038_glibc/commit/b5d9a0881ccc0a06e520c495ce7409f8e077fb20#diff-8297a1191cc06a8a7af6fc6ac33af4bef2d7d72efd901d1634d910b56631e0c7R13

Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 2/3] msqid: Provide internal copy of struct __msqid64_ds

[Adhemerval Zanella]

[-- Attachment #1.1: Type: text/plain, Size: 8445 bytes --]



On 11/11/2020 20:57, Lukasz Majewski wrote:
> Hi Adhemerval,
> 
>> On 09/11/2020 21:02, Lukasz Majewski wrote:
>>> Hi Joseph, Adhemerval,
>>>   
>>>> On Mon, 9 Nov 2020, Lukasz Majewski wrote:
>>>>  
>>>>>> If in a particular case defining the struct tag as a macro is
>>>>>> unsafe,    
>>>>>
>>>>> Could you explain why having construct as:
>>>>>
>>>>> #define timespec __timespec64 in
>>>>> ./time/bits/types/struct_timespec.c is unsafe (after having the
>>>>> file ./time/bits/types/struct___timespec64.c exported)?    
>>>>
>>>> I think "#define stat __stat64_time64" is unsafe, since people may
>>>> do "#undef stat" to get a function rather than a macro (and thereby
>>>> suppress the macro expansion for the struct tag as well).
>>>>
>>>> I'm less sure there would be any problems with "#define timespec 
>>>> __timespec64".  The POSIX namespace rules are clearer on e.g.
>>>> what's reserved with file scope, than on what's reserved as a
>>>> macro name that the user mustn't undefine.
>>>>  
>>>
>>> Please correct me if I'm wrong, but the scheme shall be consistent.
>>> Which approach shall be taken then?  
>>
>> As Joseph has put, some header name space allows the use of use 
>> identifiers prefix by the implementation.  However, there are
>> restriction as well of what the implementation is allowed to do,
>> check the 'Name Space' part at the POSIX definition [1].
> 
> Thanks for sharing the link. Now I do understand a bit more.
> 
>>
>> For instance, for the sys/msg.h interface we can export the
>> 'struct msqid64_ds' type since 'msg' is a reserve prefix. And I
>> think that doing
>>
>>     #if __TIMESIZE == 32 && defined __USE_TIME_BITS64
>>     # define msqid_ds __msqid64_ds
>>     #endif
>>
>> For 'sys/msg.h' should be safe as well.
> 
> According to the link [1] - the "msg" prefix is reserved in the
> 'sys/msg.h', but we do #define msqid_ds - the msq prefix is not
> reserved...
> 
> Is this correct?

Indeed, I overlooked this case. It seems we will need to go on the
same route I laid out for 'struct timespec' for the SysV interface
as well. It would require to change some arch-specific files I was
expecting to have to touch, but it seems there is no better option.

> 
>>
>> However, doing:
>>
>>   #if __TIMESIZE == 32 && defined __USE_TIME_BITS64
>>   # define timespec __timespec64
>>   #endif
>>
>> It is not allowed since 'timespec' does not fall on the reserved
>> namespace of 'time.h' header. 
> 
> Definately - the timespec is not reserved in the time.h name space.
> 
>>  As indicated by Joseph, if in a 
>> particular case defining the struct tag as a macro is unsafe (as for
>> timespec), we would need to redefine the struct member itself
>> depending of the _TIME_BITS.
>>
>> It means that for timespec, we will need to do something as:
>>
>>   1. On 'time/bits/types/struct_timespec.h' define 'struct timespec'
>> as:
>>
>>   struct timespec
>>   {
>>   #if __TIMESIZE == 32 && defined __USE_TIME_BITS64
>>     __tim64_t tv_sec;             /* Seconds.  */
>>   #else
>>     __time_t tv_sec;
>>   #endif
>>   #if __WORDSIZE == 64 \
>>     || (defined __SYSCALL_WORDSIZE && __SYSCALL_WORDSIZE == 64) \
>>     || __TIMESIZE == 32
>>     __syscall_slong_t tv_nsec;    /* Nanoseconds.  */
>>   #else
>>   # if __BYTE_ORDER == __BIG_ENDIAN
>>     int: 32;           /* Padding.  */
>>     long int tv_nsec;  /* Nanoseconds.  */
>>   # else
>>     long int tv_nsec;  /* Nanoseconds.  */
>>     int: 32;           /* Padding.  */
>>   # endif
>>   #endif
>>   };
>>
> 
> I do follow this approach - such implementation is in [2].

Yeah, I though we would be able to simplify the boilerplate required,
but it seems we are constrained by the POSIX namespace names.

> 
>>   
>>   2. And for the interfaces that uses it, on time/time.h:
>>
>>   [...]
>>   #ifdef __USE_POSIX199309
>>   [...]
>>   #if __TIMESIZE == 64 || !defined __USE_TIME_BITS64
>>   extern int nanosleep (const struct timespec *__requested_time,
>>                         struct timespec *__remaining);
>>   #else
>>   # ifdef __REDIRECT_NTH
>>   extern int __REDIRECT_NTH (nanosleep (const struct timespec
>> *__requested_time, struct timespec *__remaining),
>>                                         __nanosleep64);
>>   # else
>>   /* This might not be safe, but I don't see a better way to do it
>> without __REDIRECT_NTH support.  */
>>   #  define nanosleep __nanosleep64
>>   # endif
>>  
> 
> This is also done in the code from [2]. However, there is no need to
> have 
> #if __TIMESIZE == 64 || !defined __USE_TIME_BITS64
> as we override it when __USE_TIME_BITS64 is defined
> 
>>
>>   3. It would be good to add a internal tests to check if the export
>>      ABI for timespec does in fact match the internal definition.  A
>> test that does something:
>>
>>      #define _TIME_SIZE 64
>>
>>      _Static_assert (sizeof (struct timespec)
>>                      == sizeof (struct __timespec64),
>>                      ...);
>>   
>>      _Static_assert (sizeof ((struct timespec){0}.tv_sec)
>>                      == sizeof ((struct __timespec64){0}.tv_nsec),
>>                      ...);
>>      _Static_assert (offsetof (struct timespec, tv_sec)
>>                      == offsetof (struct __timespec64, tv_sec),
>>                      ...); 
>>      _Static_assert (sizeof ((struct timespec){0}.tv_nsec)
>>                      == sizeof ((struct __timespec64){0}.tv_nsec),
>>                      ...);
>>      _Static_assert (offsetof (struct timespec, tv_nsec)
>>                      == offsetof (struct __timespec64, tv_nsec),
>>                      ...); 
>>
> 
> This would be welcome, I will add such simple check when I add
> __USE_TIME_BITS64 support (i.e. Y2038 support).
> 
>>      I don't think it needs test for the internal fields (unless the
>> implementation does use it in some way, which I don't think it is the
>> case).
>>
> 
> Ok.
> 
>>  
>>   4. For timespec we should change all symbols that uses it,
>> including compound types.
> 
> Yes - this is also done in patch [2]. For example struct itimerspec is
> adjusted as well.
> 
>>
>>
>>   5. We need to make sure glibc does not build or define _TIME_SIZE
>> internally (we already have issues if user tries to enable the LFS
>> support, so it should be ok as-is with default values).
> 
> Do you think about _TIME_BITS=64 when the code is compiled
> (-D_TIME_BITS=64)?
> When -D_TIME_BITS=64 is passed during compilation of user program, the
> __USE_TIME_BITS64 is set to 1 in exported headers, so aliasing for
> Y2038 is enabled.

I meant building glibc itself, not user programs using glibc provided
headers.  It is to ensure that internally 'struct timespec' (or any
affected struct by 64-bit time) will have 32-bit ABI layout for ABIs
that supports it since glibc itself includes installed headers.

> 
> I'm not sure about the role of _TIME_SIZE. I only recall the __TIMESIZE
> which is set depending on __WORDSIZE (or explicitly to 64 on e.g.
> RISC-V).

My understanding is _TIME_SIZE will be the compiler flag used by the
programs to define which time ABI they want to use.

> 
>>
>>
>> I think I have outlined what is required, but I might have overlooked
>> something.
> 
> So to sum up:
> 
> - When prefix is reserved in POSIX namespaces [1] it may be allowed to
>   export __foo64 struct and make the aliasing in the corresponding
>   exported header.
> 
> - Otherwise, we need an internal glibc copy of the struct (i.e. struct
>   __timespec64) and also corresponding exported struct (i.e. struct
>   timespec) needs to be adjusted when __USE_TIME_BITS64 is defined.
> 
> The second case seems to be pervasive and is already utilized in my WIP
> patch [2].

I think to simplify we should for [2] as you initially suggested.  I
though we could mimic LFS support, but for y2038 support it seems that
we will need to re-define the types without using different names for the
affected types.

> 
>>
>>
>> [1]
>> https://pubs.opengroup.org/onlinepubs/9699919799/functions/V2_chap02.html
>>
> 
> 
> Links:
> 
> [2] -
> https://github.com/lmajewski/y2038_glibc/commit/b5d9a0881ccc0a06e520c495ce7409f8e077fb20#diff-8297a1191cc06a8a7af6fc6ac33af4bef2d7d72efd901d1634d910b56631e0c7R13


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 2/3] msqid: Provide internal copy of struct __msqid64_ds

[Lukasz Majewski]

[-- Attachment #1: Type: text/plain, Size: 10085 bytes --]

Hi Adhemerval,

> On 11/11/2020 20:57, Lukasz Majewski wrote:
> > Hi Adhemerval,
> >   
> >> On 09/11/2020 21:02, Lukasz Majewski wrote:  
> >>> Hi Joseph, Adhemerval,
> >>>     
> >>>> On Mon, 9 Nov 2020, Lukasz Majewski wrote:
> >>>>    
> >>>>>> If in a particular case defining the struct tag as a macro is
> >>>>>> unsafe,      
> >>>>>
> >>>>> Could you explain why having construct as:
> >>>>>
> >>>>> #define timespec __timespec64 in
> >>>>> ./time/bits/types/struct_timespec.c is unsafe (after having the
> >>>>> file ./time/bits/types/struct___timespec64.c exported)?      
> >>>>
> >>>> I think "#define stat __stat64_time64" is unsafe, since people
> >>>> may do "#undef stat" to get a function rather than a macro (and
> >>>> thereby suppress the macro expansion for the struct tag as well).
> >>>>
> >>>> I'm less sure there would be any problems with "#define timespec 
> >>>> __timespec64".  The POSIX namespace rules are clearer on e.g.
> >>>> what's reserved with file scope, than on what's reserved as a
> >>>> macro name that the user mustn't undefine.
> >>>>    
> >>>
> >>> Please correct me if I'm wrong, but the scheme shall be
> >>> consistent. Which approach shall be taken then?    
> >>
> >> As Joseph has put, some header name space allows the use of use 
> >> identifiers prefix by the implementation.  However, there are
> >> restriction as well of what the implementation is allowed to do,
> >> check the 'Name Space' part at the POSIX definition [1].  
> > 
> > Thanks for sharing the link. Now I do understand a bit more.
> >   
> >>
> >> For instance, for the sys/msg.h interface we can export the
> >> 'struct msqid64_ds' type since 'msg' is a reserve prefix. And I
> >> think that doing
> >>
> >>     #if __TIMESIZE == 32 && defined __USE_TIME_BITS64
> >>     # define msqid_ds __msqid64_ds
> >>     #endif
> >>
> >> For 'sys/msg.h' should be safe as well.  
> > 
> > According to the link [1] - the "msg" prefix is reserved in the
> > 'sys/msg.h', but we do #define msqid_ds - the msq prefix is not
> > reserved...
> > 
> > Is this correct?  
> 
> Indeed, I overlooked this case. It seems we will need to go on the
> same route I laid out for 'struct timespec' for the SysV interface
> as well. It would require to change some arch-specific files I was
> expecting to have to touch, but it seems there is no better option.

You may also consider reusing following patches:
https://github.com/lmajewski/y2038_glibc/commit/e7acba0ea587afab875f62eb132608ba6a679af5
https://github.com/lmajewski/y2038_glibc/commit/4c42b58c539af778dc17476187fa8330db37d7d2
https://github.com/lmajewski/y2038_glibc/commit/0ade361931988d6918888019d233335c82acc087

> 
> >   
> >>
> >> However, doing:
> >>
> >>   #if __TIMESIZE == 32 && defined __USE_TIME_BITS64
> >>   # define timespec __timespec64
> >>   #endif
> >>
> >> It is not allowed since 'timespec' does not fall on the reserved
> >> namespace of 'time.h' header.   
> > 
> > Definately - the timespec is not reserved in the time.h name space.
> >   
> >>  As indicated by Joseph, if in a 
> >> particular case defining the struct tag as a macro is unsafe (as
> >> for timespec), we would need to redefine the struct member itself
> >> depending of the _TIME_BITS.
> >>
> >> It means that for timespec, we will need to do something as:
> >>
> >>   1. On 'time/bits/types/struct_timespec.h' define 'struct
> >> timespec' as:
> >>
> >>   struct timespec
> >>   {
> >>   #if __TIMESIZE == 32 && defined __USE_TIME_BITS64
> >>     __tim64_t tv_sec;             /* Seconds.  */
> >>   #else
> >>     __time_t tv_sec;
> >>   #endif
> >>   #if __WORDSIZE == 64 \
> >>     || (defined __SYSCALL_WORDSIZE && __SYSCALL_WORDSIZE == 64) \
> >>     || __TIMESIZE == 32
> >>     __syscall_slong_t tv_nsec;    /* Nanoseconds.  */
> >>   #else
> >>   # if __BYTE_ORDER == __BIG_ENDIAN
> >>     int: 32;           /* Padding.  */
> >>     long int tv_nsec;  /* Nanoseconds.  */
> >>   # else
> >>     long int tv_nsec;  /* Nanoseconds.  */
> >>     int: 32;           /* Padding.  */
> >>   # endif
> >>   #endif
> >>   };
> >>  
> > 
> > I do follow this approach - such implementation is in [2].  
> 
> Yeah, I though we would be able to simplify the boilerplate required,
> but it seems we are constrained by the POSIX namespace names.

Ok.

> 
> >   
> >>   
> >>   2. And for the interfaces that uses it, on time/time.h:
> >>
> >>   [...]
> >>   #ifdef __USE_POSIX199309
> >>   [...]
> >>   #if __TIMESIZE == 64 || !defined __USE_TIME_BITS64
> >>   extern int nanosleep (const struct timespec *__requested_time,
> >>                         struct timespec *__remaining);
> >>   #else
> >>   # ifdef __REDIRECT_NTH
> >>   extern int __REDIRECT_NTH (nanosleep (const struct timespec
> >> *__requested_time, struct timespec *__remaining),
> >>                                         __nanosleep64);
> >>   # else
> >>   /* This might not be safe, but I don't see a better way to do it
> >> without __REDIRECT_NTH support.  */
> >>   #  define nanosleep __nanosleep64
> >>   # endif
> >>    
> > 
> > This is also done in the code from [2]. However, there is no need to
> > have 
> > #if __TIMESIZE == 64 || !defined __USE_TIME_BITS64
> > as we override it when __USE_TIME_BITS64 is defined
> >   
> >>
> >>   3. It would be good to add a internal tests to check if the
> >> export ABI for timespec does in fact match the internal
> >> definition.  A test that does something:
> >>
> >>      #define _TIME_SIZE 64
> >>
> >>      _Static_assert (sizeof (struct timespec)
> >>                      == sizeof (struct __timespec64),
> >>                      ...);
> >>   
> >>      _Static_assert (sizeof ((struct timespec){0}.tv_sec)
> >>                      == sizeof ((struct __timespec64){0}.tv_nsec),
> >>                      ...);
> >>      _Static_assert (offsetof (struct timespec, tv_sec)
> >>                      == offsetof (struct __timespec64, tv_sec),
> >>                      ...); 
> >>      _Static_assert (sizeof ((struct timespec){0}.tv_nsec)
> >>                      == sizeof ((struct __timespec64){0}.tv_nsec),
> >>                      ...);
> >>      _Static_assert (offsetof (struct timespec, tv_nsec)
> >>                      == offsetof (struct __timespec64, tv_nsec),
> >>                      ...); 
> >>  
> > 
> > This would be welcome, I will add such simple check when I add
> > __USE_TIME_BITS64 support (i.e. Y2038 support).
> >   
> >>      I don't think it needs test for the internal fields (unless
> >> the implementation does use it in some way, which I don't think it
> >> is the case).
> >>  
> > 
> > Ok.
> >   
> >>  
> >>   4. For timespec we should change all symbols that uses it,
> >> including compound types.  
> > 
> > Yes - this is also done in patch [2]. For example struct itimerspec
> > is adjusted as well.
> >   
> >>
> >>
> >>   5. We need to make sure glibc does not build or define _TIME_SIZE
> >> internally (we already have issues if user tries to enable the LFS
> >> support, so it should be ok as-is with default values).  
> > 
> > Do you think about _TIME_BITS=64 when the code is compiled
> > (-D_TIME_BITS=64)?
> > When -D_TIME_BITS=64 is passed during compilation of user program,
> > the __USE_TIME_BITS64 is set to 1 in exported headers, so aliasing
> > for Y2038 is enabled.  
> 
> I meant building glibc itself, not user programs using glibc provided
> headers.  It is to ensure that internally 'struct timespec' (or any
> affected struct by 64-bit time) will have 32-bit ABI layout for ABIs
> that supports it since glibc itself includes installed headers.
> 

Could you elaborate on this?

I do guess that _TIME_BITS when added to glibc (i.e. when it will be
recognized by "exported" headers) will enable 64 bit time support on
archs with __WORDSIZE=32 (e.g. ARM 32 bit).

Then, after some time, we will add #define _TIME_BITS 64 at e.g.
features.h (exported) on eligible ports, which would mean that by
default support for 64 bit time is enabled on them.

Am I correct?

> > 
> > I'm not sure about the role of _TIME_SIZE. I only recall the
> > __TIMESIZE which is set depending on __WORDSIZE (or explicitly to
> > 64 on e.g. RISC-V).  
> 
> My understanding is _TIME_SIZE will be the compiler flag used by the
> programs to define which time ABI they want to use.

Yes, this is also my understanding.

> 
> >   
> >>
> >>
> >> I think I have outlined what is required, but I might have
> >> overlooked something.  
> > 
> > So to sum up:
> > 
> > - When prefix is reserved in POSIX namespaces [1] it may be allowed
> > to export __foo64 struct and make the aliasing in the corresponding
> >   exported header.
> > 
> > - Otherwise, we need an internal glibc copy of the struct (i.e.
> > struct __timespec64) and also corresponding exported struct (i.e.
> > struct timespec) needs to be adjusted when __USE_TIME_BITS64 is
> > defined.
> > 
> > The second case seems to be pervasive and is already utilized in my
> > WIP patch [2].  
> 
> I think to simplify we should for [2] as you initially suggested.  I
> though we could mimic LFS support, but for y2038 support it seems that
> we will need to re-define the types without using different names for
> the affected types.

Ok.

> 
> >   
> >>
> >>
> >> [1]
> >> https://pubs.opengroup.org/onlinepubs/9699919799/functions/V2_chap02.html
> >>  
> > 
> > 
> > Links:
> > 
> > [2] -
> > https://github.com/lmajewski/y2038_glibc/commit/b5d9a0881ccc0a06e520c495ce7409f8e077fb20#diff-8297a1191cc06a8a7af6fc6ac33af4bef2d7d72efd901d1634d910b56631e0c7R13
> >  
> 




Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 2/3] msqid: Provide internal copy of struct __msqid64_ds

[Adhemerval Zanella]

[-- Attachment #1.1: Type: text/plain, Size: 1456 bytes --]



On 12/11/2020 12:27, Lukasz Majewski wrote:
>> I meant building glibc itself, not user programs using glibc provided
>> headers.  It is to ensure that internally 'struct timespec' (or any
>> affected struct by 64-bit time) will have 32-bit ABI layout for ABIs
>> that supports it since glibc itself includes installed headers.
>>
> 
> Could you elaborate on this?
> 
> I do guess that _TIME_BITS when added to glibc (i.e. when it will be
> recognized by "exported" headers) will enable 64 bit time support on
> archs with __WORDSIZE=32 (e.g. ARM 32 bit).
> 
> Then, after some time, we will add #define _TIME_BITS 64 at e.g.
> features.h (exported) on eligible ports, which would mean that by
> default support for 64 bit time is enabled on them.
> 
> Am I correct?

I mean that _TIME_BITS=64 for __WORDSIZE=32 will need to be enable only
on testing, all the glibc objects will need be built with _TIME_BITS=32
so '__time_t' and related structures do not vary depending of the
compiler flag (same as support to build glibc itself with _TIME_BITS=64).

About making the _TIME_BITS=64 as default, since it implies
_FILE_OFFSET_BITS=N we will need to sort some issues first (BZ#14106 [1],
BZ#15766 [2], BZ#13047 [3] to name the ones I am aware).

[1] https://sourceware.org/bugzilla/show_bug.cgi?id=14106
[2] https://sourceware.org/bugzilla/show_bug.cgi?id=15766 
[3] https://sourceware.org/bugzilla/show_bug.cgi?id=13047


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 2/3] msqid: Provide internal copy of struct __msqid64_ds

[Lukasz Majewski]

[-- Attachment #1: Type: text/plain, Size: 2698 bytes --]

Hi Adhemerval,

> On 12/11/2020 12:27, Lukasz Majewski wrote:
> >> I meant building glibc itself, not user programs using glibc
> >> provided headers.  It is to ensure that internally 'struct
> >> timespec' (or any affected struct by 64-bit time) will have 32-bit
> >> ABI layout for ABIs that supports it since glibc itself includes
> >> installed headers. 
> > 
> > Could you elaborate on this?
> > 
> > I do guess that _TIME_BITS when added to glibc (i.e. when it will be
> > recognized by "exported" headers) will enable 64 bit time support on
> > archs with __WORDSIZE=32 (e.g. ARM 32 bit).
> > 
> > Then, after some time, we will add #define _TIME_BITS 64 at e.g.
> > features.h (exported) on eligible ports, which would mean that by
> > default support for 64 bit time is enabled on them.
> > 
> > Am I correct?  
> 
> I mean that _TIME_BITS=64 for __WORDSIZE=32 will need to be enable
> only on testing,

To be more specific - enabled when some program (user one or tst-* from
glibc) is compiled (and will be run on Y2038 supporting version of
glibc).

> all the glibc objects will need be built with
> _TIME_BITS=32 

I would just prefer to not define it when the glibc is built itself. At
least for my Yocto/OE build I had to explicitly remove the
-D_TIME_BITS= from system wide CFLAGS.

> so '__time_t' and related structures do not vary
> depending of the compiler flag (same as support to build glibc itself
> with _TIME_BITS=64).

IMHO the __USE_TIME_BITS64 will be set to 1 in (include/features.h ->
exported as /usr/include/features.h) in some point in time.

The same approach we now have with __USE_FILE_OFFSET64 in the same file.

> 
> About making the _TIME_BITS=64 as default, since it implies
> _FILE_OFFSET_BITS=N we will need to sort some issues first (BZ#14106
> [1], BZ#15766 [2], BZ#13047 [3] to name the ones I am aware).

Those issues are not resolved since 2012, so I do guess that those are
not very urgent or easily solvable.

The Y2038 support will have consistent name scheme for functions
aliased (e.g. foo) to 64 bit versions (__foo64 or __foo_time64). If we
can get away with having the latter exported, so they will not clash, it
shall be Ok. 

> 
> [1] https://sourceware.org/bugzilla/show_bug.cgi?id=14106
> [2] https://sourceware.org/bugzilla/show_bug.cgi?id=15766 
> [3] https://sourceware.org/bugzilla/show_bug.cgi?id=13047
> 




Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 2/3] msqid: Provide internal copy of struct __msqid64_ds

[Adhemerval Zanella]

[-- Attachment #1.1: Type: text/plain, Size: 3469 bytes --]



On 12/11/2020 19:02, Lukasz Majewski wrote:
> Hi Adhemerval,
> 
>> On 12/11/2020 12:27, Lukasz Majewski wrote:
>>>> I meant building glibc itself, not user programs using glibc
>>>> provided headers.  It is to ensure that internally 'struct
>>>> timespec' (or any affected struct by 64-bit time) will have 32-bit
>>>> ABI layout for ABIs that supports it since glibc itself includes
>>>> installed headers. 
>>>
>>> Could you elaborate on this?
>>>
>>> I do guess that _TIME_BITS when added to glibc (i.e. when it will be
>>> recognized by "exported" headers) will enable 64 bit time support on
>>> archs with __WORDSIZE=32 (e.g. ARM 32 bit).
>>>
>>> Then, after some time, we will add #define _TIME_BITS 64 at e.g.
>>> features.h (exported) on eligible ports, which would mean that by
>>> default support for 64 bit time is enabled on them.
>>>
>>> Am I correct?  
>>
>> I mean that _TIME_BITS=64 for __WORDSIZE=32 will need to be enable
>> only on testing,
> 
> To be more specific - enabled when some program (user one or tst-* from
> glibc) is compiled (and will be run on Y2038 supporting version of
> glibc).

Yes, similar to what we do for some LFS tests (by defining _FILE_OFFSET_BITS
either on Makefile or on the C file itself).

> 
>> all the glibc objects will need be built with
>> _TIME_BITS=32 
> 
> I would just prefer to not define it when the glibc is built itself. At
> least for my Yocto/OE build I had to explicitly remove the
> -D_TIME_BITS= from system wide CFLAGS.

I don't think we need to explicit define it, a lot of implementations already
expects that default values to correct build the expected objects. What I
meant is we should not support passing _TIME_BITS on the glibc build CFLAGS
itself.

> 
>> so '__time_t' and related structures do not vary
>> depending of the compiler flag (same as support to build glibc itself
>> with _TIME_BITS=64).
> 
> IMHO the __USE_TIME_BITS64 will be set to 1 in (include/features.h ->
> exported as /usr/include/features.h) in some point in time.
> 
> The same approach we now have with __USE_FILE_OFFSET64 in the same file.
> 
>>
>> About making the _TIME_BITS=64 as default, since it implies
>> _FILE_OFFSET_BITS=N we will need to sort some issues first (BZ#14106
>> [1], BZ#15766 [2], BZ#13047 [3] to name the ones I am aware).
> 
> Those issues are not resolved since 2012, so I do guess that those are
> not very urgent or easily solvable.
> 
> The Y2038 support will have consistent name scheme for functions
> aliased (e.g. foo) to 64 bit versions (__foo64 or __foo_time64). If we
> can get away with having the latter exported, so they will not clash, it
> shall be Ok. 

I would guess there are not easily solvable, so I would expect that
the default _TIME_BITS for 32-bit architecture with legacy 32-bit
time_t support would still be '32'.  This unfortunately would require
to be explicit enable on the projects build system and it might not
straightforwards to enable worldwide since we got reports that recent
debian builds still do not enable LFS support on all packages.

I do think we should eventually enable both LFS and 64-bit time as
default for *all* architectures; this solves a lot of possible
issues.

>>
>> [1] https://sourceware.org/bugzilla/show_bug.cgi?id=14106
>> [2] https://sourceware.org/bugzilla/show_bug.cgi?id=15766 
>> [3] https://sourceware.org/bugzilla/show_bug.cgi?id=13047


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 2/3] msqid: Provide internal copy of struct __msqid64_ds

[Lukasz Majewski]

[-- Attachment #1: Type: text/plain, Size: 4521 bytes --]

Hi Adhemerval,

> On 12/11/2020 19:02, Lukasz Majewski wrote:
> > Hi Adhemerval,
> >   
> >> On 12/11/2020 12:27, Lukasz Majewski wrote:  
> >>>> I meant building glibc itself, not user programs using glibc
> >>>> provided headers.  It is to ensure that internally 'struct
> >>>> timespec' (or any affected struct by 64-bit time) will have
> >>>> 32-bit ABI layout for ABIs that supports it since glibc itself
> >>>> includes installed headers.   
> >>>
> >>> Could you elaborate on this?
> >>>
> >>> I do guess that _TIME_BITS when added to glibc (i.e. when it will
> >>> be recognized by "exported" headers) will enable 64 bit time
> >>> support on archs with __WORDSIZE=32 (e.g. ARM 32 bit).
> >>>
> >>> Then, after some time, we will add #define _TIME_BITS 64 at e.g.
> >>> features.h (exported) on eligible ports, which would mean that by
> >>> default support for 64 bit time is enabled on them.
> >>>
> >>> Am I correct?    
> >>
> >> I mean that _TIME_BITS=64 for __WORDSIZE=32 will need to be enable
> >> only on testing,  
> > 
> > To be more specific - enabled when some program (user one or tst-*
> > from glibc) is compiled (and will be run on Y2038 supporting
> > version of glibc).  
> 
> Yes, similar to what we do for some LFS tests (by defining
> _FILE_OFFSET_BITS either on Makefile or on the C file itself).

Ok.

> 
> >   
> >> all the glibc objects will need be built with
> >> _TIME_BITS=32   
> > 
> > I would just prefer to not define it when the glibc is built
> > itself. At least for my Yocto/OE build I had to explicitly remove
> > the -D_TIME_BITS= from system wide CFLAGS.  
> 
> I don't think we need to explicit define it, a lot of implementations
> already expects that default values to correct build the expected
> objects.

I do agree.

> What I meant is we should not support passing _TIME_BITS on
> the glibc build CFLAGS itself.

So in other words we shall check if by any chance the _TIME_BITS were
passed to glibc's CFLAGS.

> 
> >   
> >> so '__time_t' and related structures do not vary
> >> depending of the compiler flag (same as support to build glibc
> >> itself with _TIME_BITS=64).  
> > 
> > IMHO the __USE_TIME_BITS64 will be set to 1 in (include/features.h
> > -> exported as /usr/include/features.h) in some point in time.
> > 
> > The same approach we now have with __USE_FILE_OFFSET64 in the same
> > file. 
> >>
> >> About making the _TIME_BITS=64 as default, since it implies
> >> _FILE_OFFSET_BITS=N we will need to sort some issues first
> >> (BZ#14106 [1], BZ#15766 [2], BZ#13047 [3] to name the ones I am
> >> aware).  
> > 
> > Those issues are not resolved since 2012, so I do guess that those
> > are not very urgent or easily solvable.
> > 
> > The Y2038 support will have consistent name scheme for functions
> > aliased (e.g. foo) to 64 bit versions (__foo64 or __foo_time64). If
> > we can get away with having the latter exported, so they will not
> > clash, it shall be Ok.   
> 
> I would guess there are not easily solvable, so I would expect that
> the default _TIME_BITS for 32-bit architecture with legacy 32-bit
> time_t support would still be '32'. 

No,  in this case we would use the names without any suffix - e.g.
clock_gettime instead of __clock_gettime64.

> This unfortunately would require
> to be explicit enable on the projects build system and it might not
> straightforwards to enable worldwide since we got reports that recent
> debian builds still do not enable LFS support on all packages.

I think that it will not be so problematic in the end of the day. When
we set default minimal supported kernel version to 5.1+, the _TIME_BITS
= 64 can be enabled by default (on ports with __WORDSIZE=32 &&
__TIMESIZE != 64). Until then users would be allowed to pass it with
-D/Makefile.

> 
> I do think we should eventually enable both LFS and 64-bit time as
> default for *all* architectures; this solves a lot of possible
> issues.

Yes. I do agree.

> 
> >>
> >> [1] https://sourceware.org/bugzilla/show_bug.cgi?id=14106
> >> [2] https://sourceware.org/bugzilla/show_bug.cgi?id=15766 
> >> [3] https://sourceware.org/bugzilla/show_bug.cgi?id=13047  
> 




Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 2/3] msqid: Provide internal copy of struct __msqid64_ds

[Adhemerval Zanella]

[-- Attachment #1.1: Type: text/plain, Size: 2674 bytes --]



On 13/11/2020 11:42, Lukasz Majewski wrote:
> Hi Adhemerval,
> 
>> On 12/11/2020 19:02, Lukasz Majewski wrote:
>>> Those issues are not resolved since 2012, so I do guess that those
>>> are not very urgent or easily solvable.
>>>
>>> The Y2038 support will have consistent name scheme for functions
>>> aliased (e.g. foo) to 64 bit versions (__foo64 or __foo_time64). If
>>> we can get away with having the latter exported, so they will not
>>> clash, it shall be Ok.   
>>
>> I would guess there are not easily solvable, so I would expect that
>> the default _TIME_BITS for 32-bit architecture with legacy 32-bit
>> time_t support would still be '32'. 
> 
> No,  in this case we would use the names without any suffix - e.g.
> clock_gettime instead of __clock_gettime64.

The linkage name is already handle by the __REDIRECT (if compiler supports
is) macro, which creates a linkage alias.  The code will call 
clock_gettime, but static linking will bind to __clock_gettime64.

> 
>> This unfortunately would require
>> to be explicit enable on the projects build system and it might not
>> straightforwards to enable worldwide since we got reports that recent
>> debian builds still do not enable LFS support on all packages.
> 
> I think that it will not be so problematic in the end of the day. When
> we set default minimal supported kernel version to 5.1+, the _TIME_BITS
> = 64 can be enabled by default (on ports with __WORDSIZE=32 &&
> __TIMESIZE != 64). Until then users would be allowed to pass it with
> -D/Makefile.

I don't think this is that simple for 32-bit ports with 32-bit time_t,
no without fixing or discussing the LFS bug reports.  For
_FILE_OFFSET_BITS=64 I think the solution outline by Rich on comment#2
from BZ#14106 should be suffice (rename all the *64 functions to __*64
and update the headers accordingly), we will still need to keep exporting
the symbols for both compatibility and _LARGEFILE64_SOURCE.

> 
>>
>> I do think we should eventually enable both LFS and 64-bit time as
>> default for *all* architectures; this solves a lot of possible
>> issues.
> 
> Yes. I do agree.
> 
>>
>>>>
>>>> [1] https://sourceware.org/bugzilla/show_bug.cgi?id=14106
>>>> [2] https://sourceware.org/bugzilla/show_bug.cgi?id=15766 
>>>> [3] https://sourceware.org/bugzilla/show_bug.cgi?id=13047  
>>
> 
> 
> 
> 
> Best regards,
> 
> Lukasz Majewski
> 
> --
> 
> DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 2/3] msqid: Provide internal copy of struct __msqid64_ds

[Lukasz Majewski]

[-- Attachment #1: Type: text/plain, Size: 3443 bytes --]

Hi Adhemerval,

> On 13/11/2020 11:42, Lukasz Majewski wrote:
> > Hi Adhemerval,
> >   
> >> On 12/11/2020 19:02, Lukasz Majewski wrote:  
> >>> Those issues are not resolved since 2012, so I do guess that those
> >>> are not very urgent or easily solvable.
> >>>
> >>> The Y2038 support will have consistent name scheme for functions
> >>> aliased (e.g. foo) to 64 bit versions (__foo64 or __foo_time64).
> >>> If we can get away with having the latter exported, so they will
> >>> not clash, it shall be Ok.     
> >>
> >> I would guess there are not easily solvable, so I would expect that
> >> the default _TIME_BITS for 32-bit architecture with legacy 32-bit
> >> time_t support would still be '32'.   
> > 
> > No,  in this case we would use the names without any suffix - e.g.
> > clock_gettime instead of __clock_gettime64.  
> 
> The linkage name is already handle by the __REDIRECT (if compiler
> supports is) macro, which creates a linkage alias.  The code will
> call clock_gettime, but static linking will bind to __clock_gettime64.

I'm a bit confused now.

Please correct me if I'm wrong, but on 32 bit system (ARM) with
__TIMESIZE != 64 and _TIME_BITS==32 (or not defined) we will be linked
into clock_gettime (not __clock_gettime64).

> 
> >   
> >> This unfortunately would require
> >> to be explicit enable on the projects build system and it might not
> >> straightforwards to enable worldwide since we got reports that
> >> recent debian builds still do not enable LFS support on all
> >> packages.  
> > 
> > I think that it will not be so problematic in the end of the day.
> > When we set default minimal supported kernel version to 5.1+, the
> > _TIME_BITS = 64 can be enabled by default (on ports with
> > __WORDSIZE=32 && __TIMESIZE != 64). Until then users would be
> > allowed to pass it with -D/Makefile.  
> 
> I don't think this is that simple for 32-bit ports with 32-bit time_t,
> no without fixing or discussing the LFS bug reports.  For
> _FILE_OFFSET_BITS=64 I think the solution outline by Rich on comment#2
> from BZ#14106 should be suffice (rename all the *64 functions to __*64
> and update the headers accordingly), we will still need to keep
> exporting the symbols for both compatibility and _LARGEFILE64_SOURCE.
> 

Ok, I see.

However, this is just a side problem to Y2038, so it shouldn't be a
showstopper.

> >   
> >>
> >> I do think we should eventually enable both LFS and 64-bit time as
> >> default for *all* architectures; this solves a lot of possible
> >> issues.  
> > 
> > Yes. I do agree.
> >   
> >>  
> >>>>
> >>>> [1] https://sourceware.org/bugzilla/show_bug.cgi?id=14106
> >>>> [2] https://sourceware.org/bugzilla/show_bug.cgi?id=15766 
> >>>> [3] https://sourceware.org/bugzilla/show_bug.cgi?id=13047    
> >>  
> > 
> > 
> > 
> > 
> > Best regards,
> > 
> > Lukasz Majewski
> > 
> > --
> > 
> > DENX Software Engineering GmbH,      Managing Director: Wolfgang
> > Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell,
> > Germany Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email:
> > lukma@denx.de 
> 




Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 2/3] msqid: Provide internal copy of struct __msqid64_ds

[Adhemerval Zanella]

[-- Attachment #1.1: Type: text/plain, Size: 1519 bytes --]



On 13/11/2020 18:08, Lukasz Majewski wrote:
> Hi Adhemerval,
> 
>> On 13/11/2020 11:42, Lukasz Majewski wrote:
>>> Hi Adhemerval,
>>>   
>>>> On 12/11/2020 19:02, Lukasz Majewski wrote:  
>>>>> Those issues are not resolved since 2012, so I do guess that those
>>>>> are not very urgent or easily solvable.
>>>>>
>>>>> The Y2038 support will have consistent name scheme for functions
>>>>> aliased (e.g. foo) to 64 bit versions (__foo64 or __foo_time64).
>>>>> If we can get away with having the latter exported, so they will
>>>>> not clash, it shall be Ok.     
>>>>
>>>> I would guess there are not easily solvable, so I would expect that
>>>> the default _TIME_BITS for 32-bit architecture with legacy 32-bit
>>>> time_t support would still be '32'.   
>>>
>>> No,  in this case we would use the names without any suffix - e.g.
>>> clock_gettime instead of __clock_gettime64.  
>>
>> The linkage name is already handle by the __REDIRECT (if compiler
>> supports is) macro, which creates a linkage alias.  The code will
>> call clock_gettime, but static linking will bind to __clock_gettime64.
> 
> I'm a bit confused now.
> 
> Please correct me if I'm wrong, but on 32 bit system (ARM) with
> __TIMESIZE != 64 and _TIME_BITS==32 (or not defined) we will be linked
> into clock_gettime (not __clock_gettime64).

On 32-bit system with 32-bit time_t support (ARM, i386, etc.) 
_TIME_BITS=32 will link against clock_gettime while _TIME_BITS=64
should like to __clock_gettime64.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 2/3] msqid: Provide internal copy of struct __msqid64_ds

[Lukasz Majewski]

[-- Attachment #1: Type: text/plain, Size: 1976 bytes --]

On Mon, 16 Nov 2020 10:44:11 -0300
Adhemerval Zanella <adhemerval.zanella@linaro.org> wrote:

> On 13/11/2020 18:08, Lukasz Majewski wrote:
> > Hi Adhemerval,
> >   
> >> On 13/11/2020 11:42, Lukasz Majewski wrote:  
> >>> Hi Adhemerval,
> >>>     
> >>>> On 12/11/2020 19:02, Lukasz Majewski wrote:    
> >>>>> Those issues are not resolved since 2012, so I do guess that
> >>>>> those are not very urgent or easily solvable.
> >>>>>
> >>>>> The Y2038 support will have consistent name scheme for functions
> >>>>> aliased (e.g. foo) to 64 bit versions (__foo64 or __foo_time64).
> >>>>> If we can get away with having the latter exported, so they will
> >>>>> not clash, it shall be Ok.       
> >>>>
> >>>> I would guess there are not easily solvable, so I would expect
> >>>> that the default _TIME_BITS for 32-bit architecture with legacy
> >>>> 32-bit time_t support would still be '32'.     
> >>>
> >>> No,  in this case we would use the names without any suffix - e.g.
> >>> clock_gettime instead of __clock_gettime64.    
> >>
> >> The linkage name is already handle by the __REDIRECT (if compiler
> >> supports is) macro, which creates a linkage alias.  The code will
> >> call clock_gettime, but static linking will bind to
> >> __clock_gettime64.  
> > 
> > I'm a bit confused now.
> > 
> > Please correct me if I'm wrong, but on 32 bit system (ARM) with
> > __TIMESIZE != 64 and _TIME_BITS==32 (or not defined) we will be
> > linked into clock_gettime (not __clock_gettime64).  
> 
> On 32-bit system with 32-bit time_t support (ARM, i386, etc.) 
> _TIME_BITS=32 will link against clock_gettime while _TIME_BITS=64
> should like to __clock_gettime64.
> 

+1


Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 2/3] msqid: Provide internal copy of struct __msqid64_ds

[Lukasz Majewski]

[-- Attachment #1: Type: text/plain, Size: 2622 bytes --]

On Mon, 16 Nov 2020 15:25:21 +0100
Lukasz Majewski <lukma@denx.de> wrote:

> On Mon, 16 Nov 2020 10:44:11 -0300
> Adhemerval Zanella <adhemerval.zanella@linaro.org> wrote:
> 
> > On 13/11/2020 18:08, Lukasz Majewski wrote:  
> > > Hi Adhemerval,
> > >     
> > >> On 13/11/2020 11:42, Lukasz Majewski wrote:    
> > >>> Hi Adhemerval,
> > >>>       
> > >>>> On 12/11/2020 19:02, Lukasz Majewski wrote:      
> > >>>>> Those issues are not resolved since 2012, so I do guess that
> > >>>>> those are not very urgent or easily solvable.
> > >>>>>
> > >>>>> The Y2038 support will have consistent name scheme for
> > >>>>> functions aliased (e.g. foo) to 64 bit versions (__foo64 or
> > >>>>> __foo_time64). If we can get away with having the latter
> > >>>>> exported, so they will not clash, it shall be Ok.         
> > >>>>
> > >>>> I would guess there are not easily solvable, so I would expect
> > >>>> that the default _TIME_BITS for 32-bit architecture with legacy
> > >>>> 32-bit time_t support would still be '32'.       
> > >>>
> > >>> No,  in this case we would use the names without any suffix -
> > >>> e.g. clock_gettime instead of __clock_gettime64.      
> > >>
> > >> The linkage name is already handle by the __REDIRECT (if compiler
> > >> supports is) macro, which creates a linkage alias.  The code will
> > >> call clock_gettime, but static linking will bind to
> > >> __clock_gettime64.    
> > > 
> > > I'm a bit confused now.
> > > 
> > > Please correct me if I'm wrong, but on 32 bit system (ARM) with
> > > __TIMESIZE != 64 and _TIME_BITS==32 (or not defined) we will be
> > > linked into clock_gettime (not __clock_gettime64).    
> > 
> > On 32-bit system with 32-bit time_t support (ARM, i386, etc.) 
> > _TIME_BITS=32 will link against clock_gettime while _TIME_BITS=64
> > should like to __clock_gettime64.
> >   
> 
> +1

Is this patch itself OK, to be pulled in some near future? It can be
pulled now (shall not harm) and be a good preparation for the commit
adding support for Y2038.

> 
> 
> Best regards,
> 
> Lukasz Majewski
> 
> --
> 
> DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email:
> lukma@denx.de




Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 2/3] msqid: Provide internal copy of struct __msqid64_ds

[Lukasz Majewski]

[-- Attachment #1: Type: text/plain, Size: 3157 bytes --]

Dear Community,

> On Mon, 16 Nov 2020 15:25:21 +0100
> Lukasz Majewski <lukma@denx.de> wrote:
> 
> > On Mon, 16 Nov 2020 10:44:11 -0300
> > Adhemerval Zanella <adhemerval.zanella@linaro.org> wrote:
> >   
> > > On 13/11/2020 18:08, Lukasz Majewski wrote:    
> > > > Hi Adhemerval,
> > > >       
> > > >> On 13/11/2020 11:42, Lukasz Majewski wrote:      
> > > >>> Hi Adhemerval,
> > > >>>         
> > > >>>> On 12/11/2020 19:02, Lukasz Majewski wrote:        
> > > >>>>> Those issues are not resolved since 2012, so I do guess that
> > > >>>>> those are not very urgent or easily solvable.
> > > >>>>>
> > > >>>>> The Y2038 support will have consistent name scheme for
> > > >>>>> functions aliased (e.g. foo) to 64 bit versions (__foo64 or
> > > >>>>> __foo_time64). If we can get away with having the latter
> > > >>>>> exported, so they will not clash, it shall be Ok.           
> > > >>>>
> > > >>>> I would guess there are not easily solvable, so I would
> > > >>>> expect that the default _TIME_BITS for 32-bit architecture
> > > >>>> with legacy 32-bit time_t support would still be '32'.
> > > >>>>   
> > > >>>
> > > >>> No,  in this case we would use the names without any suffix -
> > > >>> e.g. clock_gettime instead of __clock_gettime64.        
> > > >>
> > > >> The linkage name is already handle by the __REDIRECT (if
> > > >> compiler supports is) macro, which creates a linkage alias.
> > > >> The code will call clock_gettime, but static linking will bind
> > > >> to __clock_gettime64.      
> > > > 
> > > > I'm a bit confused now.
> > > > 
> > > > Please correct me if I'm wrong, but on 32 bit system (ARM) with
> > > > __TIMESIZE != 64 and _TIME_BITS==32 (or not defined) we will be
> > > > linked into clock_gettime (not __clock_gettime64).      
> > > 
> > > On 32-bit system with 32-bit time_t support (ARM, i386, etc.) 
> > > _TIME_BITS=32 will link against clock_gettime while _TIME_BITS=64
> > > should like to __clock_gettime64.
> > >     
> > 
> > +1  
> 
> Is this patch itself OK, to be pulled in some near future? It can be
> pulled now (shall not harm) and be a good preparation for the commit
> adding support for Y2038.

A gentle ping for review on this patch. Thanks in advance.

> 
> > 
> > 
> > Best regards,
> > 
> > Lukasz Majewski
> > 
> > --
> > 
> > DENX Software Engineering GmbH,      Managing Director: Wolfgang
> > Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell,
> > Germany Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email:
> > lukma@denx.de  
> 
> 
> 
> 
> Best regards,
> 
> Lukasz Majewski
> 
> --
> 
> DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email:
> lukma@denx.de




Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 2/3] msqid: Provide internal copy of struct __msqid64_ds

[Adhemerval Zanella]

On 09/11/2020 15:39, Joseph Myers wrote:
> On Mon, 9 Nov 2020, Adhemerval Zanella via Libc-alpha wrote:
> 
>> On 06/11/2020 14:37, Joseph Myers wrote:
>>> On Fri, 6 Nov 2020, Lukasz Majewski wrote:
>>>
>>>> 3. The exported header (from point 1) shall have:
>>>>
>>>> #if __TIMESIZE == 32 && defined __USE_TIME_BITS64
>>>> # define timespec __timespec64
>>>> #endif
>>>
>>> You'll need to make sure that's permitted under the namespace reservations 
>>> in POSIX.
>>>
>>> For the cases where a struct tag has the same name as a function (e.g. 
>>> stat), #undef of the function name is definitely allowed for user 
>>> programs, so relying on defining the struct tag as a macro is probably 
>>> unsafe in that case.
>>
>> In these which alternative would be better?  Export __timespec64 as 
>> different name or use the strategy to redefine timespec to different
>> member sizes and layout depending of time ABI?
> 
> If in a particular case defining the struct tag as a macro is unsafe, we 
> can't avoid having different definitions of the contents of the same 
> struct tag depending on how _TIME_BITS is defined.

This might result is possible linking issues with C++ and mangling
if objects built with different time ABI get mixed together, although
I don't have a better solution.

[PATCH 3/3] msg: provide glibc local copy of struct msqid_ds

[Lukasz Majewski]

The rationale for this change is to provide y2038 support for
syscalls which use struct msqid_ds as __USE_TIME64_BITS is only available
in exported headers.

After this commit there is no need to unconditionally
#include <sysvipc/sys/msg.h> header, which prevents from using the
exported struct msqid_ds. The aforementioned header is only included when
_ISOMAC is defined - e.g. when the testsuite is built and executed.

For example the ./sysdeps/unix/sysv/linux/msgctl.c includes <sys/msg.h>:
			(include/sys/msg.h) which has
			#include_next <sys/msg.h>
				    |
				   \|/
			./sysdeps/unix/sysv/linux/include/sys/msg.h
				    |
				   \|/
	(./sysvipc/sys/msg.h) which unconditionally includes
        ./sysdeps/unix/sysv/linux/bits/msq.h
				    |
				   \|/
        ./sysdeps/unix/sysv/linux/bits/types/struct_msqid_ds.h

As a result the externally exported struct msqid_ds.h is used internally
by glibc.

To fix this issue - the internal for glibc copy of struct msqid_ds
has been re-introduced. This will allow clear separation between exported
and internal glibc types.

Build tests:
./src/scripts/build-many-glibcs.py glibcs
---
 sysdeps/unix/sysv/linux/include/sys/msg.h | 66 +++++++++++++++++++++--
 1 file changed, 62 insertions(+), 4 deletions(-)

diff --git a/sysdeps/unix/sysv/linux/include/sys/msg.h b/sysdeps/unix/sysv/linux/include/sys/msg.h
index 522903f818..1057c2e278 100644
--- a/sysdeps/unix/sysv/linux/include/sys/msg.h
+++ b/sysdeps/unix/sysv/linux/include/sys/msg.h
@@ -1,7 +1,63 @@
 #ifndef _SYS_MSG_H
-#include <sysvipc/sys/msg.h>
-
 #ifndef _ISOMAC
+# include <time.h>
+# include <sys/ipc.h>
+
+/* Types used in the MSQID_DS structure definition.  */
+typedef __syscall_ulong_t msgqnum_t;
+typedef __syscall_ulong_t msglen_t;
+
+struct msqid_ds
+{
+  struct ipc_perm msg_perm;	/* structure describing operation permission */
+# if __TIMESIZE == 32
+  __time_t msg_stime;		/* time of last msgsnd command */
+  unsigned long int __msg_stime_high;
+  __time_t msg_rtime;		/* time of last msgsnd command */
+  unsigned long int __msg_rtime_high;
+  __time_t msg_ctime;		/* time of last change */
+  unsigned long int __msg_ctime_high;
+# else
+  __time_t msg_stime;		/* time of last msgsnd command */
+  __time_t msg_rtime;		/* time of last msgsnd command */
+  __time_t msg_ctime;		/* time of last change */
+# endif
+  __syscall_ulong_t __msg_cbytes; /* current number of bytes on queue */
+  msgqnum_t msg_qnum;		/* number of messages currently on queue */
+  msglen_t msg_qbytes;		/* max number of bytes allowed on queue */
+  __pid_t msg_lspid;		/* pid of last msgsnd() */
+  __pid_t msg_lrpid;		/* pid of last msgrcv() */
+  __syscall_ulong_t __glibc_reserved4;
+  __syscall_ulong_t __glibc_reserved5;
+};
+
+# ifdef __USE_MISC
+#  define msg_cbytes	__msg_cbytes
+
+/* ipcs ctl commands */
+#  define MSG_STAT 11
+#  define MSG_INFO 12
+#  define MSG_STAT_ANY 13
+
+/* buffer for msgctl calls IPC_INFO, MSG_INFO */
+struct msginfo
+  {
+    int msgpool;
+    int msgmap;
+    int msgmax;
+    int msgmnb;
+    int msgmni;
+    int msgssz;
+    int msgtql;
+    unsigned short int msgseg;
+  };
+# endif /* __USE_MISC */
+
+# ifndef __ssize_t_defined
+typedef __ssize_t ssize_t;
+#  define __ssize_t_defined
+# endif
+
 extern ssize_t __libc_msgrcv (int msqid, void *msgp, size_t msgsz,
 			      long int msgtyp, int msgflg);
 extern int __libc_msgsnd (int msqid, const void *msgp, size_t msgsz,
@@ -15,7 +71,9 @@ extern int __libc_msgsnd (int msqid, const void *msgp, size_t msgsz,
 extern int __msgctl64 (int msqid, int cmd, struct __msqid64_ds *buf);
 libc_hidden_proto (__msgctl64);
 # endif
-
-#endif
+#else /* _ISOMAC = 1 - e.g. testsuite */
+/* Provide exported sys/msg.h header */
+# include <sysvipc/sys/msg.h>
+#endif /* _ISOMAC */
 
 #endif
-- 
2.20.1

Re: [PATCH 3/3] msg: provide glibc local copy of struct msqid_ds

[Lukasz Majewski]

[-- Attachment #1: Type: text/plain, Size: 4489 bytes --]

Dear Community,

> The rationale for this change is to provide y2038 support for
> syscalls which use struct msqid_ds as __USE_TIME64_BITS is only
> available in exported headers.
> 
> After this commit there is no need to unconditionally
> #include <sysvipc/sys/msg.h> header, which prevents from using the
> exported struct msqid_ds. The aforementioned header is only included
> when _ISOMAC is defined - e.g. when the testsuite is built and
> executed.
> 
> For example the ./sysdeps/unix/sysv/linux/msgctl.c includes
> <sys/msg.h>: (include/sys/msg.h) which has
> 			#include_next <sys/msg.h>
> 				    |
> 				   \|/
> 			./sysdeps/unix/sysv/linux/include/sys/msg.h
> 				    |
> 				   \|/
> 	(./sysvipc/sys/msg.h) which unconditionally includes
>         ./sysdeps/unix/sysv/linux/bits/msq.h
> 				    |
> 				   \|/
>         ./sysdeps/unix/sysv/linux/bits/types/struct_msqid_ds.h
> 
> As a result the externally exported struct msqid_ds.h is used
> internally by glibc.
> 
> To fix this issue - the internal for glibc copy of struct msqid_ds
> has been re-introduced. This will allow clear separation between
> exported and internal glibc types.
> 
> Build tests:
> ./src/scripts/build-many-glibcs.py glibcs

I do assume that this approach is OK?

> ---
>  sysdeps/unix/sysv/linux/include/sys/msg.h | 66
> +++++++++++++++++++++-- 1 file changed, 62 insertions(+), 4
> deletions(-)
> 
> diff --git a/sysdeps/unix/sysv/linux/include/sys/msg.h
> b/sysdeps/unix/sysv/linux/include/sys/msg.h index
> 522903f818..1057c2e278 100644 ---
> a/sysdeps/unix/sysv/linux/include/sys/msg.h +++
> b/sysdeps/unix/sysv/linux/include/sys/msg.h @@ -1,7 +1,63 @@
>  #ifndef _SYS_MSG_H
> -#include <sysvipc/sys/msg.h>
> -
>  #ifndef _ISOMAC
> +# include <time.h>
> +# include <sys/ipc.h>
> +
> +/* Types used in the MSQID_DS structure definition.  */
> +typedef __syscall_ulong_t msgqnum_t;
> +typedef __syscall_ulong_t msglen_t;
> +
> +struct msqid_ds
> +{
> +  struct ipc_perm msg_perm;	/* structure describing operation
> permission */ +# if __TIMESIZE == 32
> +  __time_t msg_stime;		/* time of last msgsnd command
> */
> +  unsigned long int __msg_stime_high;
> +  __time_t msg_rtime;		/* time of last msgsnd command
> */
> +  unsigned long int __msg_rtime_high;
> +  __time_t msg_ctime;		/* time of last change */
> +  unsigned long int __msg_ctime_high;
> +# else
> +  __time_t msg_stime;		/* time of last msgsnd command
> */
> +  __time_t msg_rtime;		/* time of last msgsnd command
> */
> +  __time_t msg_ctime;		/* time of last change */
> +# endif
> +  __syscall_ulong_t __msg_cbytes; /* current number of bytes on
> queue */
> +  msgqnum_t msg_qnum;		/* number of messages currently
> on queue */
> +  msglen_t msg_qbytes;		/* max number of bytes allowed
> on queue */
> +  __pid_t msg_lspid;		/* pid of last msgsnd() */
> +  __pid_t msg_lrpid;		/* pid of last msgrcv() */
> +  __syscall_ulong_t __glibc_reserved4;
> +  __syscall_ulong_t __glibc_reserved5;
> +};
> +
> +# ifdef __USE_MISC
> +#  define msg_cbytes	__msg_cbytes
> +
> +/* ipcs ctl commands */
> +#  define MSG_STAT 11
> +#  define MSG_INFO 12
> +#  define MSG_STAT_ANY 13
> +
> +/* buffer for msgctl calls IPC_INFO, MSG_INFO */
> +struct msginfo
> +  {
> +    int msgpool;
> +    int msgmap;
> +    int msgmax;
> +    int msgmnb;
> +    int msgmni;
> +    int msgssz;
> +    int msgtql;
> +    unsigned short int msgseg;
> +  };
> +# endif /* __USE_MISC */
> +
> +# ifndef __ssize_t_defined
> +typedef __ssize_t ssize_t;
> +#  define __ssize_t_defined
> +# endif
> +
>  extern ssize_t __libc_msgrcv (int msqid, void *msgp, size_t msgsz,
>  			      long int msgtyp, int msgflg);
>  extern int __libc_msgsnd (int msqid, const void *msgp, size_t msgsz,
> @@ -15,7 +71,9 @@ extern int __libc_msgsnd (int msqid, const void
> *msgp, size_t msgsz, extern int __msgctl64 (int msqid, int cmd,
> struct __msqid64_ds *buf); libc_hidden_proto (__msgctl64);
>  # endif
> -
> -#endif
> +#else /* _ISOMAC = 1 - e.g. testsuite */
> +/* Provide exported sys/msg.h header */
> +# include <sysvipc/sys/msg.h>
> +#endif /* _ISOMAC */
>  
>  #endif




Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 3/3] msg: provide glibc local copy of struct msqid_ds

[Lukasz Majewski]

[-- Attachment #1: Type: text/plain, Size: 5161 bytes --]

On Fri, 4 Dec 2020 00:29:23 +0100
Lukasz Majewski <lukma@denx.de> wrote:

> Dear Community,
> 
> > The rationale for this change is to provide y2038 support for
> > syscalls which use struct msqid_ds as __USE_TIME64_BITS is only
> > available in exported headers.
> > 
> > After this commit there is no need to unconditionally
> > #include <sysvipc/sys/msg.h> header, which prevents from using the
> > exported struct msqid_ds. The aforementioned header is only included
> > when _ISOMAC is defined - e.g. when the testsuite is built and
> > executed.
> > 
> > For example the ./sysdeps/unix/sysv/linux/msgctl.c includes
> > <sys/msg.h>: (include/sys/msg.h) which has
> > 			#include_next <sys/msg.h>
> > 				    |
> > 				   \|/
> > 			./sysdeps/unix/sysv/linux/include/sys/msg.h
> > 				    |
> > 				   \|/
> > 	(./sysvipc/sys/msg.h) which unconditionally includes
> >         ./sysdeps/unix/sysv/linux/bits/msq.h
> > 				    |
> > 				   \|/
> >         ./sysdeps/unix/sysv/linux/bits/types/struct_msqid_ds.h
> > 
> > As a result the externally exported struct msqid_ds.h is used
> > internally by glibc.
> > 
> > To fix this issue - the internal for glibc copy of struct msqid_ds
> > has been re-introduced. This will allow clear separation between
> > exported and internal glibc types.
> > 
> > Build tests:
> > ./src/scripts/build-many-glibcs.py glibcs  
> 
> I do assume that this approach is OK?

Gentle ping on this patch.

> 
> > ---
> >  sysdeps/unix/sysv/linux/include/sys/msg.h | 66
> > +++++++++++++++++++++-- 1 file changed, 62 insertions(+), 4
> > deletions(-)
> > 
> > diff --git a/sysdeps/unix/sysv/linux/include/sys/msg.h
> > b/sysdeps/unix/sysv/linux/include/sys/msg.h index
> > 522903f818..1057c2e278 100644 ---
> > a/sysdeps/unix/sysv/linux/include/sys/msg.h +++
> > b/sysdeps/unix/sysv/linux/include/sys/msg.h @@ -1,7 +1,63 @@
> >  #ifndef _SYS_MSG_H
> > -#include <sysvipc/sys/msg.h>
> > -
> >  #ifndef _ISOMAC
> > +# include <time.h>
> > +# include <sys/ipc.h>
> > +
> > +/* Types used in the MSQID_DS structure definition.  */
> > +typedef __syscall_ulong_t msgqnum_t;
> > +typedef __syscall_ulong_t msglen_t;
> > +
> > +struct msqid_ds
> > +{
> > +  struct ipc_perm msg_perm;	/* structure describing
> > operation permission */ +# if __TIMESIZE == 32
> > +  __time_t msg_stime;		/* time of last msgsnd command
> > */
> > +  unsigned long int __msg_stime_high;
> > +  __time_t msg_rtime;		/* time of last msgsnd command
> > */
> > +  unsigned long int __msg_rtime_high;
> > +  __time_t msg_ctime;		/* time of last change */
> > +  unsigned long int __msg_ctime_high;
> > +# else
> > +  __time_t msg_stime;		/* time of last msgsnd command
> > */
> > +  __time_t msg_rtime;		/* time of last msgsnd command
> > */
> > +  __time_t msg_ctime;		/* time of last change */
> > +# endif
> > +  __syscall_ulong_t __msg_cbytes; /* current number of bytes on
> > queue */
> > +  msgqnum_t msg_qnum;		/* number of messages
> > currently on queue */
> > +  msglen_t msg_qbytes;		/* max number of bytes
> > allowed on queue */
> > +  __pid_t msg_lspid;		/* pid of last msgsnd() */
> > +  __pid_t msg_lrpid;		/* pid of last msgrcv() */
> > +  __syscall_ulong_t __glibc_reserved4;
> > +  __syscall_ulong_t __glibc_reserved5;
> > +};
> > +
> > +# ifdef __USE_MISC
> > +#  define msg_cbytes	__msg_cbytes
> > +
> > +/* ipcs ctl commands */
> > +#  define MSG_STAT 11
> > +#  define MSG_INFO 12
> > +#  define MSG_STAT_ANY 13
> > +
> > +/* buffer for msgctl calls IPC_INFO, MSG_INFO */
> > +struct msginfo
> > +  {
> > +    int msgpool;
> > +    int msgmap;
> > +    int msgmax;
> > +    int msgmnb;
> > +    int msgmni;
> > +    int msgssz;
> > +    int msgtql;
> > +    unsigned short int msgseg;
> > +  };
> > +# endif /* __USE_MISC */
> > +
> > +# ifndef __ssize_t_defined
> > +typedef __ssize_t ssize_t;
> > +#  define __ssize_t_defined
> > +# endif
> > +
> >  extern ssize_t __libc_msgrcv (int msqid, void *msgp, size_t msgsz,
> >  			      long int msgtyp, int msgflg);
> >  extern int __libc_msgsnd (int msqid, const void *msgp, size_t
> > msgsz, @@ -15,7 +71,9 @@ extern int __libc_msgsnd (int msqid, const
> > void *msgp, size_t msgsz, extern int __msgctl64 (int msqid, int cmd,
> > struct __msqid64_ds *buf); libc_hidden_proto (__msgctl64);
> >  # endif
> > -
> > -#endif
> > +#else /* _ISOMAC = 1 - e.g. testsuite */
> > +/* Provide exported sys/msg.h header */
> > +# include <sysvipc/sys/msg.h>
> > +#endif /* _ISOMAC */
> >  
> >  #endif  
> 
> 
> 
> 
> Best regards,
> 
> Lukasz Majewski
> 
> --
> 
> DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email:
> lukma@denx.de




Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 1/3] msg: Remove redundant #include <sys/msg.h> header

[Andreas Schwab]

On Okt 27 2020, Lukasz Majewski wrote:

> The #include <sys/msg.h> is redundant as we do not use message specific
> types for issuing syscalls to handle msg and shm. Only msgctl requires
> this header.

Ok.

Andreas.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."