* [PATCH v3 5/7] y2038: Provide conversion helpers for struct __timeval64
2020-01-29 12:59 [PATCH v3 1/7] y2038: Define __suseconds64_t type to be used with struct __timeval64 Lukasz Majewski
@ 2020-01-29 12:59 ` Lukasz Majewski
2020-02-04 18:48 ` Adhemerval Zanella
2020-01-29 12:59 ` [PATCH v3 2/7] y2038: Introduce struct __timeval64 - new internal glibc type Lukasz Majewski
` (6 subsequent siblings)
7 siblings, 1 reply; 31+ messages in thread
From: Lukasz Majewski @ 2020-01-29 12:59 UTC (permalink / raw)
To: Joseph Myers, Paul Eggert, Adhemerval Zanella
Cc: Alistair Francis, Alistair Francis, GNU C Library,
Siddhesh Poyarekar, Florian Weimer, Florian Weimer,
Zack Weinberg, Carlos O'Donell, Andreas Schwab,
Lukasz Majewski
Those functions allow easy conversion between Y2038 safe, glibc internal
struct __timeval64 and other time related data structures (like struct timeval
or struct __timespec64).
Build tests:
./src/scripts/build-many-glibcs.py glibcs
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
---
Chanes for v3:
- Provide new helper function - valid_timeval64_to_timeval
Changes for v2:
- None
---
include/time.h | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 49 insertions(+)
diff --git a/include/time.h b/include/time.h
index 99492a1577..8617114052 100644
--- a/include/time.h
+++ b/include/time.h
@@ -304,6 +304,43 @@ valid_timeval_to_timespec64 (const struct timeval tv)
return ts64;
}
+/* Convert a known valid struct timeval into a struct __timeval64. */
+static inline struct __timeval64
+valid_timeval_to_timeval64 (const struct timeval tv)
+{
+ struct __timeval64 tv64;
+
+ tv64.tv_sec = tv.tv_sec;
+ tv64.tv_usec = tv.tv_usec;
+
+ return tv64;
+}
+
+/* Convert a valid and within range of struct timeval, struct
+ __timeval64 into a struct timeval. */
+static inline struct timeval
+valid_timeval64_to_timeval (const struct __timeval64 tv64)
+{
+ struct timeval tv;
+
+ tv.tv_sec = (time_t) tv64.tv_sec;
+ tv.tv_usec = (suseconds_t) tv64.tv_usec;
+
+ return tv;
+}
+
+/* Convert a struct __timeval64 into a struct __timespec64. */
+static inline struct __timespec64
+timeval64_to_timespec64 (const struct __timeval64 tv64)
+{
+ struct __timespec64 ts64;
+
+ ts64.tv_sec = tv64.tv_sec;
+ ts64.tv_nsec = tv64.tv_usec * 1000;
+
+ return ts64;
+}
+
/* Convert a known valid struct timespec into a struct __timespec64. */
static inline struct __timespec64
valid_timespec_to_timespec64 (const struct timespec ts)
@@ -342,6 +379,18 @@ valid_timespec64_to_timeval (const struct __timespec64 ts64)
return tv;
}
+/* Convert a struct __timespec64 into a struct __timeval64. */
+static inline struct __timeval64
+timespec64_to_timeval64 (const struct __timespec64 ts64)
+{
+ struct __timeval64 tv64;
+
+ tv64.tv_sec = ts64.tv_sec;
+ tv64.tv_usec = ts64.tv_nsec / 1000;
+
+ return tv64;
+}
+
/* Check if a value is in the valid nanoseconds range. Return true if
it is, false otherwise. */
static inline bool
--
2.20.1
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH v3 5/7] y2038: Provide conversion helpers for struct __timeval64
2020-01-29 12:59 ` [PATCH v3 5/7] y2038: Provide conversion helpers for " Lukasz Majewski
@ 2020-02-04 18:48 ` Adhemerval Zanella
0 siblings, 0 replies; 31+ messages in thread
From: Adhemerval Zanella @ 2020-02-04 18:48 UTC (permalink / raw)
To: Lukasz Majewski, Joseph Myers, Paul Eggert
Cc: Alistair Francis, Alistair Francis, GNU C Library,
Siddhesh Poyarekar, Florian Weimer, Florian Weimer,
Zack Weinberg, Carlos O'Donell, Andreas Schwab
On 29/01/2020 09:59, Lukasz Majewski wrote:
> Those functions allow easy conversion between Y2038 safe, glibc internal
> struct __timeval64 and other time related data structures (like struct timeval
> or struct __timespec64).
>
> Build tests:
> ./src/scripts/build-many-glibcs.py glibcs
>
> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
LGTM, thanks.
Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
>
> ---
> Chanes for v3:
> - Provide new helper function - valid_timeval64_to_timeval
>
> Changes for v2:
> - None
> ---
> include/time.h | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 49 insertions(+)
>
> diff --git a/include/time.h b/include/time.h
> index 99492a1577..8617114052 100644
> --- a/include/time.h
> +++ b/include/time.h
> @@ -304,6 +304,43 @@ valid_timeval_to_timespec64 (const struct timeval tv)
> return ts64;
> }
>
> +/* Convert a known valid struct timeval into a struct __timeval64. */
> +static inline struct __timeval64
> +valid_timeval_to_timeval64 (const struct timeval tv)
> +{
> + struct __timeval64 tv64;
> +
> + tv64.tv_sec = tv.tv_sec;
> + tv64.tv_usec = tv.tv_usec;
> +
> + return tv64;
> +}
> +
Ok.
> +/* Convert a valid and within range of struct timeval, struct
> + __timeval64 into a struct timeval. */
> +static inline struct timeval
> +valid_timeval64_to_timeval (const struct __timeval64 tv64)
> +{
> + struct timeval tv;
> +
> + tv.tv_sec = (time_t) tv64.tv_sec;
> + tv.tv_usec = (suseconds_t) tv64.tv_usec;
> +
> + return tv;
> +}
> +
Ok.
> +/* Convert a struct __timeval64 into a struct __timespec64. */
> +static inline struct __timespec64
> +timeval64_to_timespec64 (const struct __timeval64 tv64)
> +{
> + struct __timespec64 ts64;
> +
> + ts64.tv_sec = tv64.tv_sec;
> + ts64.tv_nsec = tv64.tv_usec * 1000;
> +
> + return ts64;
> +}
> +
Ok.
> /* Convert a known valid struct timespec into a struct __timespec64. */
> static inline struct __timespec64
> valid_timespec_to_timespec64 (const struct timespec ts)
> @@ -342,6 +379,18 @@ valid_timespec64_to_timeval (const struct __timespec64 ts64)
> return tv;
> }
>
> +/* Convert a struct __timespec64 into a struct __timeval64. */
> +static inline struct __timeval64
> +timespec64_to_timeval64 (const struct __timespec64 ts64)
> +{
> + struct __timeval64 tv64;
> +
> + tv64.tv_sec = ts64.tv_sec;
> + tv64.tv_usec = ts64.tv_nsec / 1000;
> +
> + return tv64;
> +}
> +
> /* Check if a value is in the valid nanoseconds range. Return true if
> it is, false otherwise. */
> static inline bool
>
Ok.
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH v3 2/7] y2038: Introduce struct __timeval64 - new internal glibc type
2020-01-29 12:59 [PATCH v3 1/7] y2038: Define __suseconds64_t type to be used with struct __timeval64 Lukasz Majewski
2020-01-29 12:59 ` [PATCH v3 5/7] y2038: Provide conversion helpers for " Lukasz Majewski
@ 2020-01-29 12:59 ` Lukasz Majewski
2020-01-30 18:01 ` Alistair Francis
2020-02-04 18:36 ` Adhemerval Zanella
2020-01-29 12:59 ` [PATCH v3 7/7] y2038: linux: Provide ___gettimeofday64 implementation Lukasz Majewski
` (5 subsequent siblings)
7 siblings, 2 replies; 31+ messages in thread
From: Lukasz Majewski @ 2020-01-29 12:59 UTC (permalink / raw)
To: Joseph Myers, Paul Eggert, Adhemerval Zanella
Cc: Alistair Francis, Alistair Francis, GNU C Library,
Siddhesh Poyarekar, Florian Weimer, Florian Weimer,
Zack Weinberg, Carlos O'Donell, Andreas Schwab,
Lukasz Majewski
This type is a glibc's "internal" type similar to struct timeval but
whose tv_sec field is a __time64_t rather than a time_t, which makes it
Y2038-proof. This struct is NOT supposed to be passed to the kernel -
instead it shall be converted to struct __timespec64 and clock_[sg]ettime
syscalls shall be used (which are now Y2038 safe).
Build tests:
./src/scripts/build-many-glibcs.py glibcs
---
Changes for v3:
- None
Changes for v2:
- Replace __suseconds_t with __suseconds64_t
---
include/time.h | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/include/time.h b/include/time.h
index 047f431a1a..99492a1577 100644
--- a/include/time.h
+++ b/include/time.h
@@ -93,6 +93,20 @@ struct __itimerspec64
};
#endif
+#if __TIMESIZE == 64
+# define __timeval64 timeval
+#else
+/* The glibc Y2038-proof struct __timeval64 structure for a time value.
+ This structure is NOT supposed to be passed to the Linux kernel.
+ Instead, it shall be converted to struct __timespec64 and time shall
+ be [sg]et via clock_[sg]ettime (which are now Y2038 safe). */
+struct __timeval64
+{
+ __time64_t tv_sec; /* Seconds */
+ __suseconds64_t tv_usec; /* Microseconds */
+};
+#endif
+
#if __TIMESIZE == 64
# define __ctime64 ctime
#else
--
2.20.1
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH v3 2/7] y2038: Introduce struct __timeval64 - new internal glibc type
2020-01-29 12:59 ` [PATCH v3 2/7] y2038: Introduce struct __timeval64 - new internal glibc type Lukasz Majewski
@ 2020-01-30 18:01 ` Alistair Francis
2020-02-04 18:36 ` Adhemerval Zanella
1 sibling, 0 replies; 31+ messages in thread
From: Alistair Francis @ 2020-01-30 18:01 UTC (permalink / raw)
To: Lukasz Majewski
Cc: Joseph Myers, Paul Eggert, Adhemerval Zanella, Alistair Francis,
GNU C Library, Siddhesh Poyarekar, Florian Weimer,
Florian Weimer, Zack Weinberg, Carlos O'Donell,
Andreas Schwab
On Wed, Jan 29, 2020 at 4:59 AM Lukasz Majewski <lukma@denx.de> wrote:
>
> This type is a glibc's "internal" type similar to struct timeval but
> whose tv_sec field is a __time64_t rather than a time_t, which makes it
> Y2038-proof. This struct is NOT supposed to be passed to the kernel -
> instead it shall be converted to struct __timespec64 and clock_[sg]ettime
> syscalls shall be used (which are now Y2038 safe).
>
> Build tests:
> ./src/scripts/build-many-glibcs.py glibcs
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Alistair
>
> ---
> Changes for v3:
> - None
>
> Changes for v2:
> - Replace __suseconds_t with __suseconds64_t
> ---
> include/time.h | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
>
> diff --git a/include/time.h b/include/time.h
> index 047f431a1a..99492a1577 100644
> --- a/include/time.h
> +++ b/include/time.h
> @@ -93,6 +93,20 @@ struct __itimerspec64
> };
> #endif
>
> +#if __TIMESIZE == 64
> +# define __timeval64 timeval
> +#else
> +/* The glibc Y2038-proof struct __timeval64 structure for a time value.
> + This structure is NOT supposed to be passed to the Linux kernel.
> + Instead, it shall be converted to struct __timespec64 and time shall
> + be [sg]et via clock_[sg]ettime (which are now Y2038 safe). */
> +struct __timeval64
> +{
> + __time64_t tv_sec; /* Seconds */
> + __suseconds64_t tv_usec; /* Microseconds */
> +};
> +#endif
> +
> #if __TIMESIZE == 64
> # define __ctime64 ctime
> #else
> --
> 2.20.1
>
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH v3 2/7] y2038: Introduce struct __timeval64 - new internal glibc type
2020-01-29 12:59 ` [PATCH v3 2/7] y2038: Introduce struct __timeval64 - new internal glibc type Lukasz Majewski
2020-01-30 18:01 ` Alistair Francis
@ 2020-02-04 18:36 ` Adhemerval Zanella
1 sibling, 0 replies; 31+ messages in thread
From: Adhemerval Zanella @ 2020-02-04 18:36 UTC (permalink / raw)
To: Lukasz Majewski, Joseph Myers, Paul Eggert
Cc: Alistair Francis, Alistair Francis, GNU C Library,
Siddhesh Poyarekar, Florian Weimer, Florian Weimer,
Zack Weinberg, Carlos O'Donell, Andreas Schwab
On 29/01/2020 09:59, Lukasz Majewski wrote:
> This type is a glibc's "internal" type similar to struct timeval but
> whose tv_sec field is a __time64_t rather than a time_t, which makes it
> Y2038-proof. This struct is NOT supposed to be passed to the kernel -
> instead it shall be converted to struct __timespec64 and clock_[sg]ettime
> syscalls shall be used (which are now Y2038 safe).
>
> Build tests:
> ./src/scripts/build-many-glibcs.py glibcs
LGTM, thanks.
Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
>
> ---
> Changes for v3:
> - None
>
> Changes for v2:
> - Replace __suseconds_t with __suseconds64_t
> ---
> include/time.h | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
>
> diff --git a/include/time.h b/include/time.h
> index 047f431a1a..99492a1577 100644
> --- a/include/time.h
> +++ b/include/time.h
> @@ -93,6 +93,20 @@ struct __itimerspec64
> };
> #endif
>
> +#if __TIMESIZE == 64
> +# define __timeval64 timeval
> +#else
> +/* The glibc Y2038-proof struct __timeval64 structure for a time value.
> + This structure is NOT supposed to be passed to the Linux kernel.
> + Instead, it shall be converted to struct __timespec64 and time shall
> + be [sg]et via clock_[sg]ettime (which are now Y2038 safe). */
> +struct __timeval64
> +{
> + __time64_t tv_sec; /* Seconds */
> + __suseconds64_t tv_usec; /* Microseconds */
> +};
> +#endif
> +
> #if __TIMESIZE == 64
> # define __ctime64 ctime
> #else
>
Ok.
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH v3 7/7] y2038: linux: Provide ___gettimeofday64 implementation
2020-01-29 12:59 [PATCH v3 1/7] y2038: Define __suseconds64_t type to be used with struct __timeval64 Lukasz Majewski
2020-01-29 12:59 ` [PATCH v3 5/7] y2038: Provide conversion helpers for " Lukasz Majewski
2020-01-29 12:59 ` [PATCH v3 2/7] y2038: Introduce struct __timeval64 - new internal glibc type Lukasz Majewski
@ 2020-01-29 12:59 ` Lukasz Majewski
2020-02-04 19:18 ` Adhemerval Zanella
2020-01-29 12:59 ` [PATCH v3 4/7] y2038: alpha: Rename valid_timeval64_to_timeval to valid_timeval_to_timeval32 Lukasz Majewski
` (4 subsequent siblings)
7 siblings, 1 reply; 31+ messages in thread
From: Lukasz Majewski @ 2020-01-29 12:59 UTC (permalink / raw)
To: Joseph Myers, Paul Eggert, Adhemerval Zanella
Cc: Alistair Francis, Alistair Francis, GNU C Library,
Siddhesh Poyarekar, Florian Weimer, Florian Weimer,
Zack Weinberg, Carlos O'Donell, Andreas Schwab,
Lukasz Majewski
In the glibc the gettimeofday can use vDSO (on power and x86 the
USE_IFUNC_GETTIMEOFDAY is defined), gettimeofday syscall or 'default'
___gettimeofday() from ./time/gettime.c (as a fallback).
In this patch the last function (___gettimeofday) has been reimplemented and
moved to ./sysdeps/unix/sysv/linux/gettimeofday.c to be Linux specific.
The new ___gettimeofday64 explicit 64 bit function for getting 64 bit time from
the kernel (by internally calling __clock_gettime64) has been introduced.
Moreover, a 32 bit version - ___gettimeofday has been refactored to internally
use __gettimeofday64.
The ___gettimeofday is now supposed to be used on systems still supporting 32
bit time (__TIMESIZE != 64) - hence the necessary check for time_t potential
overflow and conversion of struct __timeval64 to 32 bit struct timespec.
The alpha port is a bit problematic for this change - it supports 64 bit time
and can safely use gettimeofday implementation from ./time/gettimeofday.c as it
has ./sysdeps/unix/sysv/linux/alpha/gettimeofday.c, which includes
./time/gettimeofday.c, so the Linux specific one can be avoided.
For that reason the code to set default gettimeofday symbol has not been moved
to ./sysdeps/unix/sysv/linux/gettimeofday.c as only alpha defines
VERSION_gettimeofday.
Build tests:
./src/scripts/build-many-glibcs.py glibcs
Run-time tests:
- Run specific tests on ARM/x86 32bit systems (qemu):
https://github.com/lmajewski/meta-y2038 and run tests:
https://github.com/lmajewski/y2038-tests/commits/master
Above tests were performed with Y2038 redirection applied as well as without
to test proper usage of both ___gettimeofday64 and __gettimeofday.
---
Changes for v3:
- New patch
---
include/time.h | 4 +++
sysdeps/unix/sysv/linux/gettimeofday.c | 46 +++++++++++++++++++++++++-
2 files changed, 49 insertions(+), 1 deletion(-)
diff --git a/include/time.h b/include/time.h
index 0345803db2..931c9a3bd7 100644
--- a/include/time.h
+++ b/include/time.h
@@ -227,10 +227,14 @@ libc_hidden_proto (__sched_rr_get_interval64);
#if __TIMESIZE == 64
# define __settimeofday64 __settimeofday
+# define ___gettimeofday64 ___gettimeofday
#else
extern int __settimeofday64 (const struct __timeval64 *tv,
const struct timezone *tz);
libc_hidden_proto (__settimeofday64)
+extern int ___gettimeofday64 (struct __timeval64 *restrict tv,
+ void *restrict tz);
+libc_hidden_proto (___gettimeofday64)
#endif
/* Compute the `struct tm' representation of T,
diff --git a/sysdeps/unix/sysv/linux/gettimeofday.c b/sysdeps/unix/sysv/linux/gettimeofday.c
index d5cdb22495..728ef45eed 100644
--- a/sysdeps/unix/sysv/linux/gettimeofday.c
+++ b/sysdeps/unix/sysv/linux/gettimeofday.c
@@ -54,5 +54,49 @@ __gettimeofday (struct timeval *restrict tv, void *restrict tz)
# endif
weak_alias (__gettimeofday, gettimeofday)
#else /* USE_IFUNC_GETTIMEOFDAY */
-# include <time/gettimeofday.c>
+/* Conversion of gettimeofday function to support 64 bit time on archs
+ with __WORDSIZE == 32 and __TIMESIZE == 32/64 */
+#include <errno.h>
+
+int
+___gettimeofday64 (struct __timeval64 *restrict tv, void *restrict tz)
+{
+ if (__glibc_unlikely (tz != 0))
+ memset (tz, 0, sizeof (struct timezone));
+
+ struct __timespec64 ts64;
+ int ret = __clock_gettime64 (CLOCK_REALTIME, &ts64);
+
+ if (ret == 0 && tv)
+ *tv = timespec64_to_timeval64 (ts64);
+
+ return ret;
+}
+
+# if __TIMESIZE != 64
+libc_hidden_def (___gettimeofday64)
+
+int
+___gettimeofday (struct timeval *restrict tv, void *restrict tz)
+{
+ struct __timeval64 tv64;
+ int ret = ___gettimeofday64 (&tv64, tz);
+
+ if (ret == 0)
+ {
+ if (! in_time_t_range (tv64.tv_sec))
+ {
+ __set_errno (EOVERFLOW);
+ return -1;
+ }
+
+ if (tv)
+ *tv = valid_timeval64_to_timeval (tv64);
+ }
+
+ return ret;
+}
+# endif
+strong_alias (___gettimeofday, __gettimeofday)
+weak_alias (___gettimeofday, gettimeofday)
#endif
--
2.20.1
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH v3 7/7] y2038: linux: Provide ___gettimeofday64 implementation
2020-01-29 12:59 ` [PATCH v3 7/7] y2038: linux: Provide ___gettimeofday64 implementation Lukasz Majewski
@ 2020-02-04 19:18 ` Adhemerval Zanella
2020-02-05 0:06 ` Lukasz Majewski
0 siblings, 1 reply; 31+ messages in thread
From: Adhemerval Zanella @ 2020-02-04 19:18 UTC (permalink / raw)
To: Lukasz Majewski, Joseph Myers, Paul Eggert
Cc: Alistair Francis, Alistair Francis, GNU C Library,
Siddhesh Poyarekar, Florian Weimer, Florian Weimer,
Zack Weinberg, Carlos O'Donell, Andreas Schwab
On 29/01/2020 09:59, Lukasz Majewski wrote:
> In the glibc the gettimeofday can use vDSO (on power and x86 the
> USE_IFUNC_GETTIMEOFDAY is defined), gettimeofday syscall or 'default'
> ___gettimeofday() from ./time/gettime.c (as a fallback).
>
> In this patch the last function (___gettimeofday) has been reimplemented and
> moved to ./sysdeps/unix/sysv/linux/gettimeofday.c to be Linux specific.
>
> The new ___gettimeofday64 explicit 64 bit function for getting 64 bit time from
> the kernel (by internally calling __clock_gettime64) has been introduced.
>
> Moreover, a 32 bit version - ___gettimeofday has been refactored to internally
> use __gettimeofday64.
>
> The ___gettimeofday is now supposed to be used on systems still supporting 32
> bit time (__TIMESIZE != 64) - hence the necessary check for time_t potential
> overflow and conversion of struct __timeval64 to 32 bit struct timespec.
>
> The alpha port is a bit problematic for this change - it supports 64 bit time
> and can safely use gettimeofday implementation from ./time/gettimeofday.c as it
> has ./sysdeps/unix/sysv/linux/alpha/gettimeofday.c, which includes
> ./time/gettimeofday.c, so the Linux specific one can be avoided.
> For that reason the code to set default gettimeofday symbol has not been moved
> to ./sysdeps/unix/sysv/linux/gettimeofday.c as only alpha defines
> VERSION_gettimeofday.
>
>
> Build tests:
> ./src/scripts/build-many-glibcs.py glibcs
>
> Run-time tests:
> - Run specific tests on ARM/x86 32bit systems (qemu):
> https://github.com/lmajewski/meta-y2038 and run tests:
> https://github.com/lmajewski/y2038-tests/commits/master
>
> Above tests were performed with Y2038 redirection applied as well as without
> to test proper usage of both ___gettimeofday64 and __gettimeofday.
>
> ---
> Changes for v3:
> - New patch
> ---
> include/time.h | 4 +++
> sysdeps/unix/sysv/linux/gettimeofday.c | 46 +++++++++++++++++++++++++-
> 2 files changed, 49 insertions(+), 1 deletion(-)
>
> diff --git a/include/time.h b/include/time.h
> index 0345803db2..931c9a3bd7 100644
> --- a/include/time.h
> +++ b/include/time.h
> @@ -227,10 +227,14 @@ libc_hidden_proto (__sched_rr_get_interval64);
>
> #if __TIMESIZE == 64
> # define __settimeofday64 __settimeofday
> +# define ___gettimeofday64 ___gettimeofday
> #else
> extern int __settimeofday64 (const struct __timeval64 *tv,
> const struct timezone *tz);
> libc_hidden_proto (__settimeofday64)
> +extern int ___gettimeofday64 (struct __timeval64 *restrict tv,
> + void *restrict tz);
> +libc_hidden_proto (___gettimeofday64)
> #endif
>
> /* Compute the `struct tm' representation of T,
Why ___gettimeofday64 and not __gettimeofday?
> diff --git a/sysdeps/unix/sysv/linux/gettimeofday.c b/sysdeps/unix/sysv/linux/gettimeofday.c
> index d5cdb22495..728ef45eed 100644
> --- a/sysdeps/unix/sysv/linux/gettimeofday.c
> +++ b/sysdeps/unix/sysv/linux/gettimeofday.c
> @@ -54,5 +54,49 @@ __gettimeofday (struct timeval *restrict tv, void *restrict tz)
> # endif
> weak_alias (__gettimeofday, gettimeofday)
We still need to handle 32-bit architecture that define USE_IFUNC_GETTIMEOFDAY,
currently i686 and powerpc.
We can either:
1. Define the INLINE_VSYSCALL __gettimeofday for USE_IFUNC_GETTIMEOFDAY,
or ___gettimeofday that calls __clock_gettime64 otherwise. The
__gettimeofday64 will call either of this implementations.
2. Do not define USE_IFUNC_GETTIMEOFDAY for i686 and powerpc32, and add
a _Static_assert (__TIMESIZE == 64) within iFUNC definition.
I am more inclined to 2., it simplifies the somewhat macro convolution and
such optimization most likely won't make much difference for the specific
platforms. Thoughts?
> #else /* USE_IFUNC_GETTIMEOFDAY */
> -# include <time/gettimeofday.c>
> +/* Conversion of gettimeofday function to support 64 bit time on archs
> + with __WORDSIZE == 32 and __TIMESIZE == 32/64 */
> +#include <errno.h>
> +
> +int
> +___gettimeofday64 (struct __timeval64 *restrict tv, void *restrict tz)
> +{
> + if (__glibc_unlikely (tz != 0))
> + memset (tz, 0, sizeof (struct timezone));
> +
> + struct __timespec64 ts64;
> + int ret = __clock_gettime64 (CLOCK_REALTIME, &ts64);
> +
> + if (ret == 0 && tv)
> + *tv = timespec64_to_timeval64 (ts64);
No implicit checks. Also, we already set 'tv' with nonull attribute, so I
am not sure if it is worth to add an extra check for 'tv' validity
(specially because users tend to expect low latency for the symbol).
In any case, if the idea is to add such check as QoI I think it would
be better to do a early bail before actually issue __clock_gettime64.
> +
> + return ret;
> +}
Ok (no 64-bit gettimeofday on any architecture).
> +
> +# if __TIMESIZE != 64
> +libc_hidden_def (___gettimeofday64)
> +
> +int
> +___gettimeofday (struct timeval *restrict tv, void *restrict tz)
> +{
> + struct __timeval64 tv64;
> + int ret = ___gettimeofday64 (&tv64, tz);
> +
> + if (ret == 0)
> + {
> + if (! in_time_t_range (tv64.tv_sec))
> + {
> + __set_errno (EOVERFLOW);
> + return -1;
> + }
> +
> + if (tv)
> + *tv = valid_timeval64_to_timeval (tv64);
> + }
> +
> + return ret;
> +}
> +# endif
> +strong_alias (___gettimeofday, __gettimeofday)
I am failing to see the need of this alias.
> +weak_alias (___gettimeofday, gettimeofday)
> #endif
>
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH v3 7/7] y2038: linux: Provide ___gettimeofday64 implementation
2020-02-04 19:18 ` Adhemerval Zanella
@ 2020-02-05 0:06 ` Lukasz Majewski
2020-02-05 16:23 ` Adhemerval Zanella
2020-02-08 22:16 ` Lukasz Majewski
0 siblings, 2 replies; 31+ messages in thread
From: Lukasz Majewski @ 2020-02-05 0:06 UTC (permalink / raw)
To: Adhemerval Zanella
Cc: Joseph Myers, Paul Eggert, Alistair Francis, Alistair Francis,
GNU C Library, Siddhesh Poyarekar, Florian Weimer,
Florian Weimer, Zack Weinberg, Carlos O'Donell,
Andreas Schwab
[-- Attachment #1: Type: text/plain, Size: 7566 bytes --]
Hi Adhemerval,
> On 29/01/2020 09:59, Lukasz Majewski wrote:
> > In the glibc the gettimeofday can use vDSO (on power and x86 the
> > USE_IFUNC_GETTIMEOFDAY is defined), gettimeofday syscall or
> > 'default' ___gettimeofday() from ./time/gettime.c (as a fallback).
> >
> > In this patch the last function (___gettimeofday) has been
> > reimplemented and moved to ./sysdeps/unix/sysv/linux/gettimeofday.c
> > to be Linux specific.
> >
> > The new ___gettimeofday64 explicit 64 bit function for getting 64
> > bit time from the kernel (by internally calling __clock_gettime64)
> > has been introduced.
> >
> > Moreover, a 32 bit version - ___gettimeofday has been refactored to
> > internally use __gettimeofday64.
> >
> > The ___gettimeofday is now supposed to be used on systems still
> > supporting 32 bit time (__TIMESIZE != 64) - hence the necessary
> > check for time_t potential overflow and conversion of struct
> > __timeval64 to 32 bit struct timespec.
> >
> > The alpha port is a bit problematic for this change - it supports
> > 64 bit time and can safely use gettimeofday implementation from
> > ./time/gettimeofday.c as it has
> > ./sysdeps/unix/sysv/linux/alpha/gettimeofday.c, which includes
> > ./time/gettimeofday.c, so the Linux specific one can be avoided.
> > For that reason the code to set default gettimeofday symbol has not
> > been moved to ./sysdeps/unix/sysv/linux/gettimeofday.c as only
> > alpha defines VERSION_gettimeofday.
> >
> >
> > Build tests:
> > ./src/scripts/build-many-glibcs.py glibcs
> >
> > Run-time tests:
> > - Run specific tests on ARM/x86 32bit systems (qemu):
> > https://github.com/lmajewski/meta-y2038 and run tests:
> > https://github.com/lmajewski/y2038-tests/commits/master
> >
> > Above tests were performed with Y2038 redirection applied as well
> > as without to test proper usage of both ___gettimeofday64 and
> > __gettimeofday.
> >
> > ---
> > Changes for v3:
> > - New patch
> > ---
> > include/time.h | 4 +++
> > sysdeps/unix/sysv/linux/gettimeofday.c | 46
> > +++++++++++++++++++++++++- 2 files changed, 49 insertions(+), 1
> > deletion(-)
> >
> > diff --git a/include/time.h b/include/time.h
> > index 0345803db2..931c9a3bd7 100644
> > --- a/include/time.h
> > +++ b/include/time.h
> > @@ -227,10 +227,14 @@ libc_hidden_proto (__sched_rr_get_interval64);
> >
> > #if __TIMESIZE == 64
> > # define __settimeofday64 __settimeofday
> > +# define ___gettimeofday64 ___gettimeofday
> > #else
> > extern int __settimeofday64 (const struct __timeval64 *tv,
> > const struct timezone *tz);
> > libc_hidden_proto (__settimeofday64)
> > +extern int ___gettimeofday64 (struct __timeval64 *restrict tv,
> > + void *restrict tz);
> > +libc_hidden_proto (___gettimeofday64)
> > #endif
> >
> > /* Compute the `struct tm' representation of T,
>
> Why ___gettimeofday64 and not __gettimeofday?
Unfortunately, the __gettimeofday was already used in some symbol
aliasing in ./time/gettimeofday.c for alpha.
To avoid any potential name clashing (when using both symbols -
__gettimeofday/__gettimeofday64 with aliasing), I've decided to
introduce new ones - namely ___gettimeofday/___gettimeofday64.
Or maybe I'm wrong here?
>
> > diff --git a/sysdeps/unix/sysv/linux/gettimeofday.c
> > b/sysdeps/unix/sysv/linux/gettimeofday.c index
> > d5cdb22495..728ef45eed 100644 ---
> > a/sysdeps/unix/sysv/linux/gettimeofday.c +++
> > b/sysdeps/unix/sysv/linux/gettimeofday.c @@ -54,5 +54,49 @@
> > __gettimeofday (struct timeval *restrict tv, void *restrict tz) #
> > endif weak_alias (__gettimeofday, gettimeofday)
>
> We still need to handle 32-bit architecture that define
> USE_IFUNC_GETTIMEOFDAY, currently i686 and powerpc.
By "handle" do you mean to make them Y2038 safe? The code in this patch
target archs, which do not have vDSO support for gettimeofday (like
arm32).
>
> We can either:
>
> 1. Define the INLINE_VSYSCALL __gettimeofday for
> USE_IFUNC_GETTIMEOFDAY, or ___gettimeofday that calls
> __clock_gettime64 otherwise. The __gettimeofday64 will call either
> of this implementations.
>
> 2. Do not define USE_IFUNC_GETTIMEOFDAY for i686 and powerpc32, and
> add a _Static_assert (__TIMESIZE == 64) within iFUNC definition.
Ok, so we do need a fallback for vDSO gettimeofday. I will poke around
and try to add this in v2 (but I'm only runtime testing Y2039 on ARM32).
>
> I am more inclined to 2., it simplifies the somewhat macro
> convolution and such optimization most likely won't make much
> difference for the specific platforms. Thoughts?
>
> > #else /* USE_IFUNC_GETTIMEOFDAY */
> > -# include <time/gettimeofday.c>
> > +/* Conversion of gettimeofday function to support 64 bit time on
> > archs
> > + with __WORDSIZE == 32 and __TIMESIZE == 32/64 */
> > +#include <errno.h>
> > +
> > +int
> > +___gettimeofday64 (struct __timeval64 *restrict tv, void *restrict
> > tz) +{
> > + if (__glibc_unlikely (tz != 0))
> > + memset (tz, 0, sizeof (struct timezone));
> > +
> > + struct __timespec64 ts64;
> > + int ret = __clock_gettime64 (CLOCK_REALTIME, &ts64);
> > +
> > + if (ret == 0 && tv)
> > + *tv = timespec64_to_timeval64 (ts64);
>
> No implicit checks. Also, we already set 'tv' with nonull attribute,
> so I am not sure if it is worth to add an extra check for 'tv'
> validity (specially because users tend to expect low latency for the
> symbol).
>
> In any case, if the idea is to add such check as QoI I think it would
> be better to do a early bail before actually issue __clock_gettime64.
No, this was just my mistake. There was a discussion with Paul and
Joseph earlier. We shall _only_ check for NULL when it is required by
syscalls/command documentation. This is the case for e.g. setitimer's
*old_value pointer.
In other examples we just allow segfault when dereference the NULL
pointer.
I will fix it in v2 of this patch series.
>
> > +
> > + return ret;
> > +}
>
> Ok (no 64-bit gettimeofday on any architecture).
>
> > +
> > +# if __TIMESIZE != 64
> > +libc_hidden_def (___gettimeofday64)
> > +
> > +int
> > +___gettimeofday (struct timeval *restrict tv, void *restrict tz)
> > +{
> > + struct __timeval64 tv64;
> > + int ret = ___gettimeofday64 (&tv64, tz);
> > +
> > + if (ret == 0)
> > + {
> > + if (! in_time_t_range (tv64.tv_sec))
> > + {
> > + __set_errno (EOVERFLOW);
> > + return -1;
> > + }
> > +
> > + if (tv)
> > + *tv = valid_timeval64_to_timeval (tv64);
> > + }
> > +
> > + return ret;
> > +}
> > +# endif
> > +strong_alias (___gettimeofday, __gettimeofday)
>
> I am failing to see the need of this alias.
I've followed the idiom from ./time/gettimeofday.c
The strong alias will bind this local definition of ___gettimeofday to
glibc's internal __gettimeofday (used internally by glibc).
In the case where gettimeofday exported function is not defined, the
___gettimeofday will be a weak alias for it.
>
> > +weak_alias (___gettimeofday, gettimeofday)
> > #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 --]
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH v3 7/7] y2038: linux: Provide ___gettimeofday64 implementation
2020-02-05 0:06 ` Lukasz Majewski
@ 2020-02-05 16:23 ` Adhemerval Zanella
2020-02-05 16:58 ` Lukasz Majewski
2020-02-08 22:16 ` Lukasz Majewski
1 sibling, 1 reply; 31+ messages in thread
From: Adhemerval Zanella @ 2020-02-05 16:23 UTC (permalink / raw)
To: Lukasz Majewski
Cc: Joseph Myers, Paul Eggert, Alistair Francis, Alistair Francis,
GNU C Library, Siddhesh Poyarekar, Florian Weimer,
Florian Weimer, Zack Weinberg, Carlos O'Donell,
Andreas Schwab
On 04/02/2020 21:05, Lukasz Majewski wrote:
> Hi Adhemerval,
>
>> On 29/01/2020 09:59, Lukasz Majewski wrote:
>>> In the glibc the gettimeofday can use vDSO (on power and x86 the
>>> USE_IFUNC_GETTIMEOFDAY is defined), gettimeofday syscall or
>>> 'default' ___gettimeofday() from ./time/gettime.c (as a fallback).
>>>
>>> In this patch the last function (___gettimeofday) has been
>>> reimplemented and moved to ./sysdeps/unix/sysv/linux/gettimeofday.c
>>> to be Linux specific.
>>>
>>> The new ___gettimeofday64 explicit 64 bit function for getting 64
>>> bit time from the kernel (by internally calling __clock_gettime64)
>>> has been introduced.
>>>
>>> Moreover, a 32 bit version - ___gettimeofday has been refactored to
>>> internally use __gettimeofday64.
>>>
>>> The ___gettimeofday is now supposed to be used on systems still
>>> supporting 32 bit time (__TIMESIZE != 64) - hence the necessary
>>> check for time_t potential overflow and conversion of struct
>>> __timeval64 to 32 bit struct timespec.
>>>
>>> The alpha port is a bit problematic for this change - it supports
>>> 64 bit time and can safely use gettimeofday implementation from
>>> ./time/gettimeofday.c as it has
>>> ./sysdeps/unix/sysv/linux/alpha/gettimeofday.c, which includes
>>> ./time/gettimeofday.c, so the Linux specific one can be avoided.
>>> For that reason the code to set default gettimeofday symbol has not
>>> been moved to ./sysdeps/unix/sysv/linux/gettimeofday.c as only
>>> alpha defines VERSION_gettimeofday.
>>>
>>>
>>> Build tests:
>>> ./src/scripts/build-many-glibcs.py glibcs
>>>
>>> Run-time tests:
>>> - Run specific tests on ARM/x86 32bit systems (qemu):
>>> https://github.com/lmajewski/meta-y2038 and run tests:
>>> https://github.com/lmajewski/y2038-tests/commits/master
>>>
>>> Above tests were performed with Y2038 redirection applied as well
>>> as without to test proper usage of both ___gettimeofday64 and
>>> __gettimeofday.
>>>
>>> ---
>>> Changes for v3:
>>> - New patch
>>> ---
>>> include/time.h | 4 +++
>>> sysdeps/unix/sysv/linux/gettimeofday.c | 46
>>> +++++++++++++++++++++++++- 2 files changed, 49 insertions(+), 1
>>> deletion(-)
>>>
>>> diff --git a/include/time.h b/include/time.h
>>> index 0345803db2..931c9a3bd7 100644
>>> --- a/include/time.h
>>> +++ b/include/time.h
>>> @@ -227,10 +227,14 @@ libc_hidden_proto (__sched_rr_get_interval64);
>>>
>>> #if __TIMESIZE == 64
>>> # define __settimeofday64 __settimeofday
>>> +# define ___gettimeofday64 ___gettimeofday
>>> #else
>>> extern int __settimeofday64 (const struct __timeval64 *tv,
>>> const struct timezone *tz);
>>> libc_hidden_proto (__settimeofday64)
>>> +extern int ___gettimeofday64 (struct __timeval64 *restrict tv,
>>> + void *restrict tz);
>>> +libc_hidden_proto (___gettimeofday64)
>>> #endif
>>>
>>> /* Compute the `struct tm' representation of T,
>>
>> Why ___gettimeofday64 and not __gettimeofday?
>
> Unfortunately, the __gettimeofday was already used in some symbol
> aliasing in ./time/gettimeofday.c for alpha.
>
> To avoid any potential name clashing (when using both symbols -
> __gettimeofday/__gettimeofday64 with aliasing), I've decided to
> introduce new ones - namely ___gettimeofday/___gettimeofday64.
>
> Or maybe I'm wrong here?
Not really wrong, but now that Linux implementation is not using
the generic implementation any longer the extra strong_alias is
redundant. The only ABI that requires versioning (alpha) includes
the generic implementation with a arch-specific implementation.
>
>>
>>> diff --git a/sysdeps/unix/sysv/linux/gettimeofday.c
>>> b/sysdeps/unix/sysv/linux/gettimeofday.c index
>>> d5cdb22495..728ef45eed 100644 ---
>>> a/sysdeps/unix/sysv/linux/gettimeofday.c +++
>>> b/sysdeps/unix/sysv/linux/gettimeofday.c @@ -54,5 +54,49 @@
>>> __gettimeofday (struct timeval *restrict tv, void *restrict tz) #
>>> endif weak_alias (__gettimeofday, gettimeofday)
>>
>> We still need to handle 32-bit architecture that define
>> USE_IFUNC_GETTIMEOFDAY, currently i686 and powerpc.
>
> By "handle" do you mean to make them Y2038 safe? The code in this patch
> target archs, which do not have vDSO support for gettimeofday (like
> arm32).
>
Yes, on powerpc32 and i686 (which defines USE_IFUNC_GETTIMEOFDAY) won't
use the y2038 safe implementation.
>>
>> We can either:
>>
>> 1. Define the INLINE_VSYSCALL __gettimeofday for
>> USE_IFUNC_GETTIMEOFDAY, or ___gettimeofday that calls
>> __clock_gettime64 otherwise. The __gettimeofday64 will call either
>> of this implementations.
>>
>> 2. Do not define USE_IFUNC_GETTIMEOFDAY for i686 and powerpc32, and
>> add a _Static_assert (__TIMESIZE == 64) within iFUNC definition.
>
> Ok, so we do need a fallback for vDSO gettimeofday. I will poke around
> and try to add this in v2 (but I'm only runtime testing Y2039 on ARM32).
I really don't think optimizing USE_IFUNC_GETTIMEOFDAY for architectures
which still support fallback time32 is worth. It will require much
more code complexity and it would result in more build permutations
(one for !__ASSUME_TIME64_SYSCALLS which calls the vDSO through iFUNC,
another one that calls the vDSO fallback and finally the one that call
clock_gettime). Also, on build with time64 support the vDSO will only
used as fallback.
I would say to just not define USE_IFUNC_GETTIMEOFDAY for i686 and
powerpc32 and make them follow the expected code path for y2038 safeness.
>
>>
>> I am more inclined to 2., it simplifies the somewhat macro
>> convolution and such optimization most likely won't make much
>> difference for the specific platforms. Thoughts?
>>
>>> #else /* USE_IFUNC_GETTIMEOFDAY */
>>> -# include <time/gettimeofday.c>
>>> +/* Conversion of gettimeofday function to support 64 bit time on
>>> archs
>>> + with __WORDSIZE == 32 and __TIMESIZE == 32/64 */
>>> +#include <errno.h>
>>> +
>>> +int
>>> +___gettimeofday64 (struct __timeval64 *restrict tv, void *restrict
>>> tz) +{
>>> + if (__glibc_unlikely (tz != 0))
>>> + memset (tz, 0, sizeof (struct timezone));
>>> +
>>> + struct __timespec64 ts64;
>>> + int ret = __clock_gettime64 (CLOCK_REALTIME, &ts64);
>>> +
>>> + if (ret == 0 && tv)
>>> + *tv = timespec64_to_timeval64 (ts64);
>>
>> No implicit checks. Also, we already set 'tv' with nonull attribute,
>> so I am not sure if it is worth to add an extra check for 'tv'
>> validity (specially because users tend to expect low latency for the
>> symbol).
>>
>> In any case, if the idea is to add such check as QoI I think it would
>> be better to do a early bail before actually issue __clock_gettime64.
>
> No, this was just my mistake. There was a discussion with Paul and
> Joseph earlier. We shall _only_ check for NULL when it is required by
> syscalls/command documentation. This is the case for e.g. setitimer's
> *old_value pointer.
>
> In other examples we just allow segfault when dereference the NULL
> pointer.
>
> I will fix it in v2 of this patch series.
Ack.
>
>>
>>> +
>>> + return ret;
>>> +}
>>
>> Ok (no 64-bit gettimeofday on any architecture).
>>
>>> +
>>> +# if __TIMESIZE != 64
>>> +libc_hidden_def (___gettimeofday64)
>>> +
>>> +int
>>> +___gettimeofday (struct timeval *restrict tv, void *restrict tz)
>>> +{
>>> + struct __timeval64 tv64;
>>> + int ret = ___gettimeofday64 (&tv64, tz);
>>> +
>>> + if (ret == 0)
>>> + {
>>> + if (! in_time_t_range (tv64.tv_sec))
>>> + {
>>> + __set_errno (EOVERFLOW);
>>> + return -1;
>>> + }
>>> +
>>> + if (tv)
>>> + *tv = valid_timeval64_to_timeval (tv64);
>>> + }
>>> +
>>> + return ret;
>>> +}
>>> +# endif
>>> +strong_alias (___gettimeofday, __gettimeofday)
>>
>> I am failing to see the need of this alias.
>
> I've followed the idiom from ./time/gettimeofday.c
>
> The strong alias will bind this local definition of ___gettimeofday to
> glibc's internal __gettimeofday (used internally by glibc).
>
> In the case where gettimeofday exported function is not defined, the
> ___gettimeofday will be a weak alias for it.
>
>
>>
>>> +weak_alias (___gettimeofday, gettimeofday)
>>> #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
>
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH v3 7/7] y2038: linux: Provide ___gettimeofday64 implementation
2020-02-05 16:23 ` Adhemerval Zanella
@ 2020-02-05 16:58 ` Lukasz Majewski
2020-02-05 17:49 ` Adhemerval Zanella
0 siblings, 1 reply; 31+ messages in thread
From: Lukasz Majewski @ 2020-02-05 16:58 UTC (permalink / raw)
To: Adhemerval Zanella
Cc: Joseph Myers, Paul Eggert, Alistair Francis, Alistair Francis,
GNU C Library, Siddhesh Poyarekar, Florian Weimer,
Florian Weimer, Zack Weinberg, Carlos O'Donell,
Andreas Schwab
[-- Attachment #1: Type: text/plain, Size: 10208 bytes --]
Hi Adhemerval,
> On 04/02/2020 21:05, Lukasz Majewski wrote:
> > Hi Adhemerval,
> >
> >> On 29/01/2020 09:59, Lukasz Majewski wrote:
> >>> In the glibc the gettimeofday can use vDSO (on power and x86 the
> >>> USE_IFUNC_GETTIMEOFDAY is defined), gettimeofday syscall or
> >>> 'default' ___gettimeofday() from ./time/gettime.c (as a fallback).
> >>>
> >>> In this patch the last function (___gettimeofday) has been
> >>> reimplemented and moved to
> >>> ./sysdeps/unix/sysv/linux/gettimeofday.c to be Linux specific.
> >>>
> >>> The new ___gettimeofday64 explicit 64 bit function for getting 64
> >>> bit time from the kernel (by internally calling __clock_gettime64)
> >>> has been introduced.
> >>>
> >>> Moreover, a 32 bit version - ___gettimeofday has been refactored
> >>> to internally use __gettimeofday64.
> >>>
> >>> The ___gettimeofday is now supposed to be used on systems still
> >>> supporting 32 bit time (__TIMESIZE != 64) - hence the necessary
> >>> check for time_t potential overflow and conversion of struct
> >>> __timeval64 to 32 bit struct timespec.
> >>>
> >>> The alpha port is a bit problematic for this change - it supports
> >>> 64 bit time and can safely use gettimeofday implementation from
> >>> ./time/gettimeofday.c as it has
> >>> ./sysdeps/unix/sysv/linux/alpha/gettimeofday.c, which includes
> >>> ./time/gettimeofday.c, so the Linux specific one can be avoided.
> >>> For that reason the code to set default gettimeofday symbol has
> >>> not been moved to ./sysdeps/unix/sysv/linux/gettimeofday.c as only
> >>> alpha defines VERSION_gettimeofday.
> >>>
> >>>
> >>> Build tests:
> >>> ./src/scripts/build-many-glibcs.py glibcs
> >>>
> >>> Run-time tests:
> >>> - Run specific tests on ARM/x86 32bit systems (qemu):
> >>> https://github.com/lmajewski/meta-y2038 and run tests:
> >>> https://github.com/lmajewski/y2038-tests/commits/master
> >>>
> >>> Above tests were performed with Y2038 redirection applied as well
> >>> as without to test proper usage of both ___gettimeofday64 and
> >>> __gettimeofday.
> >>>
> >>> ---
> >>> Changes for v3:
> >>> - New patch
> >>> ---
> >>> include/time.h | 4 +++
> >>> sysdeps/unix/sysv/linux/gettimeofday.c | 46
> >>> +++++++++++++++++++++++++- 2 files changed, 49 insertions(+), 1
> >>> deletion(-)
> >>>
> >>> diff --git a/include/time.h b/include/time.h
> >>> index 0345803db2..931c9a3bd7 100644
> >>> --- a/include/time.h
> >>> +++ b/include/time.h
> >>> @@ -227,10 +227,14 @@ libc_hidden_proto
> >>> (__sched_rr_get_interval64);
> >>> #if __TIMESIZE == 64
> >>> # define __settimeofday64 __settimeofday
> >>> +# define ___gettimeofday64 ___gettimeofday
> >>> #else
> >>> extern int __settimeofday64 (const struct __timeval64 *tv,
> >>> const struct timezone *tz);
> >>> libc_hidden_proto (__settimeofday64)
> >>> +extern int ___gettimeofday64 (struct __timeval64 *restrict tv,
> >>> + void *restrict tz);
> >>> +libc_hidden_proto (___gettimeofday64)
> >>> #endif
> >>>
> >>> /* Compute the `struct tm' representation of T,
> >>
> >> Why ___gettimeofday64 and not __gettimeofday?
> >
> > Unfortunately, the __gettimeofday was already used in some symbol
> > aliasing in ./time/gettimeofday.c for alpha.
> >
> > To avoid any potential name clashing (when using both symbols -
> > __gettimeofday/__gettimeofday64 with aliasing), I've decided to
> > introduce new ones - namely ___gettimeofday/___gettimeofday64.
> >
> > Or maybe I'm wrong here?
>
> Not really wrong, but now that Linux implementation is not using
> the generic implementation any longer the extra strong_alias is
> redundant.
The strong alias would be needed only if we would call __gettimeofday
internally in glibc. I've grep'ed the source and there were no
references to it.
Is that the rationale for removing the strong alias?
And why would we need the strong_alias(___gettimeofday, __gettimeofday)
in ./time/gettimeofday,c ?
In the same file the ___gettimeofday is aliased (as weak) to "final"
gettimeofday symbol.
> The only ABI that requires versioning (alpha) includes
> the generic implementation with a arch-specific implementation.
Yes.
>
> >
> >>
> >>> diff --git a/sysdeps/unix/sysv/linux/gettimeofday.c
> >>> b/sysdeps/unix/sysv/linux/gettimeofday.c index
> >>> d5cdb22495..728ef45eed 100644 ---
> >>> a/sysdeps/unix/sysv/linux/gettimeofday.c +++
> >>> b/sysdeps/unix/sysv/linux/gettimeofday.c @@ -54,5 +54,49 @@
> >>> __gettimeofday (struct timeval *restrict tv, void *restrict tz) #
> >>> endif weak_alias (__gettimeofday, gettimeofday)
> >>
> >> We still need to handle 32-bit architecture that define
> >> USE_IFUNC_GETTIMEOFDAY, currently i686 and powerpc.
> >
> > By "handle" do you mean to make them Y2038 safe? The code in this
> > patch target archs, which do not have vDSO support for gettimeofday
> > (like arm32).
> >
>
> Yes, on powerpc32 and i686 (which defines USE_IFUNC_GETTIMEOFDAY)
> won't use the y2038 safe implementation.
As the gettimeofday is going to be obsolete, why should we care (and
implement the fallback for this vDSO syscall)?
>
> >>
> >> We can either:
> >>
> >> 1. Define the INLINE_VSYSCALL __gettimeofday for
> >> USE_IFUNC_GETTIMEOFDAY, or ___gettimeofday that calls
> >> __clock_gettime64 otherwise. The __gettimeofday64 will call either
> >> of this implementations.
> >>
> >> 2. Do not define USE_IFUNC_GETTIMEOFDAY for i686 and powerpc32,
> >> and add a _Static_assert (__TIMESIZE == 64) within iFUNC
> >> definition.
> >
> > Ok, so we do need a fallback for vDSO gettimeofday. I will poke
> > around and try to add this in v2 (but I'm only runtime testing
> > Y2039 on ARM32).
>
> I really don't think optimizing USE_IFUNC_GETTIMEOFDAY for
> architectures which still support fallback time32 is worth. It will
> require much more code complexity and it would result in more build
> permutations (one for !__ASSUME_TIME64_SYSCALLS which calls the vDSO
> through iFUNC, another one that calls the vDSO fallback and finally
> the one that call clock_gettime). Also, on build with time64 support
> the vDSO will only used as fallback.
>
> I would say to just not define USE_IFUNC_GETTIMEOFDAY for i686 and
> powerpc32 and make them follow the expected code path for y2038
> safeness.
Shall this be added to this patch? Or shall I left the
USE_IFUNC_GETTIMEOFDAY untouched?
>
> >
> >>
> >> I am more inclined to 2., it simplifies the somewhat macro
> >> convolution and such optimization most likely won't make much
> >> difference for the specific platforms. Thoughts?
> >>
> >>> #else /* USE_IFUNC_GETTIMEOFDAY */
> >>> -# include <time/gettimeofday.c>
> >>> +/* Conversion of gettimeofday function to support 64 bit time on
> >>> archs
> >>> + with __WORDSIZE == 32 and __TIMESIZE == 32/64 */
> >>> +#include <errno.h>
> >>> +
> >>> +int
> >>> +___gettimeofday64 (struct __timeval64 *restrict tv, void
> >>> *restrict tz) +{
> >>> + if (__glibc_unlikely (tz != 0))
> >>> + memset (tz, 0, sizeof (struct timezone));
> >>> +
> >>> + struct __timespec64 ts64;
> >>> + int ret = __clock_gettime64 (CLOCK_REALTIME, &ts64);
> >>> +
> >>> + if (ret == 0 && tv)
> >>> + *tv = timespec64_to_timeval64 (ts64);
> >>
> >> No implicit checks. Also, we already set 'tv' with nonull
> >> attribute, so I am not sure if it is worth to add an extra check
> >> for 'tv' validity (specially because users tend to expect low
> >> latency for the symbol).
> >>
> >> In any case, if the idea is to add such check as QoI I think it
> >> would be better to do a early bail before actually issue
> >> __clock_gettime64.
> >
> > No, this was just my mistake. There was a discussion with Paul and
> > Joseph earlier. We shall _only_ check for NULL when it is required
> > by syscalls/command documentation. This is the case for e.g.
> > setitimer's *old_value pointer.
> >
> > In other examples we just allow segfault when dereference the NULL
> > pointer.
> >
> > I will fix it in v2 of this patch series.
>
> Ack.
>
> >
> >>
> >>> +
> >>> + return ret;
> >>> +}
> >>
> >> Ok (no 64-bit gettimeofday on any architecture).
> >>
> >>> +
> >>> +# if __TIMESIZE != 64
> >>> +libc_hidden_def (___gettimeofday64)
> >>> +
> >>> +int
> >>> +___gettimeofday (struct timeval *restrict tv, void *restrict tz)
> >>> +{
> >>> + struct __timeval64 tv64;
> >>> + int ret = ___gettimeofday64 (&tv64, tz);
> >>> +
> >>> + if (ret == 0)
> >>> + {
> >>> + if (! in_time_t_range (tv64.tv_sec))
> >>> + {
> >>> + __set_errno (EOVERFLOW);
> >>> + return -1;
> >>> + }
> >>> +
> >>> + if (tv)
> >>> + *tv = valid_timeval64_to_timeval (tv64);
> >>> + }
> >>> +
> >>> + return ret;
> >>> +}
> >>> +# endif
> >>> +strong_alias (___gettimeofday, __gettimeofday)
> >>
> >> I am failing to see the need of this alias.
> >
> > I've followed the idiom from ./time/gettimeofday.c
> >
> > The strong alias will bind this local definition of ___gettimeofday
> > to glibc's internal __gettimeofday (used internally by glibc).
> >
> > In the case where gettimeofday exported function is not defined, the
> > ___gettimeofday will be a weak alias for it.
> >
> >
> >>
> >>> +weak_alias (___gettimeofday, gettimeofday)
> >>> #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 --]
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH v3 7/7] y2038: linux: Provide ___gettimeofday64 implementation
2020-02-05 16:58 ` Lukasz Majewski
@ 2020-02-05 17:49 ` Adhemerval Zanella
2020-02-06 10:08 ` Lukasz Majewski
2020-02-06 21:41 ` Joseph Myers
0 siblings, 2 replies; 31+ messages in thread
From: Adhemerval Zanella @ 2020-02-05 17:49 UTC (permalink / raw)
To: Lukasz Majewski
Cc: Joseph Myers, Paul Eggert, Alistair Francis, Alistair Francis,
GNU C Library, Siddhesh Poyarekar, Florian Weimer,
Florian Weimer, Zack Weinberg, Carlos O'Donell,
Andreas Schwab
On 05/02/2020 13:58, Lukasz Majewski wrote:
> Hi Adhemerval,
>
>> On 04/02/2020 21:05, Lukasz Majewski wrote:
>>> Hi Adhemerval,
>>>
>>>> On 29/01/2020 09:59, Lukasz Majewski wrote:
>>>>> In the glibc the gettimeofday can use vDSO (on power and x86 the
>>>>> USE_IFUNC_GETTIMEOFDAY is defined), gettimeofday syscall or
>>>>> 'default' ___gettimeofday() from ./time/gettime.c (as a fallback).
>>>>>
>>>>> In this patch the last function (___gettimeofday) has been
>>>>> reimplemented and moved to
>>>>> ./sysdeps/unix/sysv/linux/gettimeofday.c to be Linux specific.
>>>>>
>>>>> The new ___gettimeofday64 explicit 64 bit function for getting 64
>>>>> bit time from the kernel (by internally calling __clock_gettime64)
>>>>> has been introduced.
>>>>>
>>>>> Moreover, a 32 bit version - ___gettimeofday has been refactored
>>>>> to internally use __gettimeofday64.
>>>>>
>>>>> The ___gettimeofday is now supposed to be used on systems still
>>>>> supporting 32 bit time (__TIMESIZE != 64) - hence the necessary
>>>>> check for time_t potential overflow and conversion of struct
>>>>> __timeval64 to 32 bit struct timespec.
>>>>>
>>>>> The alpha port is a bit problematic for this change - it supports
>>>>> 64 bit time and can safely use gettimeofday implementation from
>>>>> ./time/gettimeofday.c as it has
>>>>> ./sysdeps/unix/sysv/linux/alpha/gettimeofday.c, which includes
>>>>> ./time/gettimeofday.c, so the Linux specific one can be avoided.
>>>>> For that reason the code to set default gettimeofday symbol has
>>>>> not been moved to ./sysdeps/unix/sysv/linux/gettimeofday.c as only
>>>>> alpha defines VERSION_gettimeofday.
>>>>>
>>>>>
>>>>> Build tests:
>>>>> ./src/scripts/build-many-glibcs.py glibcs
>>>>>
>>>>> Run-time tests:
>>>>> - Run specific tests on ARM/x86 32bit systems (qemu):
>>>>> https://github.com/lmajewski/meta-y2038 and run tests:
>>>>> https://github.com/lmajewski/y2038-tests/commits/master
>>>>>
>>>>> Above tests were performed with Y2038 redirection applied as well
>>>>> as without to test proper usage of both ___gettimeofday64 and
>>>>> __gettimeofday.
>>>>>
>>>>> ---
>>>>> Changes for v3:
>>>>> - New patch
>>>>> ---
>>>>> include/time.h | 4 +++
>>>>> sysdeps/unix/sysv/linux/gettimeofday.c | 46
>>>>> +++++++++++++++++++++++++- 2 files changed, 49 insertions(+), 1
>>>>> deletion(-)
>>>>>
>>>>> diff --git a/include/time.h b/include/time.h
>>>>> index 0345803db2..931c9a3bd7 100644
>>>>> --- a/include/time.h
>>>>> +++ b/include/time.h
>>>>> @@ -227,10 +227,14 @@ libc_hidden_proto
>>>>> (__sched_rr_get_interval64);
>>>>> #if __TIMESIZE == 64
>>>>> # define __settimeofday64 __settimeofday
>>>>> +# define ___gettimeofday64 ___gettimeofday
>>>>> #else
>>>>> extern int __settimeofday64 (const struct __timeval64 *tv,
>>>>> const struct timezone *tz);
>>>>> libc_hidden_proto (__settimeofday64)
>>>>> +extern int ___gettimeofday64 (struct __timeval64 *restrict tv,
>>>>> + void *restrict tz);
>>>>> +libc_hidden_proto (___gettimeofday64)
>>>>> #endif
>>>>>
>>>>> /* Compute the `struct tm' representation of T,
>>>>
>>>> Why ___gettimeofday64 and not __gettimeofday?
>>>
>>> Unfortunately, the __gettimeofday was already used in some symbol
>>> aliasing in ./time/gettimeofday.c for alpha.
>>>
>>> To avoid any potential name clashing (when using both symbols -
>>> __gettimeofday/__gettimeofday64 with aliasing), I've decided to
>>> introduce new ones - namely ___gettimeofday/___gettimeofday64.
>>>
>>> Or maybe I'm wrong here?
>>
>> Not really wrong, but now that Linux implementation is not using
>> the generic implementation any longer the extra strong_alias is
>> redundant.
>
> The strong alias would be needed only if we would call __gettimeofday
> internally in glibc. I've grep'ed the source and there were no
> references to it.
> Is that the rationale for removing the strong alias?
The triple underscore on 'time/gettimeofday.c' is only an implementation
detail for this specific code (it could any other name). It was done
to tie a single gettimeofday implementation to two versioned symbols
(gettimeofday and __gettimeofday) and the triple underscore is a trick
to avoid a double symbol definition by the static linker.
However this specific code for alpha is not really required, since
04da832e16a7 it implements its gettimeofday version in its own
syscalls.list. So this code is not used anywhere (along with alpha
gettimeofday.c implementation) and I push a patch to remove it.
>
> And why would we need the strong_alias(___gettimeofday, __gettimeofday)
> in ./time/gettimeofday,c ?
> In the same file the ___gettimeofday is aliased (as weak) to "final"
> gettimeofday symbol.
>
>> The only ABI that requires versioning (alpha) includes
>> the generic implementation with a arch-specific implementation.
>
> Yes.
>
>>
>>>
>>>>
>>>>> diff --git a/sysdeps/unix/sysv/linux/gettimeofday.c
>>>>> b/sysdeps/unix/sysv/linux/gettimeofday.c index
>>>>> d5cdb22495..728ef45eed 100644 ---
>>>>> a/sysdeps/unix/sysv/linux/gettimeofday.c +++
>>>>> b/sysdeps/unix/sysv/linux/gettimeofday.c @@ -54,5 +54,49 @@
>>>>> __gettimeofday (struct timeval *restrict tv, void *restrict tz) #
>>>>> endif weak_alias (__gettimeofday, gettimeofday)
>>>>
>>>> We still need to handle 32-bit architecture that define
>>>> USE_IFUNC_GETTIMEOFDAY, currently i686 and powerpc.
>>>
>>> By "handle" do you mean to make them Y2038 safe? The code in this
>>> patch target archs, which do not have vDSO support for gettimeofday
>>> (like arm32).
>>>
>>
>> Yes, on powerpc32 and i686 (which defines USE_IFUNC_GETTIMEOFDAY)
>> won't use the y2038 safe implementation.
>
> As the gettimeofday is going to be obsolete, why should we care (and
> implement the fallback for this vDSO syscall)?
Although marked obsolescent by POSIX, we can't really removed since a
program might require to build against a specific POSIX version
(_POSIX_C_SOURCE).
Once POSIX does remove the symbol, we can remove its definition from
default visibility (usually by setting the latest POSIX as default
version) and add __attribute_deprecated__.
So we still need to adapt it to be y2038 proof on current release.
>
>>
>>>>
>>>> We can either:
>>>>
>>>> 1. Define the INLINE_VSYSCALL __gettimeofday for
>>>> USE_IFUNC_GETTIMEOFDAY, or ___gettimeofday that calls
>>>> __clock_gettime64 otherwise. The __gettimeofday64 will call either
>>>> of this implementations.
>>>>
>>>> 2. Do not define USE_IFUNC_GETTIMEOFDAY for i686 and powerpc32,
>>>> and add a _Static_assert (__TIMESIZE == 64) within iFUNC
>>>> definition.
>>>
>>> Ok, so we do need a fallback for vDSO gettimeofday. I will poke
>>> around and try to add this in v2 (but I'm only runtime testing
>>> Y2039 on ARM32).
>>
>> I really don't think optimizing USE_IFUNC_GETTIMEOFDAY for
>> architectures which still support fallback time32 is worth. It will
>> require much more code complexity and it would result in more build
>> permutations (one for !__ASSUME_TIME64_SYSCALLS which calls the vDSO
>> through iFUNC, another one that calls the vDSO fallback and finally
>> the one that call clock_gettime). Also, on build with time64 support
>> the vDSO will only used as fallback.
>>
>> I would say to just not define USE_IFUNC_GETTIMEOFDAY for i686 and
>> powerpc32 and make them follow the expected code path for y2038
>> safeness.
>
> Shall this be added to this patch? Or shall I left the
> USE_IFUNC_GETTIMEOFDAY untouched?
Since this patch aims to add time64 gettimeofday for all supported
Linux ABI, I think it should be along this patch.
>
>>
>>>
>>>>
>>>> I am more inclined to 2., it simplifies the somewhat macro
>>>> convolution and such optimization most likely won't make much
>>>> difference for the specific platforms. Thoughts?
>>>>
>>>>> #else /* USE_IFUNC_GETTIMEOFDAY */
>>>>> -# include <time/gettimeofday.c>
>>>>> +/* Conversion of gettimeofday function to support 64 bit time on
>>>>> archs
>>>>> + with __WORDSIZE == 32 and __TIMESIZE == 32/64 */
>>>>> +#include <errno.h>
>>>>> +
>>>>> +int
>>>>> +___gettimeofday64 (struct __timeval64 *restrict tv, void
>>>>> *restrict tz) +{
>>>>> + if (__glibc_unlikely (tz != 0))
>>>>> + memset (tz, 0, sizeof (struct timezone));
>>>>> +
>>>>> + struct __timespec64 ts64;
>>>>> + int ret = __clock_gettime64 (CLOCK_REALTIME, &ts64);
>>>>> +
>>>>> + if (ret == 0 && tv)
>>>>> + *tv = timespec64_to_timeval64 (ts64);
>>>>
>>>> No implicit checks. Also, we already set 'tv' with nonull
>>>> attribute, so I am not sure if it is worth to add an extra check
>>>> for 'tv' validity (specially because users tend to expect low
>>>> latency for the symbol).
>>>>
>>>> In any case, if the idea is to add such check as QoI I think it
>>>> would be better to do a early bail before actually issue
>>>> __clock_gettime64.
>>>
>>> No, this was just my mistake. There was a discussion with Paul and
>>> Joseph earlier. We shall _only_ check for NULL when it is required
>>> by syscalls/command documentation. This is the case for e.g.
>>> setitimer's *old_value pointer.
>>>
>>> In other examples we just allow segfault when dereference the NULL
>>> pointer.
>>>
>>> I will fix it in v2 of this patch series.
>>
>> Ack.
>>
>>>
>>>>
>>>>> +
>>>>> + return ret;
>>>>> +}
>>>>
>>>> Ok (no 64-bit gettimeofday on any architecture).
>>>>
>>>>> +
>>>>> +# if __TIMESIZE != 64
>>>>> +libc_hidden_def (___gettimeofday64)
>>>>> +
>>>>> +int
>>>>> +___gettimeofday (struct timeval *restrict tv, void *restrict tz)
>>>>> +{
>>>>> + struct __timeval64 tv64;
>>>>> + int ret = ___gettimeofday64 (&tv64, tz);
>>>>> +
>>>>> + if (ret == 0)
>>>>> + {
>>>>> + if (! in_time_t_range (tv64.tv_sec))
>>>>> + {
>>>>> + __set_errno (EOVERFLOW);
>>>>> + return -1;
>>>>> + }
>>>>> +
>>>>> + if (tv)
>>>>> + *tv = valid_timeval64_to_timeval (tv64);
>>>>> + }
>>>>> +
>>>>> + return ret;
>>>>> +}
>>>>> +# endif
>>>>> +strong_alias (___gettimeofday, __gettimeofday)
>>>>
>>>> I am failing to see the need of this alias.
>>>
>>> I've followed the idiom from ./time/gettimeofday.c
>>>
>>> The strong alias will bind this local definition of ___gettimeofday
>>> to glibc's internal __gettimeofday (used internally by glibc).
>>>
>>> In the case where gettimeofday exported function is not defined, the
>>> ___gettimeofday will be a weak alias for it.
>>>
>>>
>>>>
>>>>> +weak_alias (___gettimeofday, gettimeofday)
>>>>> #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
>
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH v3 7/7] y2038: linux: Provide ___gettimeofday64 implementation
2020-02-05 17:49 ` Adhemerval Zanella
@ 2020-02-06 10:08 ` Lukasz Majewski
2020-02-07 13:20 ` Adhemerval Zanella
2020-02-06 21:41 ` Joseph Myers
1 sibling, 1 reply; 31+ messages in thread
From: Lukasz Majewski @ 2020-02-06 10:08 UTC (permalink / raw)
To: Adhemerval Zanella
Cc: Joseph Myers, Paul Eggert, Alistair Francis, Alistair Francis,
GNU C Library, Siddhesh Poyarekar, Florian Weimer,
Florian Weimer, Zack Weinberg, Carlos O'Donell,
Andreas Schwab
[-- Attachment #1: Type: text/plain, Size: 13008 bytes --]
Hi Adhemerval,
> On 05/02/2020 13:58, Lukasz Majewski wrote:
> > Hi Adhemerval,
> >
> >> On 04/02/2020 21:05, Lukasz Majewski wrote:
> >>> Hi Adhemerval,
> >>>
> >>>> On 29/01/2020 09:59, Lukasz Majewski wrote:
> >>>>> In the glibc the gettimeofday can use vDSO (on power and x86 the
> >>>>> USE_IFUNC_GETTIMEOFDAY is defined), gettimeofday syscall or
> >>>>> 'default' ___gettimeofday() from ./time/gettime.c (as a
> >>>>> fallback).
> >>>>>
> >>>>> In this patch the last function (___gettimeofday) has been
> >>>>> reimplemented and moved to
> >>>>> ./sysdeps/unix/sysv/linux/gettimeofday.c to be Linux specific.
> >>>>>
> >>>>> The new ___gettimeofday64 explicit 64 bit function for getting
> >>>>> 64 bit time from the kernel (by internally calling
> >>>>> __clock_gettime64) has been introduced.
> >>>>>
> >>>>> Moreover, a 32 bit version - ___gettimeofday has been refactored
> >>>>> to internally use __gettimeofday64.
> >>>>>
> >>>>> The ___gettimeofday is now supposed to be used on systems still
> >>>>> supporting 32 bit time (__TIMESIZE != 64) - hence the necessary
> >>>>> check for time_t potential overflow and conversion of struct
> >>>>> __timeval64 to 32 bit struct timespec.
> >>>>>
> >>>>> The alpha port is a bit problematic for this change - it
> >>>>> supports 64 bit time and can safely use gettimeofday
> >>>>> implementation from ./time/gettimeofday.c as it has
> >>>>> ./sysdeps/unix/sysv/linux/alpha/gettimeofday.c, which includes
> >>>>> ./time/gettimeofday.c, so the Linux specific one can be avoided.
> >>>>> For that reason the code to set default gettimeofday symbol has
> >>>>> not been moved to ./sysdeps/unix/sysv/linux/gettimeofday.c as
> >>>>> only alpha defines VERSION_gettimeofday.
> >>>>>
> >>>>>
> >>>>> Build tests:
> >>>>> ./src/scripts/build-many-glibcs.py glibcs
> >>>>>
> >>>>> Run-time tests:
> >>>>> - Run specific tests on ARM/x86 32bit systems (qemu):
> >>>>> https://github.com/lmajewski/meta-y2038 and run tests:
> >>>>> https://github.com/lmajewski/y2038-tests/commits/master
> >>>>>
> >>>>> Above tests were performed with Y2038 redirection applied as
> >>>>> well as without to test proper usage of both ___gettimeofday64
> >>>>> and __gettimeofday.
> >>>>>
> >>>>> ---
> >>>>> Changes for v3:
> >>>>> - New patch
> >>>>> ---
> >>>>> include/time.h | 4 +++
> >>>>> sysdeps/unix/sysv/linux/gettimeofday.c | 46
> >>>>> +++++++++++++++++++++++++- 2 files changed, 49 insertions(+), 1
> >>>>> deletion(-)
> >>>>>
> >>>>> diff --git a/include/time.h b/include/time.h
> >>>>> index 0345803db2..931c9a3bd7 100644
> >>>>> --- a/include/time.h
> >>>>> +++ b/include/time.h
> >>>>> @@ -227,10 +227,14 @@ libc_hidden_proto
> >>>>> (__sched_rr_get_interval64);
> >>>>> #if __TIMESIZE == 64
> >>>>> # define __settimeofday64 __settimeofday
> >>>>> +# define ___gettimeofday64 ___gettimeofday
> >>>>> #else
> >>>>> extern int __settimeofday64 (const struct __timeval64 *tv,
> >>>>> const struct timezone *tz);
> >>>>> libc_hidden_proto (__settimeofday64)
> >>>>> +extern int ___gettimeofday64 (struct __timeval64 *restrict tv,
> >>>>> + void *restrict tz);
> >>>>> +libc_hidden_proto (___gettimeofday64)
> >>>>> #endif
> >>>>>
> >>>>> /* Compute the `struct tm' representation of T,
> >>>>
> >>>> Why ___gettimeofday64 and not __gettimeofday?
> >>>
> >>> Unfortunately, the __gettimeofday was already used in some symbol
> >>> aliasing in ./time/gettimeofday.c for alpha.
> >>>
> >>> To avoid any potential name clashing (when using both symbols -
> >>> __gettimeofday/__gettimeofday64 with aliasing), I've decided to
> >>> introduce new ones - namely ___gettimeofday/___gettimeofday64.
> >>>
> >>> Or maybe I'm wrong here?
> >>
> >> Not really wrong, but now that Linux implementation is not using
> >> the generic implementation any longer the extra strong_alias is
> >> redundant.
> >
> > The strong alias would be needed only if we would call
> > __gettimeofday internally in glibc. I've grep'ed the source and
> > there were no references to it.
> > Is that the rationale for removing the strong alias?
>
> The triple underscore on 'time/gettimeofday.c' is only an
> implementation detail for this specific code (it could any other
> name). It was done to tie a single gettimeofday implementation to
> two versioned symbols (gettimeofday and __gettimeofday) and the
> triple underscore is a trick to avoid a double symbol definition by
> the static linker.
>
Thanks for the explanation. I will rename ___gettimeofday{64} to
__gettimeofday{64} in the next version of this patch.
> However this specific code for alpha is not really required, since
> 04da832e16a7 it implements its gettimeofday version in its own
> syscalls.list. So this code is not used anywhere (along with alpha
> gettimeofday.c implementation) and I push a patch to remove it.
Ok.
>
> >
> > And why would we need the strong_alias(___gettimeofday,
> > __gettimeofday) in ./time/gettimeofday,c ?
> > In the same file the ___gettimeofday is aliased (as weak) to "final"
> > gettimeofday symbol.
> >
> >> The only ABI that requires versioning (alpha) includes
> >> the generic implementation with a arch-specific implementation.
> >
> > Yes.
> >
> >>
> >>>
> >>>>
> >>>>> diff --git a/sysdeps/unix/sysv/linux/gettimeofday.c
> >>>>> b/sysdeps/unix/sysv/linux/gettimeofday.c index
> >>>>> d5cdb22495..728ef45eed 100644 ---
> >>>>> a/sysdeps/unix/sysv/linux/gettimeofday.c +++
> >>>>> b/sysdeps/unix/sysv/linux/gettimeofday.c @@ -54,5 +54,49 @@
> >>>>> __gettimeofday (struct timeval *restrict tv, void *restrict tz)
> >>>>> # endif weak_alias (__gettimeofday, gettimeofday)
> >>>>
> >>>> We still need to handle 32-bit architecture that define
> >>>> USE_IFUNC_GETTIMEOFDAY, currently i686 and powerpc.
> >>>
> >>> By "handle" do you mean to make them Y2038 safe? The code in this
> >>> patch target archs, which do not have vDSO support for
> >>> gettimeofday (like arm32).
> >>>
> >>
> >> Yes, on powerpc32 and i686 (which defines USE_IFUNC_GETTIMEOFDAY)
> >> won't use the y2038 safe implementation.
> >
> > As the gettimeofday is going to be obsolete, why should we care (and
> > implement the fallback for this vDSO syscall)?
>
> Although marked obsolescent by POSIX, we can't really removed since a
> program might require to build against a specific POSIX version
> (_POSIX_C_SOURCE).
>
> Once POSIX does remove the symbol, we can remove its definition from
> default visibility (usually by setting the latest POSIX as default
> version) and add __attribute_deprecated__.
>
> So we still need to adapt it to be y2038 proof on current release.
Ok.
>
> >
> >>
> >>>>
> >>>> We can either:
> >>>>
> >>>> 1. Define the INLINE_VSYSCALL __gettimeofday for
> >>>> USE_IFUNC_GETTIMEOFDAY, or ___gettimeofday that calls
> >>>> __clock_gettime64 otherwise. The __gettimeofday64 will call
> >>>> either of this implementations.
> >>>>
> >>>> 2. Do not define USE_IFUNC_GETTIMEOFDAY for i686 and powerpc32,
> >>>> and add a _Static_assert (__TIMESIZE == 64) within iFUNC
> >>>> definition.
> >>>
> >>> Ok, so we do need a fallback for vDSO gettimeofday. I will poke
> >>> around and try to add this in v2 (but I'm only runtime testing
> >>> Y2039 on ARM32).
> >>
> >> I really don't think optimizing USE_IFUNC_GETTIMEOFDAY for
> >> architectures which still support fallback time32 is worth. It
> >> will require much more code complexity and it would result in more
> >> build permutations (one for !__ASSUME_TIME64_SYSCALLS which calls
> >> the vDSO through iFUNC, another one that calls the vDSO fallback
> >> and finally the one that call clock_gettime). Also, on build with
> >> time64 support the vDSO will only used as fallback.
> >>
> >> I would say to just not define USE_IFUNC_GETTIMEOFDAY for i686 and
> >> powerpc32 and make them follow the expected code path for y2038
> >> safeness.
> >
> > Shall this be added to this patch? Or shall I left the
> > USE_IFUNC_GETTIMEOFDAY untouched?
>
> Since this patch aims to add time64 gettimeofday for all supported
> Linux ABI, I think it should be along this patch.
So the proposed solution here is to remove
#define USE_IFUNC_GETTIMEOFDAY
from
sysdeps/unix/sysv/linux/powerpc/gettimeofday.c
sysdeps/unix/sysv/linux/x86/gettimeofday.c
?
I do guess that it would be acceptable to trade some minimal extra
overhead for gettimeofday serving on those archs for glibc simplicity
and Y2038 safeness?
>
> >
> >>
> >>>
> >>>>
> >>>> I am more inclined to 2., it simplifies the somewhat macro
> >>>> convolution and such optimization most likely won't make much
> >>>> difference for the specific platforms. Thoughts?
> >>>>
> >>>>> #else /* USE_IFUNC_GETTIMEOFDAY */
> >>>>> -# include <time/gettimeofday.c>
> >>>>> +/* Conversion of gettimeofday function to support 64 bit time
> >>>>> on archs
> >>>>> + with __WORDSIZE == 32 and __TIMESIZE == 32/64 */
> >>>>> +#include <errno.h>
> >>>>> +
> >>>>> +int
> >>>>> +___gettimeofday64 (struct __timeval64 *restrict tv, void
> >>>>> *restrict tz) +{
> >>>>> + if (__glibc_unlikely (tz != 0))
> >>>>> + memset (tz, 0, sizeof (struct timezone));
> >>>>> +
> >>>>> + struct __timespec64 ts64;
> >>>>> + int ret = __clock_gettime64 (CLOCK_REALTIME, &ts64);
> >>>>> +
> >>>>> + if (ret == 0 && tv)
> >>>>> + *tv = timespec64_to_timeval64 (ts64);
> >>>>
> >>>> No implicit checks. Also, we already set 'tv' with nonull
> >>>> attribute, so I am not sure if it is worth to add an extra check
> >>>> for 'tv' validity (specially because users tend to expect low
> >>>> latency for the symbol).
> >>>>
> >>>> In any case, if the idea is to add such check as QoI I think it
> >>>> would be better to do a early bail before actually issue
> >>>> __clock_gettime64.
> >>>
> >>> No, this was just my mistake. There was a discussion with Paul and
> >>> Joseph earlier. We shall _only_ check for NULL when it is required
> >>> by syscalls/command documentation. This is the case for e.g.
> >>> setitimer's *old_value pointer.
> >>>
> >>> In other examples we just allow segfault when dereference the NULL
> >>> pointer.
> >>>
> >>> I will fix it in v2 of this patch series.
> >>
> >> Ack.
> >>
> >>>
> >>>>
> >>>>> +
> >>>>> + return ret;
> >>>>> +}
> >>>>
> >>>> Ok (no 64-bit gettimeofday on any architecture).
> >>>>
> >>>>> +
> >>>>> +# if __TIMESIZE != 64
> >>>>> +libc_hidden_def (___gettimeofday64)
> >>>>> +
> >>>>> +int
> >>>>> +___gettimeofday (struct timeval *restrict tv, void *restrict
> >>>>> tz) +{
> >>>>> + struct __timeval64 tv64;
> >>>>> + int ret = ___gettimeofday64 (&tv64, tz);
> >>>>> +
> >>>>> + if (ret == 0)
> >>>>> + {
> >>>>> + if (! in_time_t_range (tv64.tv_sec))
> >>>>> + {
> >>>>> + __set_errno (EOVERFLOW);
> >>>>> + return -1;
> >>>>> + }
> >>>>> +
> >>>>> + if (tv)
> >>>>> + *tv = valid_timeval64_to_timeval (tv64);
> >>>>> + }
> >>>>> +
> >>>>> + return ret;
> >>>>> +}
> >>>>> +# endif
> >>>>> +strong_alias (___gettimeofday, __gettimeofday)
> >>>>
> >>>> I am failing to see the need of this alias.
> >>>
> >>> I've followed the idiom from ./time/gettimeofday.c
> >>>
> >>> The strong alias will bind this local definition of
> >>> ___gettimeofday to glibc's internal __gettimeofday (used
> >>> internally by glibc).
> >>>
> >>> In the case where gettimeofday exported function is not defined,
> >>> the ___gettimeofday will be a weak alias for it.
> >>>
> >>>
> >>>>
> >>>>> +weak_alias (___gettimeofday, gettimeofday)
> >>>>> #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
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 --]
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH v3 7/7] y2038: linux: Provide ___gettimeofday64 implementation
2020-02-06 10:08 ` Lukasz Majewski
@ 2020-02-07 13:20 ` Adhemerval Zanella
0 siblings, 0 replies; 31+ messages in thread
From: Adhemerval Zanella @ 2020-02-07 13:20 UTC (permalink / raw)
To: Lukasz Majewski
Cc: Joseph Myers, Paul Eggert, Alistair Francis, Alistair Francis,
GNU C Library, Siddhesh Poyarekar, Florian Weimer,
Florian Weimer, Zack Weinberg, Carlos O'Donell,
Andreas Schwab
On 06/02/2020 07:08, Lukasz Majewski wrote:
>>>> I would say to just not define USE_IFUNC_GETTIMEOFDAY for i686 and
>>>> powerpc32 and make them follow the expected code path for y2038
>>>> safeness.
>>>
>>> Shall this be added to this patch? Or shall I left the
>>> USE_IFUNC_GETTIMEOFDAY untouched?
>>
>> Since this patch aims to add time64 gettimeofday for all supported
>> Linux ABI, I think it should be along this patch.
>
> So the proposed solution here is to remove
>
> #define USE_IFUNC_GETTIMEOFDAY
>
> from
>
> sysdeps/unix/sysv/linux/powerpc/gettimeofday.c
> sysdeps/unix/sysv/linux/x86/gettimeofday.c
Only if the ABI have legacy 32 bits time support. More specifically,
for x86 implementation:
#ifdef __x86_64__
# define USE_IFUNC_GETTIMEOFDAY
#endif
And for powerpc:
#ifdef __powerpc64__
# define USE_IFUNC_GETTIMEOFDAY
#endif
>
> ?
>
> I do guess that it would be acceptable to trade some minimal extra
> overhead for gettimeofday serving on those archs for glibc simplicity
> and Y2038 safeness?
My point is the vDSO ifunc optimization on a y2038 safe kernel does not
really matter, since there won't be a 64-bit time gettimeofday implementation
and all architectures will need to call clock_gettime64 (either by syscall or
vDSO) anyway.
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v3 7/7] y2038: linux: Provide ___gettimeofday64 implementation
2020-02-05 17:49 ` Adhemerval Zanella
2020-02-06 10:08 ` Lukasz Majewski
@ 2020-02-06 21:41 ` Joseph Myers
2020-02-07 12:54 ` Adhemerval Zanella
1 sibling, 1 reply; 31+ messages in thread
From: Joseph Myers @ 2020-02-06 21:41 UTC (permalink / raw)
To: Adhemerval Zanella
Cc: Lukasz Majewski, Paul Eggert, Alistair Francis, Alistair Francis,
GNU C Library, Siddhesh Poyarekar, Florian Weimer,
Florian Weimer, Zack Weinberg, Carlos O'Donell,
Andreas Schwab
On Wed, 5 Feb 2020, Adhemerval Zanella wrote:
> Although marked obsolescent by POSIX, we can't really removed since a
> program might require to build against a specific POSIX version
> (_POSIX_C_SOURCE).
>
> Once POSIX does remove the symbol, we can remove its definition from
> default visibility (usually by setting the latest POSIX as default
> version) and add __attribute_deprecated__.
I don't see that being appropriate for a very long time.
Some symbols marked obsolescent by POSIX are genuinely out of use for a
very long time, or genuinely have serious deficiencies, and may be
candidates for obsoleting in various ways in glibc. Some, such as
gettimeofday are in very widespread use and pose no particular problems -
so when it's not in current POSIX, I expect we should still declare it for
__USE_MISC, without a deprecation attribute. (And some symbols are marked
obsolescent by POSIX because of issues specifying them portably, but are
very relevant for less-portable code - vfork and getcontext, for example.)
--
Joseph S. Myers
joseph@codesourcery.com
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v3 7/7] y2038: linux: Provide ___gettimeofday64 implementation
2020-02-06 21:41 ` Joseph Myers
@ 2020-02-07 12:54 ` Adhemerval Zanella
0 siblings, 0 replies; 31+ messages in thread
From: Adhemerval Zanella @ 2020-02-07 12:54 UTC (permalink / raw)
To: Joseph Myers
Cc: Lukasz Majewski, Paul Eggert, Alistair Francis, Alistair Francis,
GNU C Library, Siddhesh Poyarekar, Florian Weimer,
Florian Weimer, Zack Weinberg, Carlos O'Donell,
Andreas Schwab
On 06/02/2020 18:41, Joseph Myers wrote:
> On Wed, 5 Feb 2020, Adhemerval Zanella wrote:
>
>> Although marked obsolescent by POSIX, we can't really removed since a
>> program might require to build against a specific POSIX version
>> (_POSIX_C_SOURCE).
>>
>> Once POSIX does remove the symbol, we can remove its definition from
>> default visibility (usually by setting the latest POSIX as default
>> version) and add __attribute_deprecated__.
>
> I don't see that being appropriate for a very long time.
>
> Some symbols marked obsolescent by POSIX are genuinely out of use for a
> very long time, or genuinely have serious deficiencies, and may be
> candidates for obsoleting in various ways in glibc. Some, such as
> gettimeofday are in very widespread use and pose no particular problems -
> so when it's not in current POSIX, I expect we should still declare it for
> __USE_MISC, without a deprecation attribute. (And some symbols are marked
> obsolescent by POSIX because of issues specifying them portably, but are
> very relevant for less-portable code - vfork and getcontext, for example.)
>
I see this as a possible option, I agree that for 'gettimeofday' due its
extensive usage I don't see much gain in setting it as deprecated in near
future.
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v3 7/7] y2038: linux: Provide ___gettimeofday64 implementation
2020-02-05 0:06 ` Lukasz Majewski
2020-02-05 16:23 ` Adhemerval Zanella
@ 2020-02-08 22:16 ` Lukasz Majewski
2020-02-10 16:15 ` Adhemerval Zanella
1 sibling, 1 reply; 31+ messages in thread
From: Lukasz Majewski @ 2020-02-08 22:16 UTC (permalink / raw)
To: Adhemerval Zanella, Joseph Myers
Cc: Paul Eggert, Alistair Francis, Alistair Francis, GNU C Library,
Siddhesh Poyarekar, Florian Weimer, Florian Weimer,
Zack Weinberg, Carlos O'Donell, Andreas Schwab
[-- Attachment #1: Type: text/plain, Size: 2142 bytes --]
Hi Adhemerval,
> > > #else /* USE_IFUNC_GETTIMEOFDAY */
> > > -# include <time/gettimeofday.c>
> > > +/* Conversion of gettimeofday function to support 64 bit time on
> > > archs
> > > + with __WORDSIZE == 32 and __TIMESIZE == 32/64 */
> > > +#include <errno.h>
> > > +
> > > +int
> > > +___gettimeofday64 (struct __timeval64 *restrict tv, void
> > > *restrict tz) +{
> > > + if (__glibc_unlikely (tz != 0))
> > > + memset (tz, 0, sizeof (struct timezone));
> > > +
> > > + struct __timespec64 ts64;
> > > + int ret = __clock_gettime64 (CLOCK_REALTIME, &ts64);
> > > +
> > > + if (ret == 0 && tv)
> > > + *tv = timespec64_to_timeval64 (ts64);
> >
> > No implicit checks. Also, we already set 'tv' with nonull
> > attribute, so I am not sure if it is worth to add an extra check
> > for 'tv' validity (specially because users tend to expect low
> > latency for the symbol).
> >
> > In any case, if the idea is to add such check as QoI I think it
> > would be better to do a early bail before actually issue
> > __clock_gettime64.
>
> No, this was just my mistake. There was a discussion with Paul and
> Joseph earlier. We shall _only_ check for NULL when it is required by
> syscalls/command documentation. This is the case for e.g. setitimer's
> *old_value pointer.
I've double check this and in the documentation/manual [1] for
gettimeofday there is a sentence:
----8<--------
If either tv or tz is NULL, the corresponding structure is not set or
returned. (However, compilation warnings will result if tv is NULL.)
---->8--------
That was the rationale to add the check
if (ret == 0 && tv)
I also think that the code as is now is correct - it returns the result
of getting the time from Linux, but it is not updating the tv structure.
Links:
[1] - https://linux.die.net/man/2/gettimeofday
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 --]
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH v3 7/7] y2038: linux: Provide ___gettimeofday64 implementation
2020-02-08 22:16 ` Lukasz Majewski
@ 2020-02-10 16:15 ` Adhemerval Zanella
2020-02-10 16:57 ` Lukasz Majewski
0 siblings, 1 reply; 31+ messages in thread
From: Adhemerval Zanella @ 2020-02-10 16:15 UTC (permalink / raw)
To: Lukasz Majewski, Joseph Myers
Cc: Paul Eggert, Alistair Francis, Alistair Francis, GNU C Library,
Siddhesh Poyarekar, Florian Weimer, Florian Weimer,
Zack Weinberg, Carlos O'Donell, Andreas Schwab
On 08/02/2020 19:15, Lukasz Majewski wrote:
> Hi Adhemerval,
>
>>>> #else /* USE_IFUNC_GETTIMEOFDAY */
>>>> -# include <time/gettimeofday.c>
>>>> +/* Conversion of gettimeofday function to support 64 bit time on
>>>> archs
>>>> + with __WORDSIZE == 32 and __TIMESIZE == 32/64 */
>>>> +#include <errno.h>
>>>> +
>>>> +int
>>>> +___gettimeofday64 (struct __timeval64 *restrict tv, void
>>>> *restrict tz) +{
>>>> + if (__glibc_unlikely (tz != 0))
>>>> + memset (tz, 0, sizeof (struct timezone));
>>>> +
>>>> + struct __timespec64 ts64;
>>>> + int ret = __clock_gettime64 (CLOCK_REALTIME, &ts64);
>>>> +
>>>> + if (ret == 0 && tv)
>>>> + *tv = timespec64_to_timeval64 (ts64);
>>>
>>> No implicit checks. Also, we already set 'tv' with nonull
>>> attribute, so I am not sure if it is worth to add an extra check
>>> for 'tv' validity (specially because users tend to expect low
>>> latency for the symbol).
>>>
>>> In any case, if the idea is to add such check as QoI I think it
>>> would be better to do a early bail before actually issue
>>> __clock_gettime64.
>>
>> No, this was just my mistake. There was a discussion with Paul and
>> Joseph earlier. We shall _only_ check for NULL when it is required by
>> syscalls/command documentation. This is the case for e.g. setitimer's
>> *old_value pointer.
>
> I've double check this and in the documentation/manual [1] for
> gettimeofday there is a sentence:
>
> ----8<--------
> If either tv or tz is NULL, the corresponding structure is not set or
> returned. (However, compilation warnings will result if tv is NULL.)
> ---->8--------
>
> That was the rationale to add the check
> if (ret == 0 && tv)
>
>
> I also think that the code as is now is correct - it returns the result
> of getting the time from Linux, but it is not updating the tv structure.
The man-pages is not really the glibc manual, but rather documents de facto
glibc/kernel behaviour. The glibc 'gettimeofday' entry in manual
(manual/time.texi) is also not explicit about this, and the generic
implementation (time/gettimeofday.c) also does not add this test.
But again, I am not against of this change as QoI and it is what kernel vDSO
symbol does anyway (lib/vdso/gettimeofday.c:270). However, I think we should
be done in a separated patch and for 'all' implementation to get a concise
behaviour.
>
>
> Links:
>
> [1] - https://linux.die.net/man/2/gettimeofday
>
>
> Best regards,
>
> Lukasz Majewski
>
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH v3 7/7] y2038: linux: Provide ___gettimeofday64 implementation
2020-02-10 16:15 ` Adhemerval Zanella
@ 2020-02-10 16:57 ` Lukasz Majewski
2020-02-10 17:26 ` Adhemerval Zanella
0 siblings, 1 reply; 31+ messages in thread
From: Lukasz Majewski @ 2020-02-10 16:57 UTC (permalink / raw)
To: Adhemerval Zanella
Cc: Joseph Myers, Paul Eggert, Alistair Francis, Alistair Francis,
GNU C Library, Siddhesh Poyarekar, Florian Weimer,
Florian Weimer, Zack Weinberg, Carlos O'Donell,
Andreas Schwab
[-- Attachment #1: Type: text/plain, Size: 3508 bytes --]
Hi Adhemerval,
> On 08/02/2020 19:15, Lukasz Majewski wrote:
> > Hi Adhemerval,
> >
> >>>> #else /* USE_IFUNC_GETTIMEOFDAY */
> >>>> -# include <time/gettimeofday.c>
> >>>> +/* Conversion of gettimeofday function to support 64 bit time on
> >>>> archs
> >>>> + with __WORDSIZE == 32 and __TIMESIZE == 32/64 */
> >>>> +#include <errno.h>
> >>>> +
> >>>> +int
> >>>> +___gettimeofday64 (struct __timeval64 *restrict tv, void
> >>>> *restrict tz) +{
> >>>> + if (__glibc_unlikely (tz != 0))
> >>>> + memset (tz, 0, sizeof (struct timezone));
> >>>> +
> >>>> + struct __timespec64 ts64;
> >>>> + int ret = __clock_gettime64 (CLOCK_REALTIME, &ts64);
> >>>> +
> >>>> + if (ret == 0 && tv)
> >>>> + *tv = timespec64_to_timeval64 (ts64);
> >>>
> >>> No implicit checks. Also, we already set 'tv' with nonull
> >>> attribute, so I am not sure if it is worth to add an extra check
> >>> for 'tv' validity (specially because users tend to expect low
> >>> latency for the symbol).
> >>>
> >>> In any case, if the idea is to add such check as QoI I think it
> >>> would be better to do a early bail before actually issue
> >>> __clock_gettime64.
> >>
> >> No, this was just my mistake. There was a discussion with Paul and
> >> Joseph earlier. We shall _only_ check for NULL when it is required
> >> by syscalls/command documentation. This is the case for e.g.
> >> setitimer's *old_value pointer.
> >
> > I've double check this and in the documentation/manual [1] for
> > gettimeofday there is a sentence:
> >
> > ----8<--------
> > If either tv or tz is NULL, the corresponding structure is not set
> > or returned. (However, compilation warnings will result if tv is
> > NULL.) ---->8--------
> >
> > That was the rationale to add the check
> > if (ret == 0 && tv)
> >
> >
> > I also think that the code as is now is correct - it returns the
> > result of getting the time from Linux, but it is not updating the
> > tv structure.
>
> The man-pages is not really the glibc manual, but rather documents de
> facto glibc/kernel behaviour. The glibc 'gettimeofday' entry in
> manual (manual/time.texi) is also not explicit about this, and the
> generic implementation (time/gettimeofday.c) also does not add this
> test.
Ok.
>
> But again, I am not against of this change as QoI and it is what
> kernel vDSO symbol does anyway (lib/vdso/gettimeofday.c:270).
I do guess that you refer to:
https://elixir.bootlin.com/linux/latest/source/lib/vdso/gettimeofday.c#L144
We could rely on the kernel if there weren't conversions (64 <->
32 bit time ) needed.
> However, I think we should be done in a separated patch and for 'all'
> implementation to get a concise behaviour.
Shall I:
1. Extend this patch to add this extra check to all eligible
gettimeofday places.
2. Do not check if tv is NULL at all
3. Do not check if tv is NULL in this particular patch and prepare next
patch which would add check for tv != NULL to all gettimeofday
implementations in glibc ?
>
>
> >
> >
> > Links:
> >
> > [1] - https://linux.die.net/man/2/gettimeofday
> >
> >
> > Best regards,
> >
> > Lukasz Majewski
> >
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 --]
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH v3 7/7] y2038: linux: Provide ___gettimeofday64 implementation
2020-02-10 16:57 ` Lukasz Majewski
@ 2020-02-10 17:26 ` Adhemerval Zanella
0 siblings, 0 replies; 31+ messages in thread
From: Adhemerval Zanella @ 2020-02-10 17:26 UTC (permalink / raw)
To: Lukasz Majewski
Cc: Joseph Myers, Paul Eggert, Alistair Francis, Alistair Francis,
GNU C Library, Siddhesh Poyarekar, Florian Weimer,
Florian Weimer, Zack Weinberg, Carlos O'Donell,
Andreas Schwab
On 10/02/2020 13:57, Lukasz Majewski wrote:
>>>> No, this was just my mistake. There was a discussion with Paul and
>>>> Joseph earlier. We shall _only_ check for NULL when it is required
>>>> by syscalls/command documentation. This is the case for e.g.
>>>> setitimer's *old_value pointer.
>>>
>>> I've double check this and in the documentation/manual [1] for
>>> gettimeofday there is a sentence:
>>>
>>> ----8<--------
>>> If either tv or tz is NULL, the corresponding structure is not set
>>> or returned. (However, compilation warnings will result if tv is
>>> NULL.) ---->8--------
>>>
>>> That was the rationale to add the check
>>> if (ret == 0 && tv)
>>>
>>>
>>> I also think that the code as is now is correct - it returns the
>>> result of getting the time from Linux, but it is not updating the
>>> tv structure.
>>
>> The man-pages is not really the glibc manual, but rather documents de
>> facto glibc/kernel behaviour. The glibc 'gettimeofday' entry in
>> manual (manual/time.texi) is also not explicit about this, and the
>> generic implementation (time/gettimeofday.c) also does not add this
>> test.
>
> Ok.
>
>>
>> But again, I am not against of this change as QoI and it is what
>> kernel vDSO symbol does anyway (lib/vdso/gettimeofday.c:270).
>
> I do guess that you refer to:
> https://elixir.bootlin.com/linux/latest/source/lib/vdso/gettimeofday.c#L144
Yes (I was using current master branch instead of a tag release).
>
> We could rely on the kernel if there weren't conversions (64 <->
> 32 bit time ) needed.
My point was that kernel vDSO gettimeofday implementation already does
the tv check before setting its value, so it should be an QoI to do this
on glibc as well.
>
>> However, I think we should be done in a separated patch and for 'all'
>> implementation to get a concise behaviour.
>
> Shall I:
>
> 1. Extend this patch to add this extra check to all eligible
> gettimeofday places.
>
> 2. Do not check if tv is NULL at all
>
> 3. Do not check if tv is NULL in this particular patch and prepare next
> patch which would add check for tv != NULL to all gettimeofday
> implementations in glibc ?
I would prefer to remove the check on this patch and send an extra
patch to actually adding the check on all implementations.
>
>>
>>
>>>
>>>
>>> Links:
>>>
>>> [1] - https://linux.die.net/man/2/gettimeofday
>>>
>>>
>>> Best regards,
>>>
>>> Lukasz Majewski
>>>
>
>
>
>
> 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
>
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH v3 4/7] y2038: alpha: Rename valid_timeval64_to_timeval to valid_timeval_to_timeval32
2020-01-29 12:59 [PATCH v3 1/7] y2038: Define __suseconds64_t type to be used with struct __timeval64 Lukasz Majewski
` (2 preceding siblings ...)
2020-01-29 12:59 ` [PATCH v3 7/7] y2038: linux: Provide ___gettimeofday64 implementation Lukasz Majewski
@ 2020-01-29 12:59 ` Lukasz Majewski
2020-01-30 21:41 ` Alistair Francis
2020-02-04 18:43 ` Adhemerval Zanella
2020-01-29 12:59 ` [PATCH v3 6/7] y2038: linux: Provide __settimeofday64 implementation Lukasz Majewski
` (3 subsequent siblings)
7 siblings, 2 replies; 31+ messages in thread
From: Lukasz Majewski @ 2020-01-29 12:59 UTC (permalink / raw)
To: Joseph Myers, Paul Eggert, Adhemerval Zanella
Cc: Alistair Francis, Alistair Francis, GNU C Library,
Siddhesh Poyarekar, Florian Weimer, Florian Weimer,
Zack Weinberg, Carlos O'Donell, Andreas Schwab,
Lukasz Majewski
The name 'valid_timeval64_to_timeval' suggest conversion of struct
__timeval64 to struct timeval (as in ./include/time.h).
As on the alpha the struct timeval supports 64 bit time, it seems more
feasible to emphasis struct timeval32 in the conversion function name.
Hence the helper function naming change to 'valid_timeval_to_timeval32'.
Build tests:
./src/scripts/build-many-glibcs.py glibcs
---
Changes for v3:
- New patch
---
sysdeps/unix/sysv/linux/alpha/osf_adjtime.c | 4 ++--
sysdeps/unix/sysv/linux/alpha/osf_getitimer.c | 4 ++--
sysdeps/unix/sysv/linux/alpha/osf_setitimer.c | 4 ++--
sysdeps/unix/sysv/linux/alpha/tv32-compat.h | 6 +++---
4 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/sysdeps/unix/sysv/linux/alpha/osf_adjtime.c b/sysdeps/unix/sysv/linux/alpha/osf_adjtime.c
index 5ac72e252f..9825a4734d 100644
--- a/sysdeps/unix/sysv/linux/alpha/osf_adjtime.c
+++ b/sysdeps/unix/sysv/linux/alpha/osf_adjtime.c
@@ -63,7 +63,7 @@ __adjtime_tv32 (const struct timeval32 *itv, struct timeval32 *otv)
if (__adjtime (&itv64, &otv64) == -1)
return -1;
- *otv = valid_timeval64_to_timeval (otv64);
+ *otv = valid_timeval_to_timeval32 (otv64);
return 0;
}
@@ -116,7 +116,7 @@ __adjtimex_tv32 (struct timex32 *tx)
tx->calcnt = tx64.calcnt;
tx->errcnt = tx64.errcnt;
tx->stbcnt = tx64.stbcnt;
- tx->time = valid_timeval64_to_timeval (tx64.time);
+ tx->time = valid_timeval_to_timeval32 (tx64.time);
return status;
}
diff --git a/sysdeps/unix/sysv/linux/alpha/osf_getitimer.c b/sysdeps/unix/sysv/linux/alpha/osf_getitimer.c
index 1da3d72411..e9de2b287b 100644
--- a/sysdeps/unix/sysv/linux/alpha/osf_getitimer.c
+++ b/sysdeps/unix/sysv/linux/alpha/osf_getitimer.c
@@ -33,9 +33,9 @@ __getitimer_tv32 (int which, struct itimerval32 *curr_value)
/* Write all fields of 'curr_value' regardless of overflow. */
curr_value->it_interval
- = valid_timeval64_to_timeval (curr_value_64.it_interval);
+ = valid_timeval_to_timeval32 (curr_value_64.it_interval);
curr_value->it_value
- = valid_timeval64_to_timeval (curr_value_64.it_value);
+ = valid_timeval_to_timeval32 (curr_value_64.it_value);
return 0;
}
diff --git a/sysdeps/unix/sysv/linux/alpha/osf_setitimer.c b/sysdeps/unix/sysv/linux/alpha/osf_setitimer.c
index 3935d1cfb5..7df2d1b71c 100644
--- a/sysdeps/unix/sysv/linux/alpha/osf_setitimer.c
+++ b/sysdeps/unix/sysv/linux/alpha/osf_setitimer.c
@@ -43,9 +43,9 @@ __setitimer_tv32 (int which, const struct itimerval32 *restrict new_value,
/* Write all fields of 'old_value' regardless of overflow. */
old_value->it_interval
- = valid_timeval64_to_timeval (old_value_64.it_interval);
+ = valid_timeval_to_timeval32 (old_value_64.it_interval);
old_value->it_value
- = valid_timeval64_to_timeval (old_value_64.it_value);
+ = valid_timeval_to_timeval32 (old_value_64.it_value);
return 0;
}
diff --git a/sysdeps/unix/sysv/linux/alpha/tv32-compat.h b/sysdeps/unix/sysv/linux/alpha/tv32-compat.h
index 7169909259..8e34ed1c1b 100644
--- a/sysdeps/unix/sysv/linux/alpha/tv32-compat.h
+++ b/sysdeps/unix/sysv/linux/alpha/tv32-compat.h
@@ -76,7 +76,7 @@ valid_timeval32_to_timeval (const struct timeval32 tv)
}
static inline struct timeval32
-valid_timeval64_to_timeval (const struct timeval tv64)
+valid_timeval_to_timeval32 (const struct timeval tv64)
{
if (__glibc_unlikely (tv64.tv_sec > (time_t) INT32_MAX))
return (struct timeval32) { INT32_MAX, TV_USEC_MAX};
@@ -103,8 +103,8 @@ rusage64_to_rusage32 (struct rusage32 *restrict r32,
padding and reserved fields. */
memset (r32, 0, sizeof *r32);
- r32->ru_utime = valid_timeval64_to_timeval (r64->ru_utime);
- r32->ru_stime = valid_timeval64_to_timeval (r64->ru_stime);
+ r32->ru_utime = valid_timeval_to_timeval32 (r64->ru_utime);
+ r32->ru_stime = valid_timeval_to_timeval32 (r64->ru_stime);
r32->ru_maxrss = r64->ru_maxrss;
r32->ru_ixrss = r64->ru_ixrss;
r32->ru_idrss = r64->ru_idrss;
--
2.20.1
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH v3 4/7] y2038: alpha: Rename valid_timeval64_to_timeval to valid_timeval_to_timeval32
2020-01-29 12:59 ` [PATCH v3 4/7] y2038: alpha: Rename valid_timeval64_to_timeval to valid_timeval_to_timeval32 Lukasz Majewski
@ 2020-01-30 21:41 ` Alistair Francis
2020-02-04 18:43 ` Adhemerval Zanella
1 sibling, 0 replies; 31+ messages in thread
From: Alistair Francis @ 2020-01-30 21:41 UTC (permalink / raw)
To: Lukasz Majewski
Cc: Joseph Myers, Paul Eggert, Adhemerval Zanella, Alistair Francis,
GNU C Library, Siddhesh Poyarekar, Florian Weimer,
Florian Weimer, Zack Weinberg, Carlos O'Donell,
Andreas Schwab
On Wed, Jan 29, 2020 at 4:59 AM Lukasz Majewski <lukma@denx.de> wrote:
>
> The name 'valid_timeval64_to_timeval' suggest conversion of struct
> __timeval64 to struct timeval (as in ./include/time.h).
>
> As on the alpha the struct timeval supports 64 bit time, it seems more
> feasible to emphasis struct timeval32 in the conversion function name.
>
> Hence the helper function naming change to 'valid_timeval_to_timeval32'.
>
> Build tests:
> ./src/scripts/build-many-glibcs.py glibcs
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Alistair
>
> ---
> Changes for v3:
> - New patch
> ---
> sysdeps/unix/sysv/linux/alpha/osf_adjtime.c | 4 ++--
> sysdeps/unix/sysv/linux/alpha/osf_getitimer.c | 4 ++--
> sysdeps/unix/sysv/linux/alpha/osf_setitimer.c | 4 ++--
> sysdeps/unix/sysv/linux/alpha/tv32-compat.h | 6 +++---
> 4 files changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/sysdeps/unix/sysv/linux/alpha/osf_adjtime.c b/sysdeps/unix/sysv/linux/alpha/osf_adjtime.c
> index 5ac72e252f..9825a4734d 100644
> --- a/sysdeps/unix/sysv/linux/alpha/osf_adjtime.c
> +++ b/sysdeps/unix/sysv/linux/alpha/osf_adjtime.c
> @@ -63,7 +63,7 @@ __adjtime_tv32 (const struct timeval32 *itv, struct timeval32 *otv)
> if (__adjtime (&itv64, &otv64) == -1)
> return -1;
>
> - *otv = valid_timeval64_to_timeval (otv64);
> + *otv = valid_timeval_to_timeval32 (otv64);
> return 0;
> }
>
> @@ -116,7 +116,7 @@ __adjtimex_tv32 (struct timex32 *tx)
> tx->calcnt = tx64.calcnt;
> tx->errcnt = tx64.errcnt;
> tx->stbcnt = tx64.stbcnt;
> - tx->time = valid_timeval64_to_timeval (tx64.time);
> + tx->time = valid_timeval_to_timeval32 (tx64.time);
>
> return status;
> }
> diff --git a/sysdeps/unix/sysv/linux/alpha/osf_getitimer.c b/sysdeps/unix/sysv/linux/alpha/osf_getitimer.c
> index 1da3d72411..e9de2b287b 100644
> --- a/sysdeps/unix/sysv/linux/alpha/osf_getitimer.c
> +++ b/sysdeps/unix/sysv/linux/alpha/osf_getitimer.c
> @@ -33,9 +33,9 @@ __getitimer_tv32 (int which, struct itimerval32 *curr_value)
>
> /* Write all fields of 'curr_value' regardless of overflow. */
> curr_value->it_interval
> - = valid_timeval64_to_timeval (curr_value_64.it_interval);
> + = valid_timeval_to_timeval32 (curr_value_64.it_interval);
> curr_value->it_value
> - = valid_timeval64_to_timeval (curr_value_64.it_value);
> + = valid_timeval_to_timeval32 (curr_value_64.it_value);
> return 0;
> }
>
> diff --git a/sysdeps/unix/sysv/linux/alpha/osf_setitimer.c b/sysdeps/unix/sysv/linux/alpha/osf_setitimer.c
> index 3935d1cfb5..7df2d1b71c 100644
> --- a/sysdeps/unix/sysv/linux/alpha/osf_setitimer.c
> +++ b/sysdeps/unix/sysv/linux/alpha/osf_setitimer.c
> @@ -43,9 +43,9 @@ __setitimer_tv32 (int which, const struct itimerval32 *restrict new_value,
>
> /* Write all fields of 'old_value' regardless of overflow. */
> old_value->it_interval
> - = valid_timeval64_to_timeval (old_value_64.it_interval);
> + = valid_timeval_to_timeval32 (old_value_64.it_interval);
> old_value->it_value
> - = valid_timeval64_to_timeval (old_value_64.it_value);
> + = valid_timeval_to_timeval32 (old_value_64.it_value);
> return 0;
> }
>
> diff --git a/sysdeps/unix/sysv/linux/alpha/tv32-compat.h b/sysdeps/unix/sysv/linux/alpha/tv32-compat.h
> index 7169909259..8e34ed1c1b 100644
> --- a/sysdeps/unix/sysv/linux/alpha/tv32-compat.h
> +++ b/sysdeps/unix/sysv/linux/alpha/tv32-compat.h
> @@ -76,7 +76,7 @@ valid_timeval32_to_timeval (const struct timeval32 tv)
> }
>
> static inline struct timeval32
> -valid_timeval64_to_timeval (const struct timeval tv64)
> +valid_timeval_to_timeval32 (const struct timeval tv64)
> {
> if (__glibc_unlikely (tv64.tv_sec > (time_t) INT32_MAX))
> return (struct timeval32) { INT32_MAX, TV_USEC_MAX};
> @@ -103,8 +103,8 @@ rusage64_to_rusage32 (struct rusage32 *restrict r32,
> padding and reserved fields. */
> memset (r32, 0, sizeof *r32);
>
> - r32->ru_utime = valid_timeval64_to_timeval (r64->ru_utime);
> - r32->ru_stime = valid_timeval64_to_timeval (r64->ru_stime);
> + r32->ru_utime = valid_timeval_to_timeval32 (r64->ru_utime);
> + r32->ru_stime = valid_timeval_to_timeval32 (r64->ru_stime);
> r32->ru_maxrss = r64->ru_maxrss;
> r32->ru_ixrss = r64->ru_ixrss;
> r32->ru_idrss = r64->ru_idrss;
> --
> 2.20.1
>
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH v3 4/7] y2038: alpha: Rename valid_timeval64_to_timeval to valid_timeval_to_timeval32
2020-01-29 12:59 ` [PATCH v3 4/7] y2038: alpha: Rename valid_timeval64_to_timeval to valid_timeval_to_timeval32 Lukasz Majewski
2020-01-30 21:41 ` Alistair Francis
@ 2020-02-04 18:43 ` Adhemerval Zanella
1 sibling, 0 replies; 31+ messages in thread
From: Adhemerval Zanella @ 2020-02-04 18:43 UTC (permalink / raw)
To: Lukasz Majewski, Joseph Myers, Paul Eggert
Cc: Alistair Francis, Alistair Francis, GNU C Library,
Siddhesh Poyarekar, Florian Weimer, Florian Weimer,
Zack Weinberg, Carlos O'Donell, Andreas Schwab
On 29/01/2020 09:59, Lukasz Majewski wrote:
> The name 'valid_timeval64_to_timeval' suggest conversion of struct
> __timeval64 to struct timeval (as in ./include/time.h).
>
> As on the alpha the struct timeval supports 64 bit time, it seems more
> feasible to emphasis struct timeval32 in the conversion function name.
>
> Hence the helper function naming change to 'valid_timeval_to_timeval32'.
>
> Build tests:
> ./src/scripts/build-many-glibcs.py glibcs
As for the 3/7 patch of this set, I don't see much gain in such changes but
it is ok.
Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
>
> ---
> Changes for v3:
> - New patch
> ---
> sysdeps/unix/sysv/linux/alpha/osf_adjtime.c | 4 ++--
> sysdeps/unix/sysv/linux/alpha/osf_getitimer.c | 4 ++--
> sysdeps/unix/sysv/linux/alpha/osf_setitimer.c | 4 ++--
> sysdeps/unix/sysv/linux/alpha/tv32-compat.h | 6 +++---
> 4 files changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/sysdeps/unix/sysv/linux/alpha/osf_adjtime.c b/sysdeps/unix/sysv/linux/alpha/osf_adjtime.c
> index 5ac72e252f..9825a4734d 100644
> --- a/sysdeps/unix/sysv/linux/alpha/osf_adjtime.c
> +++ b/sysdeps/unix/sysv/linux/alpha/osf_adjtime.c
> @@ -63,7 +63,7 @@ __adjtime_tv32 (const struct timeval32 *itv, struct timeval32 *otv)
> if (__adjtime (&itv64, &otv64) == -1)
> return -1;
>
> - *otv = valid_timeval64_to_timeval (otv64);
> + *otv = valid_timeval_to_timeval32 (otv64);
> return 0;
> }
>
Ok.
> @@ -116,7 +116,7 @@ __adjtimex_tv32 (struct timex32 *tx)
> tx->calcnt = tx64.calcnt;
> tx->errcnt = tx64.errcnt;
> tx->stbcnt = tx64.stbcnt;
> - tx->time = valid_timeval64_to_timeval (tx64.time);
> + tx->time = valid_timeval_to_timeval32 (tx64.time);
>
> return status;
> }
Ok.
> diff --git a/sysdeps/unix/sysv/linux/alpha/osf_getitimer.c b/sysdeps/unix/sysv/linux/alpha/osf_getitimer.c
> index 1da3d72411..e9de2b287b 100644
> --- a/sysdeps/unix/sysv/linux/alpha/osf_getitimer.c
> +++ b/sysdeps/unix/sysv/linux/alpha/osf_getitimer.c
> @@ -33,9 +33,9 @@ __getitimer_tv32 (int which, struct itimerval32 *curr_value)
>
> /* Write all fields of 'curr_value' regardless of overflow. */
> curr_value->it_interval
> - = valid_timeval64_to_timeval (curr_value_64.it_interval);
> + = valid_timeval_to_timeval32 (curr_value_64.it_interval);
> curr_value->it_value
> - = valid_timeval64_to_timeval (curr_value_64.it_value);
> + = valid_timeval_to_timeval32 (curr_value_64.it_value);
> return 0;
> }
>
Ok.
> diff --git a/sysdeps/unix/sysv/linux/alpha/osf_setitimer.c b/sysdeps/unix/sysv/linux/alpha/osf_setitimer.c
> index 3935d1cfb5..7df2d1b71c 100644
> --- a/sysdeps/unix/sysv/linux/alpha/osf_setitimer.c
> +++ b/sysdeps/unix/sysv/linux/alpha/osf_setitimer.c
> @@ -43,9 +43,9 @@ __setitimer_tv32 (int which, const struct itimerval32 *restrict new_value,
>
> /* Write all fields of 'old_value' regardless of overflow. */
> old_value->it_interval
> - = valid_timeval64_to_timeval (old_value_64.it_interval);
> + = valid_timeval_to_timeval32 (old_value_64.it_interval);
> old_value->it_value
> - = valid_timeval64_to_timeval (old_value_64.it_value);
> + = valid_timeval_to_timeval32 (old_value_64.it_value);
> return 0;
> }
>
Ok.
> diff --git a/sysdeps/unix/sysv/linux/alpha/tv32-compat.h b/sysdeps/unix/sysv/linux/alpha/tv32-compat.h
> index 7169909259..8e34ed1c1b 100644
> --- a/sysdeps/unix/sysv/linux/alpha/tv32-compat.h
> +++ b/sysdeps/unix/sysv/linux/alpha/tv32-compat.h
> @@ -76,7 +76,7 @@ valid_timeval32_to_timeval (const struct timeval32 tv)
> }
>
> static inline struct timeval32
> -valid_timeval64_to_timeval (const struct timeval tv64)
> +valid_timeval_to_timeval32 (const struct timeval tv64)
> {
> if (__glibc_unlikely (tv64.tv_sec > (time_t) INT32_MAX))
> return (struct timeval32) { INT32_MAX, TV_USEC_MAX};
> @@ -103,8 +103,8 @@ rusage64_to_rusage32 (struct rusage32 *restrict r32,
> padding and reserved fields. */
> memset (r32, 0, sizeof *r32);
>
> - r32->ru_utime = valid_timeval64_to_timeval (r64->ru_utime);
> - r32->ru_stime = valid_timeval64_to_timeval (r64->ru_stime);
> + r32->ru_utime = valid_timeval_to_timeval32 (r64->ru_utime);
> + r32->ru_stime = valid_timeval_to_timeval32 (r64->ru_stime);
> r32->ru_maxrss = r64->ru_maxrss;
> r32->ru_ixrss = r64->ru_ixrss;
> r32->ru_idrss = r64->ru_idrss;
>
Ok.
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH v3 6/7] y2038: linux: Provide __settimeofday64 implementation
2020-01-29 12:59 [PATCH v3 1/7] y2038: Define __suseconds64_t type to be used with struct __timeval64 Lukasz Majewski
` (3 preceding siblings ...)
2020-01-29 12:59 ` [PATCH v3 4/7] y2038: alpha: Rename valid_timeval64_to_timeval to valid_timeval_to_timeval32 Lukasz Majewski
@ 2020-01-29 12:59 ` Lukasz Majewski
2020-02-04 18:52 ` Adhemerval Zanella
2020-01-29 15:52 ` [PATCH v3 3/7] y2038: alpha: Rename valid_timeval_to_timeval64 to valid_timeval32_to_timeval Lukasz Majewski
` (2 subsequent siblings)
7 siblings, 1 reply; 31+ messages in thread
From: Lukasz Majewski @ 2020-01-29 12:59 UTC (permalink / raw)
To: Joseph Myers, Paul Eggert, Adhemerval Zanella
Cc: Alistair Francis, Alistair Francis, GNU C Library,
Siddhesh Poyarekar, Florian Weimer, Florian Weimer,
Zack Weinberg, Carlos O'Donell, Andreas Schwab,
Lukasz Majewski
This patch provides new __settimeofday64 explicit 64 bit function for setting
64 bit time in the kernel (by internally calling __clock_settime64).
Moreover, a 32 bit version - __settimeofday has been refactored to internally
use __settimeofday64.
The __settimeofday is now supposed to be used on systems still supporting 32
bit time (__TIMESIZE != 64) - hence the necessary conversion of struct
timeval to 64 bit struct __timespec64.
Internally the settimeofday uses __settimeofday64. This patch is necessary
for having architectures with __WORDSIZE == 32 Y2038 safe.
Build tests:
./src/scripts/build-many-glibcs.py glibcs
Run-time tests:
- Run specific tests on ARM/x86 32bit systems (qemu):
https://github.com/lmajewski/meta-y2038 and run tests:
https://github.com/lmajewski/y2038-tests/commits/master
Above tests were performed with Y2038 redirection applied as well as without
to test proper usage of both __settimeofday64 and __settimeofday.
---
Changes for v3:
- None
Changes for v2:
- The conversion to support 64 bit time for settimeofday() has been moved
from ./time/settimeofday.c to sysdeps/unix/sysv/linux/settimeofday.c
(as suggested by Adhemerval) to avoid the need to create __clock_settime64()
method for HURD (as 64 bit time support for machines with __WORDSIZE=32
and __TIMESIZE=32 is now developed solely for Linux).
---
include/time.h | 9 +++++
sysdeps/unix/sysv/linux/settimeofday.c | 53 ++++++++++++++++++++++++++
2 files changed, 62 insertions(+)
create mode 100644 sysdeps/unix/sysv/linux/settimeofday.c
diff --git a/include/time.h b/include/time.h
index 8617114052..0345803db2 100644
--- a/include/time.h
+++ b/include/time.h
@@ -8,6 +8,7 @@
# include <time/mktime-internal.h>
# include <endian.h>
# include <time-clockid.h>
+# include <sys/time.h>
extern __typeof (strftime_l) __strftime_l;
libc_hidden_proto (__strftime_l)
@@ -224,6 +225,14 @@ extern int __sched_rr_get_interval64 (pid_t pid, struct __timespec64 *tp);
libc_hidden_proto (__sched_rr_get_interval64);
#endif
+#if __TIMESIZE == 64
+# define __settimeofday64 __settimeofday
+#else
+extern int __settimeofday64 (const struct __timeval64 *tv,
+ const struct timezone *tz);
+libc_hidden_proto (__settimeofday64)
+#endif
+
/* Compute the `struct tm' representation of T,
offset OFFSET seconds east of UTC,
and store year, yday, mon, mday, wday, hour, min, sec into *TP.
diff --git a/sysdeps/unix/sysv/linux/settimeofday.c b/sysdeps/unix/sysv/linux/settimeofday.c
new file mode 100644
index 0000000000..ea45f1d7cb
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/settimeofday.c
@@ -0,0 +1,53 @@
+/* settimeofday -- set system time - Linux version supporting 64 bit time.
+ 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 <errno.h>
+#include <time.h>
+#include <sys/time.h>
+
+/* Set the current time of day and timezone information.
+ This call is restricted to the super-user. */
+int
+__settimeofday64 (const struct __timeval64 *tv, const struct timezone *tz)
+{
+ if (__glibc_unlikely (tz != 0))
+ {
+ if (tv != 0)
+ {
+ __set_errno (EINVAL);
+ return -1;
+ }
+ return __settimezone (tz);
+ }
+
+ struct __timespec64 ts = timeval64_to_timespec64 (*tv);
+ return __clock_settime64 (CLOCK_REALTIME, &ts);
+}
+
+#if __TIMESIZE != 64
+libc_hidden_def (__settimeofday64)
+
+int
+__settimeofday (const struct timeval *tv, const struct timezone *tz)
+{
+ struct __timeval64 tv64 = valid_timeval_to_timeval64 (*tv);
+
+ return __settimeofday64 (&tv64, tz);
+}
+#endif
+weak_alias (__settimeofday, settimeofday);
--
2.20.1
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH v3 6/7] y2038: linux: Provide __settimeofday64 implementation
2020-01-29 12:59 ` [PATCH v3 6/7] y2038: linux: Provide __settimeofday64 implementation Lukasz Majewski
@ 2020-02-04 18:52 ` Adhemerval Zanella
0 siblings, 0 replies; 31+ messages in thread
From: Adhemerval Zanella @ 2020-02-04 18:52 UTC (permalink / raw)
To: Lukasz Majewski, Joseph Myers, Paul Eggert
Cc: Alistair Francis, Alistair Francis, GNU C Library,
Siddhesh Poyarekar, Florian Weimer, Florian Weimer,
Zack Weinberg, Carlos O'Donell, Andreas Schwab
On 29/01/2020 09:59, Lukasz Majewski wrote:
> This patch provides new __settimeofday64 explicit 64 bit function for setting
> 64 bit time in the kernel (by internally calling __clock_settime64).
> Moreover, a 32 bit version - __settimeofday has been refactored to internally
> use __settimeofday64.
>
> The __settimeofday is now supposed to be used on systems still supporting 32
> bit time (__TIMESIZE != 64) - hence the necessary conversion of struct
> timeval to 64 bit struct __timespec64.
>
> Internally the settimeofday uses __settimeofday64. This patch is necessary
> for having architectures with __WORDSIZE == 32 Y2038 safe.
>
> Build tests:
> ./src/scripts/build-many-glibcs.py glibcs
>
> Run-time tests:
> - Run specific tests on ARM/x86 32bit systems (qemu):
> https://github.com/lmajewski/meta-y2038 and run tests:
> https://github.com/lmajewski/y2038-tests/commits/master
>
> Above tests were performed with Y2038 redirection applied as well as without
> to test proper usage of both __settimeofday64 and __settimeofday.
>
LGTM, thanks.
Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
> ---
> Changes for v3:
> - None
>
> Changes for v2:
> - The conversion to support 64 bit time for settimeofday() has been moved
> from ./time/settimeofday.c to sysdeps/unix/sysv/linux/settimeofday.c
> (as suggested by Adhemerval) to avoid the need to create __clock_settime64()
> method for HURD (as 64 bit time support for machines with __WORDSIZE=32
> and __TIMESIZE=32 is now developed solely for Linux).
> ---
> include/time.h | 9 +++++
> sysdeps/unix/sysv/linux/settimeofday.c | 53 ++++++++++++++++++++++++++
> 2 files changed, 62 insertions(+)
> create mode 100644 sysdeps/unix/sysv/linux/settimeofday.c
>
> diff --git a/include/time.h b/include/time.h
> index 8617114052..0345803db2 100644
> --- a/include/time.h
> +++ b/include/time.h
> @@ -8,6 +8,7 @@
> # include <time/mktime-internal.h>
> # include <endian.h>
> # include <time-clockid.h>
> +# include <sys/time.h>
Ok (for settimeofday).
>
> extern __typeof (strftime_l) __strftime_l;
> libc_hidden_proto (__strftime_l)
> @@ -224,6 +225,14 @@ extern int __sched_rr_get_interval64 (pid_t pid, struct __timespec64 *tp);
> libc_hidden_proto (__sched_rr_get_interval64);
> #endif
>
> +#if __TIMESIZE == 64
> +# define __settimeofday64 __settimeofday
> +#else
> +extern int __settimeofday64 (const struct __timeval64 *tv,
> + const struct timezone *tz);
> +libc_hidden_proto (__settimeofday64)
> +#endif
> +
> /* Compute the `struct tm' representation of T,
> offset OFFSET seconds east of UTC,
> and store year, yday, mon, mday, wday, hour, min, sec into *TP.
Ok.
> diff --git a/sysdeps/unix/sysv/linux/settimeofday.c b/sysdeps/unix/sysv/linux/settimeofday.c
> new file mode 100644
> index 0000000000..ea45f1d7cb
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/settimeofday.c
> @@ -0,0 +1,53 @@
> +/* settimeofday -- set system time - Linux version supporting 64 bit time.
> + 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 <errno.h>
> +#include <time.h>
> +#include <sys/time.h>
> +
> +/* Set the current time of day and timezone information.
> + This call is restricted to the super-user. */
> +int
> +__settimeofday64 (const struct __timeval64 *tv, const struct timezone *tz)
> +{
> + if (__glibc_unlikely (tz != 0))
> + {
> + if (tv != 0)
> + {
> + __set_errno (EINVAL);
> + return -1;
> + }
> + return __settimezone (tz);
> + }
> +
> + struct __timespec64 ts = timeval64_to_timespec64 (*tv);
> + return __clock_settime64 (CLOCK_REALTIME, &ts);
> +}
> +
Ok.
> +#if __TIMESIZE != 64
> +libc_hidden_def (__settimeofday64)
> +
> +int
> +__settimeofday (const struct timeval *tv, const struct timezone *tz)
> +{
> + struct __timeval64 tv64 = valid_timeval_to_timeval64 (*tv);
> +
> + return __settimeofday64 (&tv64, tz);
> +}
> +#endif
> +weak_alias (__settimeofday, settimeofday);
>
Ok.
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH v3 3/7] y2038: alpha: Rename valid_timeval_to_timeval64 to valid_timeval32_to_timeval
2020-01-29 12:59 [PATCH v3 1/7] y2038: Define __suseconds64_t type to be used with struct __timeval64 Lukasz Majewski
` (4 preceding siblings ...)
2020-01-29 12:59 ` [PATCH v3 6/7] y2038: linux: Provide __settimeofday64 implementation Lukasz Majewski
@ 2020-01-29 15:52 ` Lukasz Majewski
2020-02-04 18:40 ` Adhemerval Zanella
2020-01-30 18:01 ` [PATCH v3 1/7] y2038: Define __suseconds64_t type to be used with struct __timeval64 Alistair Francis
2020-02-04 18:34 ` Adhemerval Zanella
7 siblings, 1 reply; 31+ messages in thread
From: Lukasz Majewski @ 2020-01-29 15:52 UTC (permalink / raw)
To: Joseph Myers, Paul Eggert, Adhemerval Zanella
Cc: Alistair Francis, Alistair Francis, GNU C Library,
Siddhesh Poyarekar, Florian Weimer, Florian Weimer,
Zack Weinberg, Carlos O'Donell, Andreas Schwab,
Lukasz Majewski
Without this patch the naming convention for functions to convert
struct timeval32 to struct timeval (which supports 64 bit time on Alpha) was
a bit misleading. The name 'valid_timeval_to_timeval64' suggest conversion
of struct timeval to struct __timeval64 (as in ./include/time.h).
As on alpha the struct timeval supports 64 bit time it seems more readable
to emphasis struct timeval32 in the conversion function name.
Hence the helper function naming change to 'valid_timeval32_to_timeval'.
Build tests:
./src/scripts/build-many-glibcs.py glibcs
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
---
Changes for v3:
- None
Changes for v2:
- None
---
sysdeps/unix/sysv/linux/alpha/osf_adjtime.c | 4 ++--
sysdeps/unix/sysv/linux/alpha/osf_setitimer.c | 4 ++--
sysdeps/unix/sysv/linux/alpha/osf_utimes.c | 4 ++--
sysdeps/unix/sysv/linux/alpha/tv32-compat.h | 2 +-
4 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/sysdeps/unix/sysv/linux/alpha/osf_adjtime.c b/sysdeps/unix/sysv/linux/alpha/osf_adjtime.c
index cd864686f6..5ac72e252f 100644
--- a/sysdeps/unix/sysv/linux/alpha/osf_adjtime.c
+++ b/sysdeps/unix/sysv/linux/alpha/osf_adjtime.c
@@ -57,7 +57,7 @@ int
attribute_compat_text_section
__adjtime_tv32 (const struct timeval32 *itv, struct timeval32 *otv)
{
- struct timeval itv64 = valid_timeval_to_timeval64 (*itv);
+ struct timeval itv64 = valid_timeval32_to_timeval (*itv);
struct timeval otv64;
if (__adjtime (&itv64, &otv64) == -1)
@@ -91,7 +91,7 @@ __adjtimex_tv32 (struct timex32 *tx)
tx64.calcnt = tx->calcnt;
tx64.errcnt = tx->errcnt;
tx64.stbcnt = tx->stbcnt;
- tx64.time = valid_timeval_to_timeval64 (tx->time);
+ tx64.time = valid_timeval32_to_timeval (tx->time);
int status = __adjtimex (&tx64);
if (status < 0)
diff --git a/sysdeps/unix/sysv/linux/alpha/osf_setitimer.c b/sysdeps/unix/sysv/linux/alpha/osf_setitimer.c
index 418efbf546..3935d1cfb5 100644
--- a/sysdeps/unix/sysv/linux/alpha/osf_setitimer.c
+++ b/sysdeps/unix/sysv/linux/alpha/osf_setitimer.c
@@ -30,9 +30,9 @@ __setitimer_tv32 (int which, const struct itimerval32 *restrict new_value,
{
struct itimerval new_value_64;
new_value_64.it_interval
- = valid_timeval_to_timeval64 (new_value->it_interval);
+ = valid_timeval32_to_timeval (new_value->it_interval);
new_value_64.it_value
- = valid_timeval_to_timeval64 (new_value->it_value);
+ = valid_timeval32_to_timeval (new_value->it_value);
if (old_value == NULL)
return __setitimer (which, &new_value_64, NULL);
diff --git a/sysdeps/unix/sysv/linux/alpha/osf_utimes.c b/sysdeps/unix/sysv/linux/alpha/osf_utimes.c
index 423c2a8ef2..6c3fad0132 100644
--- a/sysdeps/unix/sysv/linux/alpha/osf_utimes.c
+++ b/sysdeps/unix/sysv/linux/alpha/osf_utimes.c
@@ -28,8 +28,8 @@ attribute_compat_text_section
__utimes_tv32 (const char *filename, const struct timeval32 times32[2])
{
struct timeval times[2];
- times[0] = valid_timeval_to_timeval64 (times32[0]);
- times[1] = valid_timeval_to_timeval64 (times32[1]);
+ times[0] = valid_timeval32_to_timeval (times32[0]);
+ times[1] = valid_timeval32_to_timeval (times32[1]);
return __utimes (filename, times);
}
diff --git a/sysdeps/unix/sysv/linux/alpha/tv32-compat.h b/sysdeps/unix/sysv/linux/alpha/tv32-compat.h
index 6076d2ec05..7169909259 100644
--- a/sysdeps/unix/sysv/linux/alpha/tv32-compat.h
+++ b/sysdeps/unix/sysv/linux/alpha/tv32-compat.h
@@ -70,7 +70,7 @@ struct rusage32
overflow, they write { INT32_MAX, TV_USEC_MAX } to the output. */
static inline struct timeval
-valid_timeval_to_timeval64 (const struct timeval32 tv)
+valid_timeval32_to_timeval (const struct timeval32 tv)
{
return (struct timeval) { tv.tv_sec, tv.tv_usec };
}
--
2.20.1
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH v3 3/7] y2038: alpha: Rename valid_timeval_to_timeval64 to valid_timeval32_to_timeval
2020-01-29 15:52 ` [PATCH v3 3/7] y2038: alpha: Rename valid_timeval_to_timeval64 to valid_timeval32_to_timeval Lukasz Majewski
@ 2020-02-04 18:40 ` Adhemerval Zanella
2020-02-04 22:50 ` Lukasz Majewski
0 siblings, 1 reply; 31+ messages in thread
From: Adhemerval Zanella @ 2020-02-04 18:40 UTC (permalink / raw)
To: Lukasz Majewski, Joseph Myers, Paul Eggert
Cc: Alistair Francis, Alistair Francis, GNU C Library,
Siddhesh Poyarekar, Florian Weimer, Florian Weimer,
Zack Weinberg, Carlos O'Donell, Andreas Schwab
On 29/01/2020 09:59, Lukasz Majewski wrote:
> Without this patch the naming convention for functions to convert
> struct timeval32 to struct timeval (which supports 64 bit time on Alpha) was
> a bit misleading. The name 'valid_timeval_to_timeval64' suggest conversion
> of struct timeval to struct __timeval64 (as in ./include/time.h).
>
> As on alpha the struct timeval supports 64 bit time it seems more readable
> to emphasis struct timeval32 in the conversion function name.
>
> Hence the helper function naming change to 'valid_timeval32_to_timeval'.
>
> Build tests:
> ./src/scripts/build-many-glibcs.py glibcs
>
> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Usually I don't see much gain in such changes. It the current name
clashing with a new proposed internal symbol?
Anyway, it seems fine though.
Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
>
> ---
> Changes for v3:
> - None
>
> Changes for v2:
> - None
> ---
> sysdeps/unix/sysv/linux/alpha/osf_adjtime.c | 4 ++--
> sysdeps/unix/sysv/linux/alpha/osf_setitimer.c | 4 ++--
> sysdeps/unix/sysv/linux/alpha/osf_utimes.c | 4 ++--
> sysdeps/unix/sysv/linux/alpha/tv32-compat.h | 2 +-
> 4 files changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/sysdeps/unix/sysv/linux/alpha/osf_adjtime.c b/sysdeps/unix/sysv/linux/alpha/osf_adjtime.c
> index cd864686f6..5ac72e252f 100644
> --- a/sysdeps/unix/sysv/linux/alpha/osf_adjtime.c
> +++ b/sysdeps/unix/sysv/linux/alpha/osf_adjtime.c
> @@ -57,7 +57,7 @@ int
> attribute_compat_text_section
> __adjtime_tv32 (const struct timeval32 *itv, struct timeval32 *otv)
> {
> - struct timeval itv64 = valid_timeval_to_timeval64 (*itv);
> + struct timeval itv64 = valid_timeval32_to_timeval (*itv);
> struct timeval otv64;
>
> if (__adjtime (&itv64, &otv64) == -1)
> @@ -91,7 +91,7 @@ __adjtimex_tv32 (struct timex32 *tx)
> tx64.calcnt = tx->calcnt;
> tx64.errcnt = tx->errcnt;
> tx64.stbcnt = tx->stbcnt;
> - tx64.time = valid_timeval_to_timeval64 (tx->time);
> + tx64.time = valid_timeval32_to_timeval (tx->time);
>
> int status = __adjtimex (&tx64);
> if (status < 0)
Ok.
> diff --git a/sysdeps/unix/sysv/linux/alpha/osf_setitimer.c b/sysdeps/unix/sysv/linux/alpha/osf_setitimer.c
> index 418efbf546..3935d1cfb5 100644
> --- a/sysdeps/unix/sysv/linux/alpha/osf_setitimer.c
> +++ b/sysdeps/unix/sysv/linux/alpha/osf_setitimer.c
> @@ -30,9 +30,9 @@ __setitimer_tv32 (int which, const struct itimerval32 *restrict new_value,
> {
> struct itimerval new_value_64;
> new_value_64.it_interval
> - = valid_timeval_to_timeval64 (new_value->it_interval);
> + = valid_timeval32_to_timeval (new_value->it_interval);
> new_value_64.it_value
> - = valid_timeval_to_timeval64 (new_value->it_value);
> + = valid_timeval32_to_timeval (new_value->it_value);
>
> if (old_value == NULL)
> return __setitimer (which, &new_value_64, NULL);
Ok.
> diff --git a/sysdeps/unix/sysv/linux/alpha/osf_utimes.c b/sysdeps/unix/sysv/linux/alpha/osf_utimes.c
> index 423c2a8ef2..6c3fad0132 100644
> --- a/sysdeps/unix/sysv/linux/alpha/osf_utimes.c
> +++ b/sysdeps/unix/sysv/linux/alpha/osf_utimes.c
> @@ -28,8 +28,8 @@ attribute_compat_text_section
> __utimes_tv32 (const char *filename, const struct timeval32 times32[2])
> {
> struct timeval times[2];
> - times[0] = valid_timeval_to_timeval64 (times32[0]);
> - times[1] = valid_timeval_to_timeval64 (times32[1]);
> + times[0] = valid_timeval32_to_timeval (times32[0]);
> + times[1] = valid_timeval32_to_timeval (times32[1]);
> return __utimes (filename, times);
> }
>
Ok.
> diff --git a/sysdeps/unix/sysv/linux/alpha/tv32-compat.h b/sysdeps/unix/sysv/linux/alpha/tv32-compat.h
> index 6076d2ec05..7169909259 100644
> --- a/sysdeps/unix/sysv/linux/alpha/tv32-compat.h
> +++ b/sysdeps/unix/sysv/linux/alpha/tv32-compat.h
> @@ -70,7 +70,7 @@ struct rusage32
> overflow, they write { INT32_MAX, TV_USEC_MAX } to the output. */
>
> static inline struct timeval
> -valid_timeval_to_timeval64 (const struct timeval32 tv)
> +valid_timeval32_to_timeval (const struct timeval32 tv)
> {
> return (struct timeval) { tv.tv_sec, tv.tv_usec };
> }
>
Ok.
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH v3 3/7] y2038: alpha: Rename valid_timeval_to_timeval64 to valid_timeval32_to_timeval
2020-02-04 18:40 ` Adhemerval Zanella
@ 2020-02-04 22:50 ` Lukasz Majewski
2020-02-05 16:11 ` Adhemerval Zanella
0 siblings, 1 reply; 31+ messages in thread
From: Lukasz Majewski @ 2020-02-04 22:50 UTC (permalink / raw)
To: Adhemerval Zanella
Cc: Joseph Myers, Paul Eggert, Alistair Francis, Alistair Francis,
GNU C Library, Siddhesh Poyarekar, Florian Weimer,
Florian Weimer, Zack Weinberg, Carlos O'Donell,
Andreas Schwab
[-- Attachment #1: Type: text/plain, Size: 5587 bytes --]
Hi Adhemerval,
> On 29/01/2020 09:59, Lukasz Majewski wrote:
> > Without this patch the naming convention for functions to convert
> > struct timeval32 to struct timeval (which supports 64 bit time on
> > Alpha) was a bit misleading. The name 'valid_timeval_to_timeval64'
> > suggest conversion of struct timeval to struct __timeval64 (as in
> > ./include/time.h).
> >
> > As on alpha the struct timeval supports 64 bit time it seems more
> > readable to emphasis struct timeval32 in the conversion function
> > name.
> >
> > Hence the helper function naming change to
> > 'valid_timeval32_to_timeval'.
> >
> > Build tests:
> > ./src/scripts/build-many-glibcs.py glibcs
> >
> > Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
>
> Usually I don't see much gain in such changes. It the current name
> clashing with a new proposed internal symbol?
The problem here is that the name is a bit misleading, as alpha's
struct timeval has 64 bit tv.sec (alpha generally supports 64 bit time).
The rename here is to emphasis that we do convert "natural" (for alpha)
struct timeval to struct timeval32 (which is needed when passing the
pointer to syscalls).
In that way the 'valid_timeval_to_timeval64' can follow the pattern,
which we do have now in ./include/time.h.
(it is just to make the naming convention more consistent)
Just to point out - Alistair in his patch set (prepared on top of this
one):
https://patchwork.ozlabs.org/patch/1232967/
took one step further and added "alpha_" prefix to those names
(converted in this patch).
>
> Anyway, it seems fine though.
>
> Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
>
> >
> > ---
> > Changes for v3:
> > - None
> >
> > Changes for v2:
> > - None
> > ---
> > sysdeps/unix/sysv/linux/alpha/osf_adjtime.c | 4 ++--
> > sysdeps/unix/sysv/linux/alpha/osf_setitimer.c | 4 ++--
> > sysdeps/unix/sysv/linux/alpha/osf_utimes.c | 4 ++--
> > sysdeps/unix/sysv/linux/alpha/tv32-compat.h | 2 +-
> > 4 files changed, 7 insertions(+), 7 deletions(-)
> >
> > diff --git a/sysdeps/unix/sysv/linux/alpha/osf_adjtime.c
> > b/sysdeps/unix/sysv/linux/alpha/osf_adjtime.c index
> > cd864686f6..5ac72e252f 100644 ---
> > a/sysdeps/unix/sysv/linux/alpha/osf_adjtime.c +++
> > b/sysdeps/unix/sysv/linux/alpha/osf_adjtime.c @@ -57,7 +57,7 @@ int
> > attribute_compat_text_section
> > __adjtime_tv32 (const struct timeval32 *itv, struct timeval32 *otv)
> > {
> > - struct timeval itv64 = valid_timeval_to_timeval64 (*itv);
> > + struct timeval itv64 = valid_timeval32_to_timeval (*itv);
> > struct timeval otv64;
> >
> > if (__adjtime (&itv64, &otv64) == -1)
> > @@ -91,7 +91,7 @@ __adjtimex_tv32 (struct timex32 *tx)
> > tx64.calcnt = tx->calcnt;
> > tx64.errcnt = tx->errcnt;
> > tx64.stbcnt = tx->stbcnt;
> > - tx64.time = valid_timeval_to_timeval64 (tx->time);
> > + tx64.time = valid_timeval32_to_timeval (tx->time);
> >
> > int status = __adjtimex (&tx64);
> > if (status < 0)
>
> Ok.
>
> > diff --git a/sysdeps/unix/sysv/linux/alpha/osf_setitimer.c
> > b/sysdeps/unix/sysv/linux/alpha/osf_setitimer.c index
> > 418efbf546..3935d1cfb5 100644 ---
> > a/sysdeps/unix/sysv/linux/alpha/osf_setitimer.c +++
> > b/sysdeps/unix/sysv/linux/alpha/osf_setitimer.c @@ -30,9 +30,9 @@
> > __setitimer_tv32 (int which, const struct itimerval32 *restrict
> > new_value, { struct itimerval new_value_64;
> > new_value_64.it_interval
> > - = valid_timeval_to_timeval64 (new_value->it_interval);
> > + = valid_timeval32_to_timeval (new_value->it_interval);
> > new_value_64.it_value
> > - = valid_timeval_to_timeval64 (new_value->it_value);
> > + = valid_timeval32_to_timeval (new_value->it_value);
> >
> > if (old_value == NULL)
> > return __setitimer (which, &new_value_64, NULL);
>
> Ok.
>
> > diff --git a/sysdeps/unix/sysv/linux/alpha/osf_utimes.c
> > b/sysdeps/unix/sysv/linux/alpha/osf_utimes.c index
> > 423c2a8ef2..6c3fad0132 100644 ---
> > a/sysdeps/unix/sysv/linux/alpha/osf_utimes.c +++
> > b/sysdeps/unix/sysv/linux/alpha/osf_utimes.c @@ -28,8 +28,8 @@
> > attribute_compat_text_section __utimes_tv32 (const char *filename,
> > const struct timeval32 times32[2]) {
> > struct timeval times[2];
> > - times[0] = valid_timeval_to_timeval64 (times32[0]);
> > - times[1] = valid_timeval_to_timeval64 (times32[1]);
> > + times[0] = valid_timeval32_to_timeval (times32[0]);
> > + times[1] = valid_timeval32_to_timeval (times32[1]);
> > return __utimes (filename, times);
> > }
> >
>
> Ok.
>
> > diff --git a/sysdeps/unix/sysv/linux/alpha/tv32-compat.h
> > b/sysdeps/unix/sysv/linux/alpha/tv32-compat.h index
> > 6076d2ec05..7169909259 100644 ---
> > a/sysdeps/unix/sysv/linux/alpha/tv32-compat.h +++
> > b/sysdeps/unix/sysv/linux/alpha/tv32-compat.h @@ -70,7 +70,7 @@
> > struct rusage32 overflow, they write { INT32_MAX, TV_USEC_MAX } to
> > the output. */
> > static inline struct timeval
> > -valid_timeval_to_timeval64 (const struct timeval32 tv)
> > +valid_timeval32_to_timeval (const struct timeval32 tv)
> > {
> > return (struct timeval) { tv.tv_sec, tv.tv_usec };
> > }
> >
>
> Ok.
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 --]
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH v3 3/7] y2038: alpha: Rename valid_timeval_to_timeval64 to valid_timeval32_to_timeval
2020-02-04 22:50 ` Lukasz Majewski
@ 2020-02-05 16:11 ` Adhemerval Zanella
0 siblings, 0 replies; 31+ messages in thread
From: Adhemerval Zanella @ 2020-02-05 16:11 UTC (permalink / raw)
To: Lukasz Majewski
Cc: Joseph Myers, Paul Eggert, Alistair Francis, Alistair Francis,
GNU C Library, Siddhesh Poyarekar, Florian Weimer,
Florian Weimer, Zack Weinberg, Carlos O'Donell,
Andreas Schwab
On 04/02/2020 19:50, Lukasz Majewski wrote:
> Hi Adhemerval,
>
>> On 29/01/2020 09:59, Lukasz Majewski wrote:
>>> Without this patch the naming convention for functions to convert
>>> struct timeval32 to struct timeval (which supports 64 bit time on
>>> Alpha) was a bit misleading. The name 'valid_timeval_to_timeval64'
>>> suggest conversion of struct timeval to struct __timeval64 (as in
>>> ./include/time.h).
>>>
>>> As on alpha the struct timeval supports 64 bit time it seems more
>>> readable to emphasis struct timeval32 in the conversion function
>>> name.
>>>
>>> Hence the helper function naming change to
>>> 'valid_timeval32_to_timeval'.
>>>
>>> Build tests:
>>> ./src/scripts/build-many-glibcs.py glibcs
>>>
>>> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
>>
>> Usually I don't see much gain in such changes. It the current name
>> clashing with a new proposed internal symbol?
>
> The problem here is that the name is a bit misleading, as alpha's
> struct timeval has 64 bit tv.sec (alpha generally supports 64 bit time).
>
> The rename here is to emphasis that we do convert "natural" (for alpha)
> struct timeval to struct timeval32 (which is needed when passing the
> pointer to syscalls).
>
> In that way the 'valid_timeval_to_timeval64' can follow the pattern,
> which we do have now in ./include/time.h.
>
> (it is just to make the naming convention more consistent)
I understand it and Iam not really against it in fact, it is usually I see
refactoring more profitable when it simplifies the resulting code by either
removing redundancy or adhering to newer code practices.
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v3 1/7] y2038: Define __suseconds64_t type to be used with struct __timeval64
2020-01-29 12:59 [PATCH v3 1/7] y2038: Define __suseconds64_t type to be used with struct __timeval64 Lukasz Majewski
` (5 preceding siblings ...)
2020-01-29 15:52 ` [PATCH v3 3/7] y2038: alpha: Rename valid_timeval_to_timeval64 to valid_timeval32_to_timeval Lukasz Majewski
@ 2020-01-30 18:01 ` Alistair Francis
2020-02-04 18:34 ` Adhemerval Zanella
7 siblings, 0 replies; 31+ messages in thread
From: Alistair Francis @ 2020-01-30 18:01 UTC (permalink / raw)
To: Lukasz Majewski
Cc: Joseph Myers, Paul Eggert, Adhemerval Zanella, Alistair Francis,
GNU C Library, Siddhesh Poyarekar, Florian Weimer,
Florian Weimer, Zack Weinberg, Carlos O'Donell,
Andreas Schwab
On Wed, Jan 29, 2020 at 4:59 AM Lukasz Majewski <lukma@denx.de> wrote:
>
> The __suseconds64_t type is supposed to be the 64 bit type across all
> architectures.
>
> It would be mostly used internally in the glibc - however, when passed to
> Linux kernel (very unlikely), if necessary, it shall be converted to 32
> bit type (i.e. __suseconds_t)
>
> Build tests:
> ./src/scripts/build-many-glibcs.py glibcs
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Alistair
>
> ---
> Changes for v3:
> - Fix indentation (from spaces to tab) for
> #define __SUSECONDS64_T_TYPE __SQUAD_TYPE
>
> Changes for v2:
> - New patch
> ---
> bits/typesizes.h | 1 +
> posix/bits/types.h | 1 +
> sysdeps/mach/hurd/bits/typesizes.h | 1 +
> sysdeps/unix/sysv/linux/alpha/bits/typesizes.h | 1 +
> sysdeps/unix/sysv/linux/generic/bits/typesizes.h | 1 +
> sysdeps/unix/sysv/linux/s390/bits/typesizes.h | 1 +
> sysdeps/unix/sysv/linux/sparc/bits/typesizes.h | 1 +
> sysdeps/unix/sysv/linux/x86/bits/typesizes.h | 1 +
> 8 files changed, 8 insertions(+)
>
> diff --git a/bits/typesizes.h b/bits/typesizes.h
> index 014c9aab21..599408973e 100644
> --- a/bits/typesizes.h
> +++ b/bits/typesizes.h
> @@ -50,6 +50,7 @@
> #define __TIME_T_TYPE __SLONGWORD_TYPE
> #define __USECONDS_T_TYPE __U32_TYPE
> #define __SUSECONDS_T_TYPE __SLONGWORD_TYPE
> +#define __SUSECONDS64_T_TYPE __SQUAD_TYPE
> #define __DADDR_T_TYPE __S32_TYPE
> #define __KEY_T_TYPE __S32_TYPE
> #define __CLOCKID_T_TYPE __S32_TYPE
> diff --git a/posix/bits/types.h b/posix/bits/types.h
> index adba926b45..a26cd383e4 100644
> --- a/posix/bits/types.h
> +++ b/posix/bits/types.h
> @@ -160,6 +160,7 @@ __STD_TYPE __ID_T_TYPE __id_t; /* General type for IDs. */
> __STD_TYPE __TIME_T_TYPE __time_t; /* Seconds since the Epoch. */
> __STD_TYPE __USECONDS_T_TYPE __useconds_t; /* Count of microseconds. */
> __STD_TYPE __SUSECONDS_T_TYPE __suseconds_t; /* Signed count of microseconds. */
> +__STD_TYPE __SUSECONDS64_T_TYPE __suseconds64_t;
>
> __STD_TYPE __DADDR_T_TYPE __daddr_t; /* The type of a disk address. */
> __STD_TYPE __KEY_T_TYPE __key_t; /* Type of an IPC key. */
> diff --git a/sysdeps/mach/hurd/bits/typesizes.h b/sysdeps/mach/hurd/bits/typesizes.h
> index b429379d7d..10f3ac231a 100644
> --- a/sysdeps/mach/hurd/bits/typesizes.h
> +++ b/sysdeps/mach/hurd/bits/typesizes.h
> @@ -50,6 +50,7 @@
> #define __TIME_T_TYPE __SLONGWORD_TYPE
> #define __USECONDS_T_TYPE __U32_TYPE
> #define __SUSECONDS_T_TYPE __SLONGWORD_TYPE
> +#define __SUSECONDS64_T_TYPE __SQUAD_TYPE
> #define __DADDR_T_TYPE __S32_TYPE
> #define __KEY_T_TYPE __S32_TYPE
> #define __CLOCKID_T_TYPE __S32_TYPE
> diff --git a/sysdeps/unix/sysv/linux/alpha/bits/typesizes.h b/sysdeps/unix/sysv/linux/alpha/bits/typesizes.h
> index 30356ba6d6..489e5d12e2 100644
> --- a/sysdeps/unix/sysv/linux/alpha/bits/typesizes.h
> +++ b/sysdeps/unix/sysv/linux/alpha/bits/typesizes.h
> @@ -49,6 +49,7 @@
> #define __TIME_T_TYPE __SLONGWORD_TYPE
> #define __USECONDS_T_TYPE __U32_TYPE
> #define __SUSECONDS_T_TYPE __S64_TYPE
> +#define __SUSECONDS64_T_TYPE __SQUAD_TYPE
> #define __DADDR_T_TYPE __S32_TYPE
> #define __KEY_T_TYPE __S32_TYPE
> #define __CLOCKID_T_TYPE __S32_TYPE
> diff --git a/sysdeps/unix/sysv/linux/generic/bits/typesizes.h b/sysdeps/unix/sysv/linux/generic/bits/typesizes.h
> index a916dea047..7c963e523e 100644
> --- a/sysdeps/unix/sysv/linux/generic/bits/typesizes.h
> +++ b/sysdeps/unix/sysv/linux/generic/bits/typesizes.h
> @@ -51,6 +51,7 @@
> #define __TIME_T_TYPE __SLONGWORD_TYPE
> #define __USECONDS_T_TYPE __U32_TYPE
> #define __SUSECONDS_T_TYPE __SLONGWORD_TYPE
> +#define __SUSECONDS64_T_TYPE __SQUAD_TYPE
> #define __DADDR_T_TYPE __S32_TYPE
> #define __KEY_T_TYPE __S32_TYPE
> #define __CLOCKID_T_TYPE __S32_TYPE
> diff --git a/sysdeps/unix/sysv/linux/s390/bits/typesizes.h b/sysdeps/unix/sysv/linux/s390/bits/typesizes.h
> index 45f70184ea..e775e460bb 100644
> --- a/sysdeps/unix/sysv/linux/s390/bits/typesizes.h
> +++ b/sysdeps/unix/sysv/linux/s390/bits/typesizes.h
> @@ -50,6 +50,7 @@
> #define __TIME_T_TYPE __SLONGWORD_TYPE
> #define __USECONDS_T_TYPE __U32_TYPE
> #define __SUSECONDS_T_TYPE __SLONGWORD_TYPE
> +#define __SUSECONDS64_T_TYPE __SQUAD_TYPE
> #define __DADDR_T_TYPE __S32_TYPE
> #define __KEY_T_TYPE __S32_TYPE
> #define __CLOCKID_T_TYPE __S32_TYPE
> diff --git a/sysdeps/unix/sysv/linux/sparc/bits/typesizes.h b/sysdeps/unix/sysv/linux/sparc/bits/typesizes.h
> index 1f3bbc8002..ac48c23e37 100644
> --- a/sysdeps/unix/sysv/linux/sparc/bits/typesizes.h
> +++ b/sysdeps/unix/sysv/linux/sparc/bits/typesizes.h
> @@ -50,6 +50,7 @@
> #define __TIME_T_TYPE __SLONGWORD_TYPE
> #define __USECONDS_T_TYPE __U32_TYPE
> #define __SUSECONDS_T_TYPE __S32_TYPE
> +#define __SUSECONDS64_T_TYPE __SQUAD_TYPE
> #define __DADDR_T_TYPE __S32_TYPE
> #define __KEY_T_TYPE __S32_TYPE
> #define __CLOCKID_T_TYPE __S32_TYPE
> diff --git a/sysdeps/unix/sysv/linux/x86/bits/typesizes.h b/sysdeps/unix/sysv/linux/x86/bits/typesizes.h
> index d084145597..87c50a4f32 100644
> --- a/sysdeps/unix/sysv/linux/x86/bits/typesizes.h
> +++ b/sysdeps/unix/sysv/linux/x86/bits/typesizes.h
> @@ -64,6 +64,7 @@
> #define __TIME_T_TYPE __SYSCALL_SLONG_TYPE
> #define __USECONDS_T_TYPE __U32_TYPE
> #define __SUSECONDS_T_TYPE __SYSCALL_SLONG_TYPE
> +#define __SUSECONDS64_T_TYPE __SQUAD_TYPE
> #define __DADDR_T_TYPE __S32_TYPE
> #define __KEY_T_TYPE __S32_TYPE
> #define __CLOCKID_T_TYPE __S32_TYPE
> --
> 2.20.1
>
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH v3 1/7] y2038: Define __suseconds64_t type to be used with struct __timeval64
2020-01-29 12:59 [PATCH v3 1/7] y2038: Define __suseconds64_t type to be used with struct __timeval64 Lukasz Majewski
` (6 preceding siblings ...)
2020-01-30 18:01 ` [PATCH v3 1/7] y2038: Define __suseconds64_t type to be used with struct __timeval64 Alistair Francis
@ 2020-02-04 18:34 ` Adhemerval Zanella
7 siblings, 0 replies; 31+ messages in thread
From: Adhemerval Zanella @ 2020-02-04 18:34 UTC (permalink / raw)
To: Lukasz Majewski, Joseph Myers, Paul Eggert
Cc: Alistair Francis, Alistair Francis, GNU C Library,
Siddhesh Poyarekar, Florian Weimer, Florian Weimer,
Zack Weinberg, Carlos O'Donell, Andreas Schwab
On 29/01/2020 09:59, Lukasz Majewski wrote:
> The __suseconds64_t type is supposed to be the 64 bit type across all
> architectures.
>
> It would be mostly used internally in the glibc - however, when passed to
> Linux kernel (very unlikely), if necessary, it shall be converted to 32
> bit type (i.e. __suseconds_t)
>
> Build tests:
> ./src/scripts/build-many-glibcs.py glibcs
>
> ---
> Changes for v3:
> - Fix indentation (from spaces to tab) for
> #define __SUSECONDS64_T_TYPE __SQUAD_TYPE
>
> Changes for v2:
> - New patch
LGTM, with a small nit below.
Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
> ---
> bits/typesizes.h | 1 +
> posix/bits/types.h | 1 +
> sysdeps/mach/hurd/bits/typesizes.h | 1 +
> sysdeps/unix/sysv/linux/alpha/bits/typesizes.h | 1 +
> sysdeps/unix/sysv/linux/generic/bits/typesizes.h | 1 +
> sysdeps/unix/sysv/linux/s390/bits/typesizes.h | 1 +
> sysdeps/unix/sysv/linux/sparc/bits/typesizes.h | 1 +
> sysdeps/unix/sysv/linux/x86/bits/typesizes.h | 1 +
> 8 files changed, 8 insertions(+)
>
> diff --git a/bits/typesizes.h b/bits/typesizes.h
> index 014c9aab21..599408973e 100644
> --- a/bits/typesizes.h
> +++ b/bits/typesizes.h
> @@ -50,6 +50,7 @@
> #define __TIME_T_TYPE __SLONGWORD_TYPE
> #define __USECONDS_T_TYPE __U32_TYPE
> #define __SUSECONDS_T_TYPE __SLONGWORD_TYPE
> +#define __SUSECONDS64_T_TYPE __SQUAD_TYPE
> #define __DADDR_T_TYPE __S32_TYPE
> #define __KEY_T_TYPE __S32_TYPE
> #define __CLOCKID_T_TYPE __S32_TYPE
Ok.
> diff --git a/posix/bits/types.h b/posix/bits/types.h
> index adba926b45..a26cd383e4 100644
> --- a/posix/bits/types.h
> +++ b/posix/bits/types.h
> @@ -160,6 +160,7 @@ __STD_TYPE __ID_T_TYPE __id_t; /* General type for IDs. */
> __STD_TYPE __TIME_T_TYPE __time_t; /* Seconds since the Epoch. */
> __STD_TYPE __USECONDS_T_TYPE __useconds_t; /* Count of microseconds. */
> __STD_TYPE __SUSECONDS_T_TYPE __suseconds_t; /* Signed count of microseconds. */
> +__STD_TYPE __SUSECONDS64_T_TYPE __suseconds64_t;
>
> __STD_TYPE __DADDR_T_TYPE __daddr_t; /* The type of a disk address. */
> __STD_TYPE __KEY_T_TYPE __key_t; /* Type of an IPC key. */
Ok.
> diff --git a/sysdeps/mach/hurd/bits/typesizes.h b/sysdeps/mach/hurd/bits/typesizes.h
> index b429379d7d..10f3ac231a 100644
> --- a/sysdeps/mach/hurd/bits/typesizes.h
> +++ b/sysdeps/mach/hurd/bits/typesizes.h
> @@ -50,6 +50,7 @@
> #define __TIME_T_TYPE __SLONGWORD_TYPE
> #define __USECONDS_T_TYPE __U32_TYPE
> #define __SUSECONDS_T_TYPE __SLONGWORD_TYPE
> +#define __SUSECONDS64_T_TYPE __SQUAD_TYPE
> #define __DADDR_T_TYPE __S32_TYPE
> #define __KEY_T_TYPE __S32_TYPE
> #define __CLOCKID_T_TYPE __S32_TYPE
Ok.
> diff --git a/sysdeps/unix/sysv/linux/alpha/bits/typesizes.h b/sysdeps/unix/sysv/linux/alpha/bits/typesizes.h
> index 30356ba6d6..489e5d12e2 100644
> --- a/sysdeps/unix/sysv/linux/alpha/bits/typesizes.h
> +++ b/sysdeps/unix/sysv/linux/alpha/bits/typesizes.h
> @@ -49,6 +49,7 @@
> #define __TIME_T_TYPE __SLONGWORD_TYPE
> #define __USECONDS_T_TYPE __U32_TYPE
> #define __SUSECONDS_T_TYPE __S64_TYPE
> +#define __SUSECONDS64_T_TYPE __SQUAD_TYPE
> #define __DADDR_T_TYPE __S32_TYPE
> #define __KEY_T_TYPE __S32_TYPE
> #define __CLOCKID_T_TYPE __S32_TYPE
I think it should use __S64_TYPE to follow current file convention.
> diff --git a/sysdeps/unix/sysv/linux/generic/bits/typesizes.h b/sysdeps/unix/sysv/linux/generic/bits/typesizes.h
> index a916dea047..7c963e523e 100644
> --- a/sysdeps/unix/sysv/linux/generic/bits/typesizes.h
> +++ b/sysdeps/unix/sysv/linux/generic/bits/typesizes.h
> @@ -51,6 +51,7 @@
> #define __TIME_T_TYPE __SLONGWORD_TYPE
> #define __USECONDS_T_TYPE __U32_TYPE
> #define __SUSECONDS_T_TYPE __SLONGWORD_TYPE
> +#define __SUSECONDS64_T_TYPE __SQUAD_TYPE
> #define __DADDR_T_TYPE __S32_TYPE
> #define __KEY_T_TYPE __S32_TYPE
> #define __CLOCKID_T_TYPE __S32_TYPE
Ok.
> diff --git a/sysdeps/unix/sysv/linux/s390/bits/typesizes.h b/sysdeps/unix/sysv/linux/s390/bits/typesizes.h
> index 45f70184ea..e775e460bb 100644
> --- a/sysdeps/unix/sysv/linux/s390/bits/typesizes.h
> +++ b/sysdeps/unix/sysv/linux/s390/bits/typesizes.h
> @@ -50,6 +50,7 @@
> #define __TIME_T_TYPE __SLONGWORD_TYPE
> #define __USECONDS_T_TYPE __U32_TYPE
> #define __SUSECONDS_T_TYPE __SLONGWORD_TYPE
> +#define __SUSECONDS64_T_TYPE __SQUAD_TYPE
> #define __DADDR_T_TYPE __S32_TYPE
> #define __KEY_T_TYPE __S32_TYPE
> #define __CLOCKID_T_TYPE __S32_TYPE
Ok.
> diff --git a/sysdeps/unix/sysv/linux/sparc/bits/typesizes.h b/sysdeps/unix/sysv/linux/sparc/bits/typesizes.h
> index 1f3bbc8002..ac48c23e37 100644
> --- a/sysdeps/unix/sysv/linux/sparc/bits/typesizes.h
> +++ b/sysdeps/unix/sysv/linux/sparc/bits/typesizes.h
> @@ -50,6 +50,7 @@
> #define __TIME_T_TYPE __SLONGWORD_TYPE
> #define __USECONDS_T_TYPE __U32_TYPE
> #define __SUSECONDS_T_TYPE __S32_TYPE
> +#define __SUSECONDS64_T_TYPE __SQUAD_TYPE
> #define __DADDR_T_TYPE __S32_TYPE
> #define __KEY_T_TYPE __S32_TYPE
> #define __CLOCKID_T_TYPE __S32_TYPE
Ok.
> diff --git a/sysdeps/unix/sysv/linux/x86/bits/typesizes.h b/sysdeps/unix/sysv/linux/x86/bits/typesizes.h
> index d084145597..87c50a4f32 100644
> --- a/sysdeps/unix/sysv/linux/x86/bits/typesizes.h
> +++ b/sysdeps/unix/sysv/linux/x86/bits/typesizes.h
> @@ -64,6 +64,7 @@
> #define __TIME_T_TYPE __SYSCALL_SLONG_TYPE
> #define __USECONDS_T_TYPE __U32_TYPE
> #define __SUSECONDS_T_TYPE __SYSCALL_SLONG_TYPE
> +#define __SUSECONDS64_T_TYPE __SQUAD_TYPE
> #define __DADDR_T_TYPE __S32_TYPE
> #define __KEY_T_TYPE __S32_TYPE
> #define __CLOCKID_T_TYPE __S32_TYPE
>
Ok.
^ permalink raw reply [flat|nested] 31+ messages in thread