[PATCH v2 14/25] y2038: Use a common definition for msqid_ds
Carlos O'Donell
carlos@redhat.com
Fri Jun 4 19:38:06 GMT 2021
On 5/18/21 4:56 PM, Adhemerval Zanella wrote:
> From: Lukasz Majewski <lukma@denx.de>
>
> Instead of replicate the same definitions from struct_msqid64_ds.h
> on the multiple struct_msqid_ds.h, use a common header which is included
> when required (struct_msqid64_ds_helper.h).
>
> The __USE_TIME_BITS64 is not defined internally yet, although the
> internal header is used when building the 64-bit stat implementations.
Requesting a v3 please.
Please add the __glibc_reserved* members to the structures to preserve
maximum compatibility with the kernel structure.
What I would like to see is that __glibc_reserved4 and __glibc_reserved5
are *preserved* for both ABIs in the dual-time case. The kernel for msqid
still has the following:
26 struct msqid64_ds {
27 struct ipc64_perm msg_perm;
28 #if __BITS_PER_LONG == 64
29 long msg_stime; /* last msgsnd time */
30 long msg_rtime; /* last msgrcv time */
31 long msg_ctime; /* last change time */
32 #else
33 unsigned long msg_stime; /* last msgsnd time */
34 unsigned long msg_stime_high;
35 unsigned long msg_rtime; /* last msgrcv time */
36 unsigned long msg_rtime_high;
37 unsigned long msg_ctime; /* last change time */
38 unsigned long msg_ctime_high;
39 #endif
40 unsigned long msg_cbytes; /* current number of bytes on queue */
41 unsigned long msg_qnum; /* number of messages in queue */
42 unsigned long msg_qbytes; /* max number of bytes on queue */
43 __kernel_pid_t msg_lspid; /* pid of last msgsnd */
44 __kernel_pid_t msg_lrpid; /* last receive pid */
45 unsigned long __unused4;
46 unsigned long __unused5;
47 };
For maximum compatibility with future kernel changes we should keep
the unused fields in the glibc equivalent structure. If the values are
ever used then they would already be allocated as part of the size of
the structure without the need for an ABI change. Given that these are
somewhat legacy APIs we should be aiming for preserving existing behaviour
rather than aiming for the smallest possible size structure.
On i686 the new 64-bit time_t struct is only 80 bytes, while the original
struct size is 88 bytes. Adding the reserved entries brings this back to
88 bytes in the new ABI. We can add static asserts if we think we need to
check exactly for alignment of 64-bit time_t, but since we copy from a
kernel structure I'm largely worried about the ABI being too small to cary
the information that we might need to copy from the matching kernel struct.
See my suggestions below for changes.
> ---
> sysdeps/unix/sysv/linux/Makefile | 3 +-
> .../linux/bits/struct_stat_time64_helper.h | 2 +-
> .../sysv/linux/bits/types/struct_msqid64_ds.h | 10 +------
> .../bits/types/struct_msqid64_ds_helper.h | 28 +++++++++++++++++++
> .../sysv/linux/bits/types/struct_msqid_ds.h | 12 ++++++--
> .../linux/hppa/bits/types/struct_msqid_ds.h | 12 ++++++--
> .../linux/mips/bits/types/struct_msqid_ds.h | 18 ++++++++----
> .../powerpc/bits/types/struct_msqid_ds.h | 12 ++++++--
> .../linux/sparc/bits/types/struct_msqid_ds.h | 12 ++++++--
> 9 files changed, 80 insertions(+), 29 deletions(-)
> create mode 100644 sysdeps/unix/sysv/linux/bits/types/struct_msqid64_ds_helper.h
>
> diff --git a/sysdeps/unix/sysv/linux/Makefile b/sysdeps/unix/sysv/linux/Makefile
> index f83f147ed1..193b7c46b9 100644
> --- a/sysdeps/unix/sysv/linux/Makefile
> +++ b/sysdeps/unix/sysv/linux/Makefile
> @@ -101,7 +101,8 @@ sysdep_headers += sys/mount.h sys/acct.h \
> bits/types/struct_shmid_ds.h \
> bits/ipc-perm.h \
> bits/struct_stat.h \
> - bits/struct_stat_time64_helper.h
> + bits/struct_stat_time64_helper.h \
> + bits/types/struct_msqid64_ds_helper.h
OK. Create new helper.
>
> tests += tst-clone tst-clone2 tst-clone3 tst-fanotify tst-personality \
> tst-quota tst-sync_file_range tst-sysconf-iov_max tst-ttyname \
> diff --git a/sysdeps/unix/sysv/linux/bits/struct_stat_time64_helper.h b/sysdeps/unix/sysv/linux/bits/struct_stat_time64_helper.h
> index 77385e9073..b37056f5c4 100644
> --- a/sysdeps/unix/sysv/linux/bits/struct_stat_time64_helper.h
> +++ b/sysdeps/unix/sysv/linux/bits/struct_stat_time64_helper.h
> @@ -1,4 +1,4 @@
> -/* Definition for helper to define struct stat with 64 bit time.
> +/* Common defitions for struct stat with 64 bit time.
This is a spurious cleanup from the last patch IMO. If possible please move it
into the previous commit.
s/defitions/definitions/g
> Copyright (C) 2021 Free Software Foundation, Inc.
> This file is part of the GNU C Library.
>
> 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 43e8cd7cfc..992734914a 100644
> --- a/sysdeps/unix/sysv/linux/bits/types/struct_msqid64_ds.h
> +++ b/sysdeps/unix/sysv/linux/bits/types/struct_msqid64_ds.h
> @@ -25,14 +25,6 @@
> #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() */
> +# include <bits/types/struct_msqid64_ds_helper.h>
OK. Used unconditionally since this is 64-bit time_t.
> };
> #endif
> diff --git a/sysdeps/unix/sysv/linux/bits/types/struct_msqid64_ds_helper.h b/sysdeps/unix/sysv/linux/bits/types/struct_msqid64_ds_helper.h
> new file mode 100644
> index 0000000000..02dfddaa2b
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/bits/types/struct_msqid64_ds_helper.h
> @@ -0,0 +1,28 @@
> +/* Common defintions for struct msqid_ds with 64 bit time.
s/defintions/definitions/g
> + Copyright (C) 2021 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/>. */
> +
Reviewing...
> + /* Content of internal __msqid64_ds. */
> + struct ipc_perm msg_perm; /* structure describing operation permission */
OK.
> + __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 */
OK. Usually a tuple of * and *_high (endian swapping for order applied).
> + __syscall_ulong_t __msg_cbytes; /* current number of bytes on queue */
OK.
> + msgqnum_t msg_qnum; /* number of messages currently on queue */
OK.
> + msglen_t msg_qbytes; /* max number of bytes allowed on queue */
OK.
> + __pid_t msg_lspid; /* pid of last msgsnd() */
> + __pid_t msg_lrpid; /* pid of last msgrcv() */
OK.
Add __glibc_reserved4/5 entries here.
> diff --git a/sysdeps/unix/sysv/linux/bits/types/struct_msqid_ds.h b/sysdeps/unix/sysv/linux/bits/types/struct_msqid_ds.h
> index 1ed041ae30..ae10a48452 100644
> --- a/sysdeps/unix/sysv/linux/bits/types/struct_msqid_ds.h
> +++ b/sysdeps/unix/sysv/linux/bits/types/struct_msqid_ds.h
> @@ -20,23 +20,28 @@
> # error "Never use <bits/msq.h> directly; include <sys/msg.h> instead."
> #endif
>
> +#include <bits/types/time_t.h>
> +
> /* Structure of record for one message inside the kernel.
> The type `struct msg' is opaque. */
> struct msqid_ds
> {
> +#ifdef __USE_TIME_BITS64
> +# include <bits/types/struct_msqid64_ds_helper.h>
OK. Use helper.
> +#else
> struct ipc_perm msg_perm; /* structure describing operation permission */
> -#if __TIMESIZE == 32
> +# 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
> +# 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
> +# 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 */
> @@ -44,4 +49,5 @@ struct msqid_ds
> __pid_t msg_lrpid; /* pid of last msgrcv() */
> __syscall_ulong_t __glibc_reserved4;
> __syscall_ulong_t __glibc_reserved5;
> +#endif
> };
> diff --git a/sysdeps/unix/sysv/linux/hppa/bits/types/struct_msqid_ds.h b/sysdeps/unix/sysv/linux/hppa/bits/types/struct_msqid_ds.h
> index d943edeb78..5b82dd7f5e 100644
> --- a/sysdeps/unix/sysv/linux/hppa/bits/types/struct_msqid_ds.h
> +++ b/sysdeps/unix/sysv/linux/hppa/bits/types/struct_msqid_ds.h
> @@ -20,23 +20,28 @@
> # error "Never use <bits/msq.h> directly; include <sys/msg.h> instead."
> #endif
>
> +#include <bits/types/time_t.h>
> +
> /* Structure of record for one message inside the kernel.
> The type `struct msg' is opaque. */
> struct msqid_ds
> {
> +#ifdef __USE_TIME_BITS64
> +# include <bits/types/struct_msqid64_ds_helper.h>
OK. Use helper.
> +#else
> struct ipc_perm msg_perm; /* structure describing operation permission */
> -#if __TIMESIZE == 32
> +# if __TIMESIZE == 32
> unsigned long int __msg_stime_high;
> __time_t msg_stime; /* time of last msgsnd command */
> unsigned long int __msg_rtime_high;
> __time_t msg_rtime; /* time of last msgsnd command */
> unsigned long int __msg_ctime_high;
> __time_t msg_ctime; /* time of last change */
> -#else
> +# 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
> +# 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 */
> @@ -44,4 +49,5 @@ struct msqid_ds
> __pid_t msg_lrpid; /* pid of last msgrcv() */
> __syscall_ulong_t __glibc_reserved4;
> __syscall_ulong_t __glibc_reserved5;
> +#endif
> };
> diff --git a/sysdeps/unix/sysv/linux/mips/bits/types/struct_msqid_ds.h b/sysdeps/unix/sysv/linux/mips/bits/types/struct_msqid_ds.h
> index bdca5e5fe2..00c1804245 100644
> --- a/sysdeps/unix/sysv/linux/mips/bits/types/struct_msqid_ds.h
> +++ b/sysdeps/unix/sysv/linux/mips/bits/types/struct_msqid_ds.h
> @@ -20,32 +20,37 @@
> # error "Never use <bits/msq.h> directly; include <sys/msg.h> instead."
> #endif
>
> +#include <bits/types/time_t.h>
> +
> /* Structure of record for one message inside the kernel.
> The type `struct msg' is opaque. */
> struct msqid_ds
> {
> +#ifdef __USE_TIME_BITS64
> +# include <bits/types/struct_msqid64_ds_helper.h>
OK. Use helper.
> +#else
> struct ipc_perm msg_perm; /* structure describing operation permission */
> -#if __TIMESIZE == 32
> -# ifdef __MIPSEL__
> +# if __TIMESIZE == 32
> +# ifdef __MIPSEL__
> __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
> +# else
> unsigned long int __msg_stime_high;
> __time_t msg_stime; /* time of last msgsnd command */
> unsigned long int __msg_rtime_high;
> __time_t msg_rtime; /* time of last msgsnd command */
> unsigned long int __msg_ctime_high;
> __time_t msg_ctime; /* time of last change */
> -# endif
> -#else
> +# endif
> +# 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
> +# 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 */
> @@ -53,4 +58,5 @@ struct msqid_ds
> __pid_t msg_lrpid; /* pid of last msgrcv() */
> __syscall_ulong_t __glibc_reserved4;
> __syscall_ulong_t __glibc_reserved5;
> +#endif
> };
> diff --git a/sysdeps/unix/sysv/linux/powerpc/bits/types/struct_msqid_ds.h b/sysdeps/unix/sysv/linux/powerpc/bits/types/struct_msqid_ds.h
> index 72842ed747..8c296d2342 100644
> --- a/sysdeps/unix/sysv/linux/powerpc/bits/types/struct_msqid_ds.h
> +++ b/sysdeps/unix/sysv/linux/powerpc/bits/types/struct_msqid_ds.h
> @@ -20,23 +20,28 @@
> # error "Never use <bits/msq.h> directly; include <sys/msg.h> instead."
> #endif
>
> +#include <bits/types/time_t.h>
> +
> /* Structure of record for one message inside the kernel.
> The type `struct msg' is opaque. */
> struct msqid_ds
> {
> +#ifdef __USE_TIME_BITS64
> +# include <bits/types/struct_msqid64_ds_helper.h>
OK.
> +#else
> struct ipc_perm msg_perm; /* structure describing operation permission */
> -#if __TIMESIZE == 32
> +# if __TIMESIZE == 32
> unsigned long int __msg_stime_high;
> __time_t msg_stime; /* time of last msgsnd command */
> unsigned long int __msg_rtime_high;
> __time_t msg_rtime; /* time of last msgsnd command */
> unsigned long int __msg_ctime_high;
> __time_t msg_ctime; /* time of last change */
> -#else
> +# 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
> +# 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 */
> @@ -44,4 +49,5 @@ struct msqid_ds
> __pid_t msg_lrpid; /* pid of last msgrcv() */
> __syscall_ulong_t __glibc_reserved4;
> __syscall_ulong_t __glibc_reserved5;
> +#endif
> };
> diff --git a/sysdeps/unix/sysv/linux/sparc/bits/types/struct_msqid_ds.h b/sysdeps/unix/sysv/linux/sparc/bits/types/struct_msqid_ds.h
> index 22e1839de1..3c1b68ccc0 100644
> --- a/sysdeps/unix/sysv/linux/sparc/bits/types/struct_msqid_ds.h
> +++ b/sysdeps/unix/sysv/linux/sparc/bits/types/struct_msqid_ds.h
> @@ -20,23 +20,28 @@
> # error "Never use <bits/msq.h> directly; include <sys/msg.h> instead."
> #endif
>
> +#include <bits/types/time_t.h>
> +
> /* Structure of record for one message inside the kernel.
> The type `struct msg' is opaque. */
> struct msqid_ds
> {
> +#ifdef __USE_TIME_BITS64
> +# include <bits/types/struct_msqid64_ds_helper.h>
OK. Use helper.
> +#else
> struct ipc_perm msg_perm; /* structure describing operation permission */
> -#if __TIMESIZE == 32
> +# if __TIMESIZE == 32
> unsigned long int __msg_stime_high;
> __time_t msg_stime; /* time of last msgsnd command */
> unsigned long int __msg_rtime_high;
> __time_t msg_rtime; /* time of last msgsnd command */
> unsigned long int __msg_ctime_high;
> __time_t msg_ctime; /* time of last change */
> -#else
> +# 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
> +# 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 */
> @@ -44,4 +49,5 @@ struct msqid_ds
> __pid_t msg_lrpid; /* pid of last msgrcv() */
> __syscall_ulong_t __glibc_reserved4;
> __syscall_ulong_t __glibc_reserved5;
> +#endif
> };
>
--
Cheers,
Carlos.
More information about the Libc-alpha
mailing list