public inbox for libc-alpha@sourceware.org
 help / color / mirror / Atom feed
* [PATCH v2 0/5] y2038: Replace __clock_gettime with __clock_gettime64
@ 2020-03-26  8:06 Lukasz Majewski
  2020-03-26  8:06 ` [PATCH v2 1/5] y2038: Export __clock_gettime64 to be usable in other libraries Lukasz Majewski
                   ` (5 more replies)
  0 siblings, 6 replies; 24+ messages in thread
From: Lukasz Majewski @ 2020-03-26  8:06 UTC (permalink / raw)
  To: Joseph Myers, Adhemerval Zanella
  Cc: Alistair Francis, Alistair Francis, GNU C Library,
	Florian Weimer, Andreas Schwab, Lukasz Majewski

This patch series starts the conversion of __clock_gettime calls to
__clock_gettime64 as the latter supports 64 bit time on archs with 
__WORDSIZE == 32 and __TIMESIZE != 64.

On purpose the nptl and pthreads have been omitted, as for those separate patch
sets will be prepared.
It is an open question if sunrpc shall be converted or not.

This patch series also provides __clock_gettime64 wrapper for HURD to allow
conversion of generic in-glibc code (as HURD uses them - like e.g. pthreads).

Special care had to be taken for s390 as it has its own definition of utmp.h
and hence requires using TIMESPEC_TO_TIMEVAL macro.

Lukasz Majewski (5):
  y2038: Export __clock_gettime64 to be usable in other libraries
  y2038: hurd: Provide __clock_gettime64 function
  y2038: inet: Convert inet deadline to support 64 bit time
  y2038: nscd: Modify nscd_helper to use __clock_gettime64
  y2038: Replace __clock_gettime with __clock_gettime64

 benchtests/bench-timing.h                       |  2 +-
 include/random-bits.h                           |  4 ++--
 inet/deadline.c                                 |  4 ++--
 inet/net-internal.h                             |  5 +++--
 login/logout.c                                  |  4 ++--
 login/logwtmp.c                                 |  5 +++--
 nis/nis_call.c                                  |  6 +++---
 nscd/nscd_helper.c                              | 17 +++++++++--------
 sysdeps/generic/hp-timing.h                     |  4 ++--
 sysdeps/generic/memusage.h                      |  4 ++--
 sysdeps/mach/clock_gettime.c                    | 14 ++++++++++++++
 .../unix/sysv/linux/alpha/osf_gettimeofday.c    |  4 ++--
 sysdeps/unix/sysv/linux/clock.c                 |  7 ++-----
 time/Versions                                   |  1 +
 14 files changed, 48 insertions(+), 33 deletions(-)

-- 
2.20.1


^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH v2 1/5] y2038: Export __clock_gettime64 to be usable in other libraries
  2020-03-26  8:06 [PATCH v2 0/5] y2038: Replace __clock_gettime with __clock_gettime64 Lukasz Majewski
@ 2020-03-26  8:06 ` Lukasz Majewski
  2020-03-26  8:06 ` [PATCH v2 2/5] y2038: hurd: Provide __clock_gettime64 function Lukasz Majewski
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 24+ messages in thread
From: Lukasz Majewski @ 2020-03-26  8:06 UTC (permalink / raw)
  To: Joseph Myers, Adhemerval Zanella
  Cc: Alistair Francis, Alistair Francis, GNU C Library,
	Florian Weimer, Andreas Schwab, Lukasz Majewski

In the glibc project calls to clock_gettime shall be replaced with
__clock_gettime64, which is supporting 64 bit time.
To allow that the __clock_gettime64 needs to be exported as a GLIBC_PRIVATE
symbol.
---
 time/Versions | 1 +
 1 file changed, 1 insertion(+)

diff --git a/time/Versions b/time/Versions
index 8788e192ce..df22ac7f6a 100644
--- a/time/Versions
+++ b/time/Versions
@@ -77,5 +77,6 @@ libc {
   GLIBC_PRIVATE {
     # same as clock_gettime; used in other libraries
     __clock_gettime;
+    __clock_gettime64;
   }
 }
-- 
2.20.1


^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH v2 2/5] y2038: hurd: Provide __clock_gettime64 function
  2020-03-26  8:06 [PATCH v2 0/5] y2038: Replace __clock_gettime with __clock_gettime64 Lukasz Majewski
  2020-03-26  8:06 ` [PATCH v2 1/5] y2038: Export __clock_gettime64 to be usable in other libraries Lukasz Majewski
@ 2020-03-26  8:06 ` Lukasz Majewski
  2020-03-26  8:06 ` [PATCH v2 3/5] y2038: inet: Convert inet deadline to support 64 bit time Lukasz Majewski
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 24+ messages in thread
From: Lukasz Majewski @ 2020-03-26  8:06 UTC (permalink / raw)
  To: Joseph Myers, Adhemerval Zanella
  Cc: Alistair Francis, Alistair Francis, GNU C Library,
	Florian Weimer, Andreas Schwab, Lukasz Majewski, Samuel Thibault

For Linux glibc ports the __TIMESIZE == 64 ensures proper aliasing for
__clock_gettime64 (to __clock_gettime).
When __TIMESIZE != 64 (like ARM32, PPC) the glibc expects separate definition
of the __clock_gettime64.

The HURD port only provides __clock_gettime, so this patch adds
__clock_gettime64 as a tiny wrapper on it.
Acked-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
---
 sysdeps/mach/clock_gettime.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/sysdeps/mach/clock_gettime.c b/sysdeps/mach/clock_gettime.c
index ac3547df3c..fbd80536d5 100644
--- a/sysdeps/mach/clock_gettime.c
+++ b/sysdeps/mach/clock_gettime.c
@@ -49,3 +49,17 @@ versioned_symbol (libc, __clock_gettime, clock_gettime, GLIBC_2_17);
 strong_alias (__clock_gettime, __clock_gettime_2);
 compat_symbol (libc, __clock_gettime_2, clock_gettime, GLIBC_2_2);
 #endif
+
+int
+__clock_gettime64 (clockid_t clock_id, struct __timespec64 *ts64)
+{
+  struct timespec ts;
+  int ret;
+
+  ret = __clock_gettime (clock_id, &ts);
+  if (ret == 0)
+    *ts64 = valid_timespec_to_timespec64 (ts);
+
+  return ret;
+}
+libc_hidden_def (__clock_gettime64)
-- 
2.20.1


^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH v2 3/5] y2038: inet: Convert inet deadline to support 64 bit time
  2020-03-26  8:06 [PATCH v2 0/5] y2038: Replace __clock_gettime with __clock_gettime64 Lukasz Majewski
  2020-03-26  8:06 ` [PATCH v2 1/5] y2038: Export __clock_gettime64 to be usable in other libraries Lukasz Majewski
  2020-03-26  8:06 ` [PATCH v2 2/5] y2038: hurd: Provide __clock_gettime64 function Lukasz Majewski
@ 2020-03-26  8:06 ` Lukasz Majewski
  2020-04-30 20:51   ` Adhemerval Zanella
  2020-03-26  8:06 ` [PATCH v2 4/5] y2038: nscd: Modify nscd_helper to use __clock_gettime64 Lukasz Majewski
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 24+ messages in thread
From: Lukasz Majewski @ 2020-03-26  8:06 UTC (permalink / raw)
  To: Joseph Myers, Adhemerval Zanella
  Cc: Alistair Francis, Alistair Francis, GNU C Library,
	Florian Weimer, Andreas Schwab, Lukasz Majewski

This change brings 64 bit time support to inet deadline related code for
architectures with __WORDSIZE == 32 && __TIMESIZE != 64.

It is also safe to replace struct timespec with struct __timespec64 in
deadline related structures as:

- The __deadline_to_ms () returns the number of miliseconds to deadline to
  be used with __poll (and hence it is a relative value).
- To calculate the deadline from timeval (which will be converted latter)
  the uintmax_t type is used (unsinged long long int).
---
 inet/deadline.c     | 4 ++--
 inet/net-internal.h | 5 +++--
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/inet/deadline.c b/inet/deadline.c
index ebf9a4f52c..eac7afd1a8 100644
--- a/inet/deadline.c
+++ b/inet/deadline.c
@@ -28,8 +28,8 @@ struct deadline_current_time
 __deadline_current_time (void)
 {
   struct deadline_current_time result;
-  if (__clock_gettime (CLOCK_MONOTONIC, &result.current) != 0)
-    __clock_gettime (CLOCK_REALTIME, &result.current);
+  if (__clock_gettime64 (CLOCK_MONOTONIC, &result.current) != 0)
+    __clock_gettime64 (CLOCK_REALTIME, &result.current);
   assert (result.current.tv_sec >= 0);
   return result;
 }
diff --git a/inet/net-internal.h b/inet/net-internal.h
index 3ca301a9be..50c7e1c482 100644
--- a/inet/net-internal.h
+++ b/inet/net-internal.h
@@ -24,6 +24,7 @@
 #include <stdint.h>
 #include <sys/time.h>
 #include <libc-diag.h>
+#include <struct___timespec64.h>
 
 int __inet6_scopeid_pton (const struct in6_addr *address,
                           const char *scope, uint32_t *result);
@@ -76,7 +77,7 @@ enum idna_name_classification __idna_name_classify (const char *name)
    timeouts and vice versa.  */
 struct deadline_current_time
 {
-  struct timespec current;
+  struct __timespec64 current;
 };
 
 /* Return the current time.  Terminates the process if the current
@@ -86,7 +87,7 @@ struct deadline_current_time __deadline_current_time (void) attribute_hidden;
 /* Computed absolute deadline.  */
 struct deadline
 {
-  struct timespec absolute;
+  struct __timespec64 absolute;
 };
 
 
-- 
2.20.1


^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH v2 4/5] y2038: nscd: Modify nscd_helper to use __clock_gettime64
  2020-03-26  8:06 [PATCH v2 0/5] y2038: Replace __clock_gettime with __clock_gettime64 Lukasz Majewski
                   ` (2 preceding siblings ...)
  2020-03-26  8:06 ` [PATCH v2 3/5] y2038: inet: Convert inet deadline to support 64 bit time Lukasz Majewski
@ 2020-03-26  8:06 ` Lukasz Majewski
  2020-04-30 20:54   ` Adhemerval Zanella
  2020-03-26  8:06 ` [PATCH v2 5/5] y2038: Replace __clock_gettime with __clock_gettime64 Lukasz Majewski
  2020-04-06 15:16 ` [PATCH v2 0/5] " Lukasz Majewski
  5 siblings, 1 reply; 24+ messages in thread
From: Lukasz Majewski @ 2020-03-26  8:06 UTC (permalink / raw)
  To: Joseph Myers, Adhemerval Zanella
  Cc: Alistair Francis, Alistair Francis, GNU C Library,
	Florian Weimer, Andreas Schwab, Lukasz Majewski

The nscd/nscd_helper.c uses __clock_gettime to get current time and on this
basis calculate the relative timeout for poll.
By using __clock_gettime64 on systems with __WORDSIZE == 32 && __TIMESIZE != 64
the timeout is correctly calculated after time_t overflow.
---
 nscd/nscd_helper.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/nscd/nscd_helper.c b/nscd/nscd_helper.c
index d2d7d15f26..a4f3312f90 100644
--- a/nscd/nscd_helper.c
+++ b/nscd/nscd_helper.c
@@ -37,6 +37,7 @@
 #include <not-cancel.h>
 #include <kernel-features.h>
 #include <nss.h>
+#include <struct___timespec64.h>
 
 #include "nscd-client.h"
 
@@ -59,10 +60,10 @@ wait_on_socket (int sock, long int usectmo)
       /* Handle the case where the poll() call is interrupted by a
 	 signal.  We cannot just use TEMP_FAILURE_RETRY since it might
 	 lead to infinite loops.  */
-      struct timespec now;
-      __clock_gettime (CLOCK_REALTIME, &now);
-      long int end = (now.tv_sec * 1000 + usectmo
-                      + (now.tv_nsec + 500000) / 1000000);
+      struct __timespec64 now;
+      __clock_gettime64 (CLOCK_REALTIME, &now);
+      int64_t end = (now.tv_sec * 1000 + usectmo
+                     + (now.tv_nsec + 500000) / 1000000);
       long int timeout = usectmo;
       while (1)
 	{
@@ -71,7 +72,7 @@ wait_on_socket (int sock, long int usectmo)
 	    break;
 
 	  /* Recompute the timeout time.  */
-          __clock_gettime (CLOCK_REALTIME, &now);
+          __clock_gettime64 (CLOCK_REALTIME, &now);
 	  timeout = end - ((now.tv_sec * 1000
                             + (now.tv_nsec + 500000) / 1000000));
 	}
@@ -193,7 +194,7 @@ open_socket (request_type type, const char *key, size_t keylen)
   memcpy (reqdata->key, key, keylen);
 
   bool first_try = true;
-  struct timespec tvend = { 0, 0 };
+  struct __timespec64 tvend = { 0, 0 };
   while (1)
     {
 #ifndef MSG_NOSIGNAL
@@ -212,8 +213,8 @@ open_socket (request_type type, const char *key, size_t keylen)
 
       /* The daemon is busy wait for it.  */
       int to;
-      struct timespec now;
-      __clock_gettime (CLOCK_REALTIME, &now);
+      struct __timespec64 now;
+      __clock_gettime64 (CLOCK_REALTIME, &now);
       if (first_try)
 	{
 	  tvend.tv_nsec = now.tv_nsec;
-- 
2.20.1


^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH v2 5/5] y2038: Replace __clock_gettime with __clock_gettime64
  2020-03-26  8:06 [PATCH v2 0/5] y2038: Replace __clock_gettime with __clock_gettime64 Lukasz Majewski
                   ` (3 preceding siblings ...)
  2020-03-26  8:06 ` [PATCH v2 4/5] y2038: nscd: Modify nscd_helper to use __clock_gettime64 Lukasz Majewski
@ 2020-03-26  8:06 ` Lukasz Majewski
  2020-04-06 16:07   ` Florian Weimer
  2020-04-30 21:03   ` Adhemerval Zanella
  2020-04-06 15:16 ` [PATCH v2 0/5] " Lukasz Majewski
  5 siblings, 2 replies; 24+ messages in thread
From: Lukasz Majewski @ 2020-03-26  8:06 UTC (permalink / raw)
  To: Joseph Myers, Adhemerval Zanella
  Cc: Alistair Francis, Alistair Francis, GNU C Library,
	Florian Weimer, Andreas Schwab, Lukasz Majewski

The __clock_gettime internal function is not supporting 64 bit time on
architectures with __WORDSIZE == 32 and __TIMESIZE != 64 (like e.g. ARM 32
bit).

The __clock_gettime64 shall be used instead in the glibc itself as it
supports 64 bit time on those systems.
This change does not bring any change to systems with __WORDSIZE == 64 as
for them the __clock_gettime64 is aliased to __clock_gettime
(in ./include/time.h).

---
Changes for v2:
- Use only TIMESPEC_TO_TIMEVAL instead of valid_timespec64_to_timeval in
  logout.c and logwtmp.c as it is generic enough to also support struct
  __timespec64 conversion to struct timeval
---
 benchtests/bench-timing.h                        | 2 +-
 include/random-bits.h                            | 4 ++--
 login/logout.c                                   | 4 ++--
 login/logwtmp.c                                  | 5 +++--
 nis/nis_call.c                                   | 6 +++---
 sysdeps/generic/hp-timing.h                      | 4 ++--
 sysdeps/generic/memusage.h                       | 4 ++--
 sysdeps/unix/sysv/linux/alpha/osf_gettimeofday.c | 4 ++--
 sysdeps/unix/sysv/linux/clock.c                  | 7 ++-----
 9 files changed, 19 insertions(+), 21 deletions(-)

diff --git a/benchtests/bench-timing.h b/benchtests/bench-timing.h
index 5b9a8384bb..a0d6f82465 100644
--- a/benchtests/bench-timing.h
+++ b/benchtests/bench-timing.h
@@ -18,7 +18,7 @@
 
 #undef attribute_hidden
 #define attribute_hidden
-#define __clock_gettime clock_gettime
+#define __clock_gettime __clock_gettime64
 #include <hp-timing.h>
 #include <stdint.h>
 
diff --git a/include/random-bits.h b/include/random-bits.h
index fd3fa01f9b..7561e55ca6 100644
--- a/include/random-bits.h
+++ b/include/random-bits.h
@@ -30,8 +30,8 @@
 static inline uint32_t
 random_bits (void)
 {
-  struct timespec tv;
-  __clock_gettime (CLOCK_MONOTONIC, &tv);
+  struct __timespec64 tv;
+  __clock_gettime64 (CLOCK_MONOTONIC, &tv);
   /* Shuffle the lower bits to minimize the clock bias.  */
   uint32_t ret = tv.tv_nsec ^ tv.tv_sec;
   ret ^= (ret << 24) | (ret >> 8);
diff --git a/login/logout.c b/login/logout.c
index 7653fe8886..091312eb1d 100644
--- a/login/logout.c
+++ b/login/logout.c
@@ -47,8 +47,8 @@ logout (const char *line)
       memset (ut->ut_name, '\0', sizeof ut->ut_name);
       memset (ut->ut_host, '\0', sizeof ut->ut_host);
 
-      struct timespec ts;
-      __clock_gettime (CLOCK_REALTIME, &ts);
+      struct __timespec64 ts;
+      __clock_gettime64 (CLOCK_REALTIME, &ts);
       TIMESPEC_TO_TIMEVAL (&ut->ut_tv, &ts);
       ut->ut_type = DEAD_PROCESS;
 
diff --git a/login/logwtmp.c b/login/logwtmp.c
index 90406acc3d..050219c153 100644
--- a/login/logwtmp.c
+++ b/login/logwtmp.c
@@ -21,6 +21,7 @@
 #include <time.h>
 #include <unistd.h>
 #include <utmp.h>
+#include <struct___timespec64.h>
 
 
 void
@@ -36,8 +37,8 @@ logwtmp (const char *line, const char *name, const char *host)
   strncpy (ut.ut_name, name, sizeof ut.ut_name);
   strncpy (ut.ut_host, host, sizeof ut.ut_host);
 
-  struct timespec ts;
-  __clock_gettime (CLOCK_REALTIME, &ts);
+  struct __timespec64 ts;
+  __clock_gettime64 (CLOCK_REALTIME, &ts);
   TIMESPEC_TO_TIMEVAL (&ut.ut_tv, &ts);
 
   updwtmp (_PATH_WTMP, &ut);
diff --git a/nis/nis_call.c b/nis/nis_call.c
index 92c70e97aa..9c6f62a753 100644
--- a/nis/nis_call.c
+++ b/nis/nis_call.c
@@ -709,7 +709,7 @@ __nisfind_server (const_nis_name name, int search_parent,
   nis_error status;
   directory_obj *obj;
   struct timeval now;
-  struct timespec ts;
+  struct __timespec64 ts;
   unsigned int server_used = ~0;
   unsigned int current_ep = ~0;
 
@@ -719,8 +719,8 @@ __nisfind_server (const_nis_name name, int search_parent,
   if (*dir != NULL)
     return NIS_SUCCESS;
 
-  __clock_gettime (CLOCK_REALTIME, &ts);
-  TIMESPEC_TO_TIMEVAL (&now, &ts);
+  __clock_gettime64 (CLOCK_REALTIME, &ts);
+  now = valid_timespec64_to_timeval (ts);
 
   if ((flags & NO_CACHE) == 0)
     *dir = nis_server_cache_search (name, search_parent, &server_used,
diff --git a/sysdeps/generic/hp-timing.h b/sysdeps/generic/hp-timing.h
index e2d7447212..af9d92f7f7 100644
--- a/sysdeps/generic/hp-timing.h
+++ b/sysdeps/generic/hp-timing.h
@@ -34,8 +34,8 @@ typedef uint64_t hp_timing_t;
    vDSO symbol.  */
 #define HP_TIMING_NOW(var) \
 ({								\
-  struct timespec tv;						\
-  __clock_gettime (CLOCK_MONOTONIC, &tv);			\
+  struct __timespec64 tv;						\
+  __clock_gettime64 (CLOCK_MONOTONIC, &tv);			\
   (var) = (tv.tv_nsec + UINT64_C(1000000000) * tv.tv_sec);	\
 })
 
diff --git a/sysdeps/generic/memusage.h b/sysdeps/generic/memusage.h
index a111864b0b..91e56d24de 100644
--- a/sysdeps/generic/memusage.h
+++ b/sysdeps/generic/memusage.h
@@ -28,9 +28,9 @@
 #ifndef GETTIME
 # define GETTIME(low,high)						   \
   {									   \
-    struct timespec now;						   \
+    struct __timespec64 now;						   \
     uint64_t usecs;							   \
-    clock_gettime (CLOCK_REALTIME, &now);				   \
+    __clock_gettime64 (CLOCK_REALTIME, &now);				   \
     usecs = (uint64_t)now.tv_nsec / 1000 + (uint64_t)now.tv_sec * 1000000; \
     low = usecs & 0xffffffff;						   \
     high = usecs >> 32;							   \
diff --git a/sysdeps/unix/sysv/linux/alpha/osf_gettimeofday.c b/sysdeps/unix/sysv/linux/alpha/osf_gettimeofday.c
index 8cf5d303f9..5075ae0444 100644
--- a/sysdeps/unix/sysv/linux/alpha/osf_gettimeofday.c
+++ b/sysdeps/unix/sysv/linux/alpha/osf_gettimeofday.c
@@ -35,8 +35,8 @@ __gettimeofday_tv32 (struct __timeval32 *restrict tv32, void *restrict tz)
   if (__glibc_unlikely (tz != 0))
     memset (tz, 0, sizeof (struct timezone));
 
-  struct timespec ts;
-  __clock_gettime (CLOCK_REALTIME, &ts);
+  struct __timespec64 ts;
+  __clock_gettime64 (CLOCK_REALTIME, &ts);
 
   *tv32 = valid_timespec_to_timeval32 (ts);
   return 0;
diff --git a/sysdeps/unix/sysv/linux/clock.c b/sysdeps/unix/sysv/linux/clock.c
index 24a8df0cf5..157ae8eb3f 100644
--- a/sysdeps/unix/sysv/linux/clock.c
+++ b/sysdeps/unix/sysv/linux/clock.c
@@ -23,15 +23,12 @@
 clock_t
 clock (void)
 {
-  struct timespec ts;
+  struct __timespec64 ts;
 
   _Static_assert (CLOCKS_PER_SEC == 1000000,
 		  "CLOCKS_PER_SEC should be 1000000");
 
-  /* clock_gettime shouldn't fail here since CLOCK_PROCESS_CPUTIME_ID is
-     supported since 2.6.12.  Check the return value anyway in case the kernel
-     barfs on us for some reason.  */
-  if (__glibc_unlikely (__clock_gettime (CLOCK_PROCESS_CPUTIME_ID, &ts) != 0))
+  if (__glibc_unlikely (__clock_gettime64 (CLOCK_PROCESS_CPUTIME_ID, &ts) != 0))
     return (clock_t) -1;
 
   return (ts.tv_sec * CLOCKS_PER_SEC
-- 
2.20.1


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 0/5] y2038: Replace __clock_gettime with __clock_gettime64
  2020-03-26  8:06 [PATCH v2 0/5] y2038: Replace __clock_gettime with __clock_gettime64 Lukasz Majewski
                   ` (4 preceding siblings ...)
  2020-03-26  8:06 ` [PATCH v2 5/5] y2038: Replace __clock_gettime with __clock_gettime64 Lukasz Majewski
@ 2020-04-06 15:16 ` Lukasz Majewski
  5 siblings, 0 replies; 24+ messages in thread
From: Lukasz Majewski @ 2020-04-06 15:16 UTC (permalink / raw)
  To: Joseph Myers, Adhemerval Zanella
  Cc: Alistair Francis, Alistair Francis, GNU C Library,
	Florian Weimer, Andreas Schwab

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

Dear Community,

> This patch series starts the conversion of __clock_gettime calls to
> __clock_gettime64 as the latter supports 64 bit time on archs with 
> __WORDSIZE == 32 and __TIMESIZE != 64.
> 
> On purpose the nptl and pthreads have been omitted, as for those
> separate patch sets will be prepared.
> It is an open question if sunrpc shall be converted or not.
> 
> This patch series also provides __clock_gettime64 wrapper for HURD to
> allow conversion of generic in-glibc code (as HURD uses them - like
> e.g. pthreads).
> 
> Special care had to be taken for s390 as it has its own definition of
> utmp.h and hence requires using TIMESPEC_TO_TIMEVAL macro.
> 
> Lukasz Majewski (5):
>   y2038: Export __clock_gettime64 to be usable in other libraries
>   y2038: hurd: Provide __clock_gettime64 function
>   y2038: inet: Convert inet deadline to support 64 bit time
>   y2038: nscd: Modify nscd_helper to use __clock_gettime64
>   y2038: Replace __clock_gettime with __clock_gettime64

Are there any more comments for this patch set?

> 
>  benchtests/bench-timing.h                       |  2 +-
>  include/random-bits.h                           |  4 ++--
>  inet/deadline.c                                 |  4 ++--
>  inet/net-internal.h                             |  5 +++--
>  login/logout.c                                  |  4 ++--
>  login/logwtmp.c                                 |  5 +++--
>  nis/nis_call.c                                  |  6 +++---
>  nscd/nscd_helper.c                              | 17
> +++++++++-------- sysdeps/generic/hp-timing.h                     |
> 4 ++-- sysdeps/generic/memusage.h                      |  4 ++--
>  sysdeps/mach/clock_gettime.c                    | 14 ++++++++++++++
>  .../unix/sysv/linux/alpha/osf_gettimeofday.c    |  4 ++--
>  sysdeps/unix/sysv/linux/clock.c                 |  7 ++-----
>  time/Versions                                   |  1 +
>  14 files changed, 48 insertions(+), 33 deletions(-)
> 


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] 24+ messages in thread

* Re: [PATCH v2 5/5] y2038: Replace __clock_gettime with __clock_gettime64
  2020-03-26  8:06 ` [PATCH v2 5/5] y2038: Replace __clock_gettime with __clock_gettime64 Lukasz Majewski
@ 2020-04-06 16:07   ` Florian Weimer
  2020-04-06 21:03     ` Lukasz Majewski
  2020-04-30 21:03   ` Adhemerval Zanella
  1 sibling, 1 reply; 24+ messages in thread
From: Florian Weimer @ 2020-04-06 16:07 UTC (permalink / raw)
  To: Lukasz Majewski
  Cc: Joseph Myers, Adhemerval Zanella, GNU C Library, Andreas Schwab,
	Alistair Francis

* Lukasz Majewski:

> The __clock_gettime internal function is not supporting 64 bit time on
> architectures with __WORDSIZE == 32 and __TIMESIZE != 64 (like e.g. ARM 32
> bit).
>
> The __clock_gettime64 shall be used instead in the glibc itself as it

The __clock_gettime64 function?

> supports 64 bit time on those systems.
> This change does not bring any change to systems with __WORDSIZE == 64 as
> for them the __clock_gettime64 is aliased to __clock_gettime
> (in ./include/time.h).

Should this patch remove the __clock_gettime GLIBC_PRIVATE export?

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 5/5] y2038: Replace __clock_gettime with __clock_gettime64
  2020-04-06 16:07   ` Florian Weimer
@ 2020-04-06 21:03     ` Lukasz Majewski
  2020-04-14 12:14       ` Lukasz Majewski
  0 siblings, 1 reply; 24+ messages in thread
From: Lukasz Majewski @ 2020-04-06 21:03 UTC (permalink / raw)
  To: Florian Weimer
  Cc: Joseph Myers, Adhemerval Zanella, GNU C Library, Andreas Schwab,
	Alistair Francis

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

Hi Florian,

> * Lukasz Majewski:
> 
> > The __clock_gettime internal function is not supporting 64 bit time
> > on architectures with __WORDSIZE == 32 and __TIMESIZE != 64 (like
> > e.g. ARM 32 bit).
> >
> > The __clock_gettime64 shall be used instead in the glibc itself as
> > it  
> 
> The __clock_gettime64 function?

Yes. Correct. Thanks for spotting it.

> 
> > supports 64 bit time on those systems.
> > This change does not bring any change to systems with __WORDSIZE ==
> > 64 as for them the __clock_gettime64 is aliased to __clock_gettime
> > (in ./include/time.h).  
> 
> Should this patch remove the __clock_gettime GLIBC_PRIVATE export?

No. This work is just a first step. The nptl and pthread code still use
__clock_gettime internally. I will prepare __clock_gettime conversion
to __clock_gettime64 for those parts of glibc in latter patches.


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] 24+ messages in thread

* Re: [PATCH v2 5/5] y2038: Replace __clock_gettime with __clock_gettime64
  2020-04-06 21:03     ` Lukasz Majewski
@ 2020-04-14 12:14       ` Lukasz Majewski
  2020-04-21 13:48         ` Lukasz Majewski
  0 siblings, 1 reply; 24+ messages in thread
From: Lukasz Majewski @ 2020-04-14 12:14 UTC (permalink / raw)
  To: Florian Weimer
  Cc: Joseph Myers, Adhemerval Zanella, GNU C Library, Andreas Schwab,
	Alistair Francis

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

Dear Community, 


> Hi Florian,
> 
> > * Lukasz Majewski:
> >   
> > > The __clock_gettime internal function is not supporting 64 bit
> > > time on architectures with __WORDSIZE == 32 and __TIMESIZE != 64
> > > (like e.g. ARM 32 bit).
> > >
> > > The __clock_gettime64 shall be used instead in the glibc itself as
> > > it    
> > 
> > The __clock_gettime64 function?  
> 
> Yes. Correct. Thanks for spotting it.
> 
> >   
> > > supports 64 bit time on those systems.
> > > This change does not bring any change to systems with __WORDSIZE
> > > == 64 as for them the __clock_gettime64 is aliased to
> > > __clock_gettime (in ./include/time.h).    
> > 
> > Should this patch remove the __clock_gettime GLIBC_PRIVATE export?  
> 
> No. This work is just a first step. The nptl and pthread code still
> use __clock_gettime internally. I will prepare __clock_gettime
> conversion to __clock_gettime64 for those parts of glibc in latter
> patches.

Are there any more comments regarding this patch set? In other word -
is it eligible for pulling?

> 
> 
> 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] 24+ messages in thread

* Re: [PATCH v2 5/5] y2038: Replace __clock_gettime with __clock_gettime64
  2020-04-14 12:14       ` Lukasz Majewski
@ 2020-04-21 13:48         ` Lukasz Majewski
  2020-04-29 21:48           ` Lukasz Majewski
  0 siblings, 1 reply; 24+ messages in thread
From: Lukasz Majewski @ 2020-04-21 13:48 UTC (permalink / raw)
  To: Florian Weimer
  Cc: Andreas Schwab, Alistair Francis, GNU C Library, Joseph Myers,
	Adhemerval Zanella

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

Dear Community,

> Dear Community, 
> 
> 
> > Hi Florian,
> >   
> > > * Lukasz Majewski:
> > >     
> > > > The __clock_gettime internal function is not supporting 64 bit
> > > > time on architectures with __WORDSIZE == 32 and __TIMESIZE != 64
> > > > (like e.g. ARM 32 bit).
> > > >
> > > > The __clock_gettime64 shall be used instead in the glibc itself
> > > > as it      
> > > 
> > > The __clock_gettime64 function?    
> > 
> > Yes. Correct. Thanks for spotting it.
> >   
> > >     
> > > > supports 64 bit time on those systems.
> > > > This change does not bring any change to systems with __WORDSIZE
> > > > == 64 as for them the __clock_gettime64 is aliased to
> > > > __clock_gettime (in ./include/time.h).      
> > > 
> > > Should this patch remove the __clock_gettime GLIBC_PRIVATE
> > > export?    
> > 
> > No. This work is just a first step. The nptl and pthread code still
> > use __clock_gettime internally. I will prepare __clock_gettime
> > conversion to __clock_gettime64 for those parts of glibc in latter
> > patches.  
> 
> Are there any more comments regarding this patch set? In other word -
> is it eligible for pulling?

Are there any more comments?

> 
> > 
> > 
> > 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] 24+ messages in thread

* Re: [PATCH v2 5/5] y2038: Replace __clock_gettime with __clock_gettime64
  2020-04-21 13:48         ` Lukasz Majewski
@ 2020-04-29 21:48           ` Lukasz Majewski
  0 siblings, 0 replies; 24+ messages in thread
From: Lukasz Majewski @ 2020-04-29 21:48 UTC (permalink / raw)
  To: Florian Weimer
  Cc: Andreas Schwab, Alistair Francis, GNU C Library, Joseph Myers,
	Adhemerval Zanella

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

Dear Community,

> Dear Community,
> 
> > Dear Community, 
> > 
> >   
> > > Hi Florian,
> > >     
> > > > * Lukasz Majewski:
> > > >       
> > > > > The __clock_gettime internal function is not supporting 64 bit
> > > > > time on architectures with __WORDSIZE == 32 and __TIMESIZE !=
> > > > > 64 (like e.g. ARM 32 bit).
> > > > >
> > > > > The __clock_gettime64 shall be used instead in the glibc
> > > > > itself as it        
> > > > 
> > > > The __clock_gettime64 function?      
> > > 
> > > Yes. Correct. Thanks for spotting it.
> > >     
> > > >       
> > > > > supports 64 bit time on those systems.
> > > > > This change does not bring any change to systems with
> > > > > __WORDSIZE == 64 as for them the __clock_gettime64 is aliased
> > > > > to __clock_gettime (in ./include/time.h).        
> > > > 
> > > > Should this patch remove the __clock_gettime GLIBC_PRIVATE
> > > > export?      
> > > 
> > > No. This work is just a first step. The nptl and pthread code
> > > still use __clock_gettime internally. I will prepare
> > > __clock_gettime conversion to __clock_gettime64 for those parts
> > > of glibc in latter patches.    
> > 
> > Are there any more comments regarding this patch set? In other word
> > - is it eligible for pulling?  
> 
> Are there any more comments?

Gentle ping on this patch set...

> 
> >   
> > > 
> > > 
> > > 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




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] 24+ messages in thread

* Re: [PATCH v2 3/5] y2038: inet: Convert inet deadline to support 64 bit time
  2020-03-26  8:06 ` [PATCH v2 3/5] y2038: inet: Convert inet deadline to support 64 bit time Lukasz Majewski
@ 2020-04-30 20:51   ` Adhemerval Zanella
  0 siblings, 0 replies; 24+ messages in thread
From: Adhemerval Zanella @ 2020-04-30 20:51 UTC (permalink / raw)
  To: Lukasz Majewski, Joseph Myers
  Cc: Alistair Francis, Alistair Francis, GNU C Library,
	Florian Weimer, Andreas Schwab



On 26/03/2020 05:06, Lukasz Majewski wrote:
> This change brings 64 bit time support to inet deadline related code for
> architectures with __WORDSIZE == 32 && __TIMESIZE != 64.
> 
> It is also safe to replace struct timespec with struct __timespec64 in
> deadline related structures as:

LGTM, thanks.

Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>

> 
> - The __deadline_to_ms () returns the number of miliseconds to deadline to
>   be used with __poll (and hence it is a relative value).
> - To calculate the deadline from timeval (which will be converted latter)
>   the uintmax_t type is used (unsinged long long int).
> ---
>  inet/deadline.c     | 4 ++--
>  inet/net-internal.h | 5 +++--
>  2 files changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/inet/deadline.c b/inet/deadline.c
> index ebf9a4f52c..eac7afd1a8 100644
> --- a/inet/deadline.c
> +++ b/inet/deadline.c
> @@ -28,8 +28,8 @@ struct deadline_current_time
>  __deadline_current_time (void)
>  {
>    struct deadline_current_time result;
> -  if (__clock_gettime (CLOCK_MONOTONIC, &result.current) != 0)
> -    __clock_gettime (CLOCK_REALTIME, &result.current);
> +  if (__clock_gettime64 (CLOCK_MONOTONIC, &result.current) != 0)
> +    __clock_gettime64 (CLOCK_REALTIME, &result.current);
>    assert (result.current.tv_sec >= 0);
>    return result;
>  }

Ok.

> diff --git a/inet/net-internal.h b/inet/net-internal.h
> index 3ca301a9be..50c7e1c482 100644
> --- a/inet/net-internal.h
> +++ b/inet/net-internal.h
> @@ -24,6 +24,7 @@
>  #include <stdint.h>
>  #include <sys/time.h>
>  #include <libc-diag.h>
> +#include <struct___timespec64.h>
>  
>  int __inet6_scopeid_pton (const struct in6_addr *address,
>                            const char *scope, uint32_t *result);
> @@ -76,7 +77,7 @@ enum idna_name_classification __idna_name_classify (const char *name)
>     timeouts and vice versa.  */
>  struct deadline_current_time
>  {
> -  struct timespec current;
> +  struct __timespec64 current;
>  };
>  

Ok.

>  /* Return the current time.  Terminates the process if the current
> @@ -86,7 +87,7 @@ struct deadline_current_time __deadline_current_time (void) attribute_hidden;
>  /* Computed absolute deadline.  */
>  struct deadline
>  {
> -  struct timespec absolute;
> +  struct __timespec64 absolute;
>  };
>  
>  
> 

Ok.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 4/5] y2038: nscd: Modify nscd_helper to use __clock_gettime64
  2020-03-26  8:06 ` [PATCH v2 4/5] y2038: nscd: Modify nscd_helper to use __clock_gettime64 Lukasz Majewski
@ 2020-04-30 20:54   ` Adhemerval Zanella
  2020-05-01 11:30     ` Lukasz Majewski
  0 siblings, 1 reply; 24+ messages in thread
From: Adhemerval Zanella @ 2020-04-30 20:54 UTC (permalink / raw)
  To: Lukasz Majewski, Joseph Myers
  Cc: Alistair Francis, Alistair Francis, GNU C Library,
	Florian Weimer, Andreas Schwab



On 26/03/2020 05:06, Lukasz Majewski wrote:
> The nscd/nscd_helper.c uses __clock_gettime to get current time and on this
> basis calculate the relative timeout for poll.
> By using __clock_gettime64 on systems with __WORDSIZE == 32 && __TIMESIZE != 64
> the timeout is correctly calculated after time_t overflow.

LGTM, thanks.

> ---
>  nscd/nscd_helper.c | 17 +++++++++--------
>  1 file changed, 9 insertions(+), 8 deletions(-)
> 
> diff --git a/nscd/nscd_helper.c b/nscd/nscd_helper.c
> index d2d7d15f26..a4f3312f90 100644
> --- a/nscd/nscd_helper.c
> +++ b/nscd/nscd_helper.c
> @@ -37,6 +37,7 @@
>  #include <not-cancel.h>
>  #include <kernel-features.h>
>  #include <nss.h>
> +#include <struct___timespec64.h>
>  
>  #include "nscd-client.h"
>  
> @@ -59,10 +60,10 @@ wait_on_socket (int sock, long int usectmo)
>        /* Handle the case where the poll() call is interrupted by a
>  	 signal.  We cannot just use TEMP_FAILURE_RETRY since it might
>  	 lead to infinite loops.  */
> -      struct timespec now;
> -      __clock_gettime (CLOCK_REALTIME, &now);
> -      long int end = (now.tv_sec * 1000 + usectmo
> -                      + (now.tv_nsec + 500000) / 1000000);
> +      struct __timespec64 now;
> +      __clock_gettime64 (CLOCK_REALTIME, &now);
> +      int64_t end = (now.tv_sec * 1000 + usectmo
> +                     + (now.tv_nsec + 500000) / 1000000);
>        long int timeout = usectmo;
>        while (1)
>  	{

Ok. Maybe we could use ppoll instead here to simplify the timeout
calculation?

> @@ -71,7 +72,7 @@ wait_on_socket (int sock, long int usectmo)
>  	    break;
>  
>  	  /* Recompute the timeout time.  */
> -          __clock_gettime (CLOCK_REALTIME, &now);
> +          __clock_gettime64 (CLOCK_REALTIME, &now);
>  	  timeout = end - ((now.tv_sec * 1000
>                              + (now.tv_nsec + 500000) / 1000000));
>  	}

Ok.

> @@ -193,7 +194,7 @@ open_socket (request_type type, const char *key, size_t keylen)
>    memcpy (reqdata->key, key, keylen);
>  
>    bool first_try = true;
> -  struct timespec tvend = { 0, 0 };
> +  struct __timespec64 tvend = { 0, 0 };
>    while (1)
>      {

Ok.

>  #ifndef MSG_NOSIGNAL
> @@ -212,8 +213,8 @@ open_socket (request_type type, const char *key, size_t keylen)
>  
>        /* The daemon is busy wait for it.  */
>        int to;
> -      struct timespec now;
> -      __clock_gettime (CLOCK_REALTIME, &now);
> +      struct __timespec64 now;
> +      __clock_gettime64 (CLOCK_REALTIME, &now);
>        if (first_try)
>  	{
>  	  tvend.tv_nsec = now.tv_nsec;
> 

Ok.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 5/5] y2038: Replace __clock_gettime with __clock_gettime64
  2020-03-26  8:06 ` [PATCH v2 5/5] y2038: Replace __clock_gettime with __clock_gettime64 Lukasz Majewski
  2020-04-06 16:07   ` Florian Weimer
@ 2020-04-30 21:03   ` Adhemerval Zanella
  2020-05-01 11:56     ` Lukasz Majewski
  1 sibling, 1 reply; 24+ messages in thread
From: Adhemerval Zanella @ 2020-04-30 21:03 UTC (permalink / raw)
  To: Lukasz Majewski, Joseph Myers
  Cc: Alistair Francis, Alistair Francis, GNU C Library,
	Florian Weimer, Andreas Schwab



On 26/03/2020 05:06, Lukasz Majewski wrote:
> The __clock_gettime internal function is not supporting 64 bit time on
> architectures with __WORDSIZE == 32 and __TIMESIZE != 64 (like e.g. ARM 32
> bit).
> 
> The __clock_gettime64 shall be used instead in the glibc itself as it
> supports 64 bit time on those systems.
> This change does not bring any change to systems with __WORDSIZE == 64 as
> for them the __clock_gettime64 is aliased to __clock_gettime
> (in ./include/time.h).
> 
> ---
> Changes for v2:
> - Use only TIMESPEC_TO_TIMEVAL instead of valid_timespec64_to_timeval in
>   logout.c and logwtmp.c as it is generic enough to also support struct
>   __timespec64 conversion to struct timeval
> ---
>  benchtests/bench-timing.h                        | 2 +-
>  include/random-bits.h                            | 4 ++--
>  login/logout.c                                   | 4 ++--
>  login/logwtmp.c                                  | 5 +++--
>  nis/nis_call.c                                   | 6 +++---
>  sysdeps/generic/hp-timing.h                      | 4 ++--
>  sysdeps/generic/memusage.h                       | 4 ++--
>  sysdeps/unix/sysv/linux/alpha/osf_gettimeofday.c | 4 ++--
>  sysdeps/unix/sysv/linux/clock.c                  | 7 ++-----
>  9 files changed, 19 insertions(+), 21 deletions(-)
> 
> diff --git a/benchtests/bench-timing.h b/benchtests/bench-timing.h
> index 5b9a8384bb..a0d6f82465 100644
> --- a/benchtests/bench-timing.h
> +++ b/benchtests/bench-timing.h
> @@ -18,7 +18,7 @@
>  
>  #undef attribute_hidden
>  #define attribute_hidden
> -#define __clock_gettime clock_gettime
> +#define __clock_gettime __clock_gettime64
>  #include <hp-timing.h>
>  #include <stdint.h>
>  

Ok, although we might revise it later and build the benchmarks
with __TIMESIZE==64.

> diff --git a/include/random-bits.h b/include/random-bits.h
> index fd3fa01f9b..7561e55ca6 100644
> --- a/include/random-bits.h
> +++ b/include/random-bits.h
> @@ -30,8 +30,8 @@
>  static inline uint32_t
>  random_bits (void)
>  {
> -  struct timespec tv;
> -  __clock_gettime (CLOCK_MONOTONIC, &tv);
> +  struct __timespec64 tv;
> +  __clock_gettime64 (CLOCK_MONOTONIC, &tv);
>    /* Shuffle the lower bits to minimize the clock bias.  */
>    uint32_t ret = tv.tv_nsec ^ tv.tv_sec;
>    ret ^= (ret << 24) | (ret >> 8);

Ok.

> diff --git a/login/logout.c b/login/logout.c
> index 7653fe8886..091312eb1d 100644
> --- a/login/logout.c
> +++ b/login/logout.c
> @@ -47,8 +47,8 @@ logout (const char *line)
>        memset (ut->ut_name, '\0', sizeof ut->ut_name);
>        memset (ut->ut_host, '\0', sizeof ut->ut_host);
>  
> -      struct timespec ts;
> -      __clock_gettime (CLOCK_REALTIME, &ts);
> +      struct __timespec64 ts;
> +      __clock_gettime64 (CLOCK_REALTIME, &ts);
>        TIMESPEC_TO_TIMEVAL (&ut->ut_tv, &ts);
>        ut->ut_type = DEAD_PROCESS;
>  

Ok.

> diff --git a/login/logwtmp.c b/login/logwtmp.c
> index 90406acc3d..050219c153 100644
> --- a/login/logwtmp.c
> +++ b/login/logwtmp.c
> @@ -21,6 +21,7 @@
>  #include <time.h>
>  #include <unistd.h>
>  #include <utmp.h>
> +#include <struct___timespec64.h>
>  
>  
>  void
> @@ -36,8 +37,8 @@ logwtmp (const char *line, const char *name, const char *host)
>    strncpy (ut.ut_name, name, sizeof ut.ut_name);
>    strncpy (ut.ut_host, host, sizeof ut.ut_host);
>  
> -  struct timespec ts;
> -  __clock_gettime (CLOCK_REALTIME, &ts);
> +  struct __timespec64 ts;
> +  __clock_gettime64 (CLOCK_REALTIME, &ts);
>    TIMESPEC_TO_TIMEVAL (&ut.ut_tv, &ts);
>  
>    updwtmp (_PATH_WTMP, &ut);

Ok.

> diff --git a/nis/nis_call.c b/nis/nis_call.c
> index 92c70e97aa..9c6f62a753 100644
> --- a/nis/nis_call.c
> +++ b/nis/nis_call.c
> @@ -709,7 +709,7 @@ __nisfind_server (const_nis_name name, int search_parent,
>    nis_error status;
>    directory_obj *obj;
>    struct timeval now;
> -  struct timespec ts;
> +  struct __timespec64 ts;
>    unsigned int server_used = ~0;
>    unsigned int current_ep = ~0;
>  
> @@ -719,8 +719,8 @@ __nisfind_server (const_nis_name name, int search_parent,
>    if (*dir != NULL)
>      return NIS_SUCCESS;
>  
> -  __clock_gettime (CLOCK_REALTIME, &ts);
> -  TIMESPEC_TO_TIMEVAL (&now, &ts);
> +  __clock_gettime64 (CLOCK_REALTIME, &ts);
> +  now = valid_timespec64_to_timeval (ts);
>  
>    if ((flags & NO_CACHE) == 0)
>      *dir = nis_server_cache_search (name, search_parent, &server_used,

I think it would be simpler to just remove the timeval argument on
nis_server_cache_search and move the __clock_gettime64 call on the
function start. 

Also, it would require to change nis_server_cache to use a
__time64_t for 'expires', otherwise this change won't help in
case of a time_t overflow.


> diff --git a/sysdeps/generic/hp-timing.h b/sysdeps/generic/hp-timing.h
> index e2d7447212..af9d92f7f7 100644
> --- a/sysdeps/generic/hp-timing.h
> +++ b/sysdeps/generic/hp-timing.h
> @@ -34,8 +34,8 @@ typedef uint64_t hp_timing_t;
>     vDSO symbol.  */
>  #define HP_TIMING_NOW(var) \
>  ({								\
> -  struct timespec tv;						\
> -  __clock_gettime (CLOCK_MONOTONIC, &tv);			\
> +  struct __timespec64 tv;						\
> +  __clock_gettime64 (CLOCK_MONOTONIC, &tv);			\
>    (var) = (tv.tv_nsec + UINT64_C(1000000000) * tv.tv_sec);	\
>  })
>  

Ok.

> diff --git a/sysdeps/generic/memusage.h b/sysdeps/generic/memusage.h
> index a111864b0b..91e56d24de 100644
> --- a/sysdeps/generic/memusage.h
> +++ b/sysdeps/generic/memusage.h
> @@ -28,9 +28,9 @@
>  #ifndef GETTIME
>  # define GETTIME(low,high)						   \
>    {									   \
> -    struct timespec now;						   \
> +    struct __timespec64 now;						   \
>      uint64_t usecs;							   \
> -    clock_gettime (CLOCK_REALTIME, &now);				   \
> +    __clock_gettime64 (CLOCK_REALTIME, &now);				   \
>      usecs = (uint64_t)now.tv_nsec / 1000 + (uint64_t)now.tv_sec * 1000000; \
>      low = usecs & 0xffffffff;						   \
>      high = usecs >> 32;							   \

Is is the requirement to export __clock_gettime64 as a GLIBC_PRIVATE symbol?

In any case, I think we should try to avoid use internal symbols even for
distributed glibc libraries, so I think this change should go once we
start to export the clock_gettime64 as default symbol.

> diff --git a/sysdeps/unix/sysv/linux/alpha/osf_gettimeofday.c b/sysdeps/unix/sysv/linux/alpha/osf_gettimeofday.c
> index 8cf5d303f9..5075ae0444 100644
> --- a/sysdeps/unix/sysv/linux/alpha/osf_gettimeofday.c
> +++ b/sysdeps/unix/sysv/linux/alpha/osf_gettimeofday.c
> @@ -35,8 +35,8 @@ __gettimeofday_tv32 (struct __timeval32 *restrict tv32, void *restrict tz)
>    if (__glibc_unlikely (tz != 0))
>      memset (tz, 0, sizeof (struct timezone));
>  
> -  struct timespec ts;
> -  __clock_gettime (CLOCK_REALTIME, &ts);
> +  struct __timespec64 ts;
> +  __clock_gettime64 (CLOCK_REALTIME, &ts);
>  
>    *tv32 = valid_timespec_to_timeval32 (ts);
>    return 0;

Ok.

> diff --git a/sysdeps/unix/sysv/linux/clock.c b/sysdeps/unix/sysv/linux/clock.c
> index 24a8df0cf5..157ae8eb3f 100644
> --- a/sysdeps/unix/sysv/linux/clock.c
> +++ b/sysdeps/unix/sysv/linux/clock.c
> @@ -23,15 +23,12 @@
>  clock_t
>  clock (void)
>  {
> -  struct timespec ts;
> +  struct __timespec64 ts;
>  
>    _Static_assert (CLOCKS_PER_SEC == 1000000,
>  		  "CLOCKS_PER_SEC should be 1000000");
>  
> -  /* clock_gettime shouldn't fail here since CLOCK_PROCESS_CPUTIME_ID is
> -     supported since 2.6.12.  Check the return value anyway in case the kernel
> -     barfs on us for some reason.  */
> -  if (__glibc_unlikely (__clock_gettime (CLOCK_PROCESS_CPUTIME_ID, &ts) != 0))
> +  if (__glibc_unlikely (__clock_gettime64 (CLOCK_PROCESS_CPUTIME_ID, &ts) != 0))
>      return (clock_t) -1;
>  
>    return (ts.tv_sec * CLOCKS_PER_SEC
> 

Ok.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 4/5] y2038: nscd: Modify nscd_helper to use __clock_gettime64
  2020-04-30 20:54   ` Adhemerval Zanella
@ 2020-05-01 11:30     ` Lukasz Majewski
  2020-05-04 12:27       ` Adhemerval Zanella
  0 siblings, 1 reply; 24+ messages in thread
From: Lukasz Majewski @ 2020-05-01 11:30 UTC (permalink / raw)
  To: Adhemerval Zanella
  Cc: Joseph Myers, Alistair Francis, Alistair Francis, GNU C Library,
	Florian Weimer, Andreas Schwab

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

Hi Adhemerval,

> On 26/03/2020 05:06, Lukasz Majewski wrote:
> > The nscd/nscd_helper.c uses __clock_gettime to get current time and
> > on this basis calculate the relative timeout for poll.
> > By using __clock_gettime64 on systems with __WORDSIZE == 32 &&
> > __TIMESIZE != 64 the timeout is correctly calculated after time_t
> > overflow.  
> 
> LGTM, thanks.
> 
> > ---
> >  nscd/nscd_helper.c | 17 +++++++++--------
> >  1 file changed, 9 insertions(+), 8 deletions(-)
> > 
> > diff --git a/nscd/nscd_helper.c b/nscd/nscd_helper.c
> > index d2d7d15f26..a4f3312f90 100644
> > --- a/nscd/nscd_helper.c
> > +++ b/nscd/nscd_helper.c
> > @@ -37,6 +37,7 @@
> >  #include <not-cancel.h>
> >  #include <kernel-features.h>
> >  #include <nss.h>
> > +#include <struct___timespec64.h>
> >  
> >  #include "nscd-client.h"
> >  
> > @@ -59,10 +60,10 @@ wait_on_socket (int sock, long int usectmo)
> >        /* Handle the case where the poll() call is interrupted by a
> >  	 signal.  We cannot just use TEMP_FAILURE_RETRY since it
> > might lead to infinite loops.  */
> > -      struct timespec now;
> > -      __clock_gettime (CLOCK_REALTIME, &now);
> > -      long int end = (now.tv_sec * 1000 + usectmo
> > -                      + (now.tv_nsec + 500000) / 1000000);
> > +      struct __timespec64 now;
> > +      __clock_gettime64 (CLOCK_REALTIME, &now);
> > +      int64_t end = (now.tv_sec * 1000 + usectmo
> > +                     + (now.tv_nsec + 500000) / 1000000);
> >        long int timeout = usectmo;
> >        while (1)
> >  	{  
> 
> Ok. Maybe we could use ppoll instead here to simplify the timeout
> calculation?
> 

I wanted to change as little as possible (to not introduce any extra
bugs) to only replace __clock_gettime with __clock_gettime64. 

> > @@ -71,7 +72,7 @@ wait_on_socket (int sock, long int usectmo)
> >  	    break;
> >  
> >  	  /* Recompute the timeout time.  */
> > -          __clock_gettime (CLOCK_REALTIME, &now);
> > +          __clock_gettime64 (CLOCK_REALTIME, &now);
> >  	  timeout = end - ((now.tv_sec * 1000
> >                              + (now.tv_nsec + 500000) / 1000000));
> >  	}  
> 
> Ok.
> 
> > @@ -193,7 +194,7 @@ open_socket (request_type type, const char
> > *key, size_t keylen) memcpy (reqdata->key, key, keylen);
> >  
> >    bool first_try = true;
> > -  struct timespec tvend = { 0, 0 };
> > +  struct __timespec64 tvend = { 0, 0 };
> >    while (1)
> >      {  
> 
> Ok.
> 
> >  #ifndef MSG_NOSIGNAL
> > @@ -212,8 +213,8 @@ open_socket (request_type type, const char
> > *key, size_t keylen) 
> >        /* The daemon is busy wait for it.  */
> >        int to;
> > -      struct timespec now;
> > -      __clock_gettime (CLOCK_REALTIME, &now);
> > +      struct __timespec64 now;
> > +      __clock_gettime64 (CLOCK_REALTIME, &now);
> >        if (first_try)
> >  	{
> >  	  tvend.tv_nsec = now.tv_nsec;
> >   
> 
> 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] 24+ messages in thread

* Re: [PATCH v2 5/5] y2038: Replace __clock_gettime with __clock_gettime64
  2020-04-30 21:03   ` Adhemerval Zanella
@ 2020-05-01 11:56     ` Lukasz Majewski
  2020-05-04 14:04       ` Adhemerval Zanella
  0 siblings, 1 reply; 24+ messages in thread
From: Lukasz Majewski @ 2020-05-01 11:56 UTC (permalink / raw)
  To: Adhemerval Zanella
  Cc: Joseph Myers, Alistair Francis, Alistair Francis, GNU C Library,
	Florian Weimer, Andreas Schwab

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

Hi Adhemerval,

> On 26/03/2020 05:06, Lukasz Majewski wrote:
> > The __clock_gettime internal function is not supporting 64 bit time
> > on architectures with __WORDSIZE == 32 and __TIMESIZE != 64 (like
> > e.g. ARM 32 bit).
> > 
> > The __clock_gettime64 shall be used instead in the glibc itself as
> > it supports 64 bit time on those systems.
> > This change does not bring any change to systems with __WORDSIZE ==
> > 64 as for them the __clock_gettime64 is aliased to __clock_gettime
> > (in ./include/time.h).
> > 
> > ---
> > Changes for v2:
> > - Use only TIMESPEC_TO_TIMEVAL instead of
> > valid_timespec64_to_timeval in logout.c and logwtmp.c as it is
> > generic enough to also support struct __timespec64 conversion to
> > struct timeval ---
> >  benchtests/bench-timing.h                        | 2 +-
> >  include/random-bits.h                            | 4 ++--
> >  login/logout.c                                   | 4 ++--
> >  login/logwtmp.c                                  | 5 +++--
> >  nis/nis_call.c                                   | 6 +++---
> >  sysdeps/generic/hp-timing.h                      | 4 ++--
> >  sysdeps/generic/memusage.h                       | 4 ++--
> >  sysdeps/unix/sysv/linux/alpha/osf_gettimeofday.c | 4 ++--
> >  sysdeps/unix/sysv/linux/clock.c                  | 7 ++-----
> >  9 files changed, 19 insertions(+), 21 deletions(-)
> > 
> > diff --git a/benchtests/bench-timing.h b/benchtests/bench-timing.h
> > index 5b9a8384bb..a0d6f82465 100644
> > --- a/benchtests/bench-timing.h
> > +++ b/benchtests/bench-timing.h
> > @@ -18,7 +18,7 @@
> >  
> >  #undef attribute_hidden
> >  #define attribute_hidden
> > -#define __clock_gettime clock_gettime
> > +#define __clock_gettime __clock_gettime64
> >  #include <hp-timing.h>
> >  #include <stdint.h>
> >    
> 
> Ok, although we might revise it later and build the benchmarks
> with __TIMESIZE==64.
> 
> > diff --git a/include/random-bits.h b/include/random-bits.h
> > index fd3fa01f9b..7561e55ca6 100644
> > --- a/include/random-bits.h
> > +++ b/include/random-bits.h
> > @@ -30,8 +30,8 @@
> >  static inline uint32_t
> >  random_bits (void)
> >  {
> > -  struct timespec tv;
> > -  __clock_gettime (CLOCK_MONOTONIC, &tv);
> > +  struct __timespec64 tv;
> > +  __clock_gettime64 (CLOCK_MONOTONIC, &tv);
> >    /* Shuffle the lower bits to minimize the clock bias.  */
> >    uint32_t ret = tv.tv_nsec ^ tv.tv_sec;
> >    ret ^= (ret << 24) | (ret >> 8);  
> 
> Ok.
> 
> > diff --git a/login/logout.c b/login/logout.c
> > index 7653fe8886..091312eb1d 100644
> > --- a/login/logout.c
> > +++ b/login/logout.c
> > @@ -47,8 +47,8 @@ logout (const char *line)
> >        memset (ut->ut_name, '\0', sizeof ut->ut_name);
> >        memset (ut->ut_host, '\0', sizeof ut->ut_host);
> >  
> > -      struct timespec ts;
> > -      __clock_gettime (CLOCK_REALTIME, &ts);
> > +      struct __timespec64 ts;
> > +      __clock_gettime64 (CLOCK_REALTIME, &ts);
> >        TIMESPEC_TO_TIMEVAL (&ut->ut_tv, &ts);
> >        ut->ut_type = DEAD_PROCESS;
> >    
> 
> Ok.
> 
> > diff --git a/login/logwtmp.c b/login/logwtmp.c
> > index 90406acc3d..050219c153 100644
> > --- a/login/logwtmp.c
> > +++ b/login/logwtmp.c
> > @@ -21,6 +21,7 @@
> >  #include <time.h>
> >  #include <unistd.h>
> >  #include <utmp.h>
> > +#include <struct___timespec64.h>
> >  
> >  
> >  void
> > @@ -36,8 +37,8 @@ logwtmp (const char *line, const char *name,
> > const char *host) strncpy (ut.ut_name, name, sizeof ut.ut_name);
> >    strncpy (ut.ut_host, host, sizeof ut.ut_host);
> >  
> > -  struct timespec ts;
> > -  __clock_gettime (CLOCK_REALTIME, &ts);
> > +  struct __timespec64 ts;
> > +  __clock_gettime64 (CLOCK_REALTIME, &ts);
> >    TIMESPEC_TO_TIMEVAL (&ut.ut_tv, &ts);
> >  
> >    updwtmp (_PATH_WTMP, &ut);  
> 
> Ok.
> 
> > diff --git a/nis/nis_call.c b/nis/nis_call.c
> > index 92c70e97aa..9c6f62a753 100644
> > --- a/nis/nis_call.c
> > +++ b/nis/nis_call.c
> > @@ -709,7 +709,7 @@ __nisfind_server (const_nis_name name, int
> > search_parent, nis_error status;
> >    directory_obj *obj;
> >    struct timeval now;
> > -  struct timespec ts;
> > +  struct __timespec64 ts;
> >    unsigned int server_used = ~0;
> >    unsigned int current_ep = ~0;
> >  
> > @@ -719,8 +719,8 @@ __nisfind_server (const_nis_name name, int
> > search_parent, if (*dir != NULL)
> >      return NIS_SUCCESS;
> >  
> > -  __clock_gettime (CLOCK_REALTIME, &ts);
> > -  TIMESPEC_TO_TIMEVAL (&now, &ts);
> > +  __clock_gettime64 (CLOCK_REALTIME, &ts);
> > +  now = valid_timespec64_to_timeval (ts);
> >  
> >    if ((flags & NO_CACHE) == 0)
> >      *dir = nis_server_cache_search (name, search_parent,
> > &server_used,  
> 
> I think it would be simpler to just remove the timeval argument on
> nis_server_cache_search and move the __clock_gettime64 call on the
> function start. 

Have I understood you correctly that you recommend removing the "now"
struct timeval argument and then call explicitly __clock_gettime64 on
the beginning of nis_server_cache_search function?

> 
> Also, it would require to change nis_server_cache to use a
> __time64_t for 'expires', otherwise this change won't help in
> case of a time_t overflow.
> 

Ok. I will update this. Thanks for pointing this out.

> 
> > diff --git a/sysdeps/generic/hp-timing.h
> > b/sysdeps/generic/hp-timing.h index e2d7447212..af9d92f7f7 100644
> > --- a/sysdeps/generic/hp-timing.h
> > +++ b/sysdeps/generic/hp-timing.h
> > @@ -34,8 +34,8 @@ typedef uint64_t hp_timing_t;
> >     vDSO symbol.  */
> >  #define HP_TIMING_NOW(var) \
> >  ({								\
> > -  struct timespec tv;
> > 	\
> > -  __clock_gettime (CLOCK_MONOTONIC, &tv);			\
> > +  struct __timespec64 tv;
> > 	\
> > +  __clock_gettime64 (CLOCK_MONOTONIC, &tv);
> > \ (var) = (tv.tv_nsec + UINT64_C(1000000000) * tv.tv_sec);	\
> >  })
> >    
> 
> Ok.
> 
> > diff --git a/sysdeps/generic/memusage.h b/sysdeps/generic/memusage.h
> > index a111864b0b..91e56d24de 100644
> > --- a/sysdeps/generic/memusage.h
> > +++ b/sysdeps/generic/memusage.h
> > @@ -28,9 +28,9 @@
> >  #ifndef GETTIME
> >  # define GETTIME(low,high)
> > 	   \ {
> > 			   \
> > -    struct timespec now;
> > 	   \
> > +    struct __timespec64 now;
> > 		   \ uint64_t usecs;
> > 			   \
> > -    clock_gettime (CLOCK_REALTIME, &now);
> > 	   \
> > +    __clock_gettime64 (CLOCK_REALTIME, &now);
> > 		   \ usecs = (uint64_t)now.tv_nsec / 1000 +
> > (uint64_t)now.tv_sec * 1000000; \ low = usecs & 0xffffffff;
> > 					   \ high = usecs >>
> > 32;							   \  
> 
> Is is the requirement to export __clock_gettime64 as a GLIBC_PRIVATE
> symbol?
> 

The __clock_gettime is already exported as GLIBC_PRIVATE at
./time/Versions, so I'm following this pattern.

Moreover, the glibc will not build when __clock_gettime64 is not
exported.

> In any case, I think we should try to avoid use internal symbols even
> for distributed glibc libraries, so I think this change should go
> once we start to export the clock_gettime64 as default symbol.

Am I correct that this is a preprocessor macro, which is in the
exported header?

> 
> > diff --git a/sysdeps/unix/sysv/linux/alpha/osf_gettimeofday.c
> > b/sysdeps/unix/sysv/linux/alpha/osf_gettimeofday.c index
> > 8cf5d303f9..5075ae0444 100644 ---
> > a/sysdeps/unix/sysv/linux/alpha/osf_gettimeofday.c +++
> > b/sysdeps/unix/sysv/linux/alpha/osf_gettimeofday.c @@ -35,8 +35,8
> > @@ __gettimeofday_tv32 (struct __timeval32 *restrict tv32, void
> > *restrict tz) if (__glibc_unlikely (tz != 0)) memset (tz, 0, sizeof
> > (struct timezone)); 
> > -  struct timespec ts;
> > -  __clock_gettime (CLOCK_REALTIME, &ts);
> > +  struct __timespec64 ts;
> > +  __clock_gettime64 (CLOCK_REALTIME, &ts);
> >  
> >    *tv32 = valid_timespec_to_timeval32 (ts);
> >    return 0;  
> 
> Ok.
> 
> > diff --git a/sysdeps/unix/sysv/linux/clock.c
> > b/sysdeps/unix/sysv/linux/clock.c index 24a8df0cf5..157ae8eb3f
> > 100644 --- a/sysdeps/unix/sysv/linux/clock.c
> > +++ b/sysdeps/unix/sysv/linux/clock.c
> > @@ -23,15 +23,12 @@
> >  clock_t
> >  clock (void)
> >  {
> > -  struct timespec ts;
> > +  struct __timespec64 ts;
> >  
> >    _Static_assert (CLOCKS_PER_SEC == 1000000,
> >  		  "CLOCKS_PER_SEC should be 1000000");
> >  
> > -  /* clock_gettime shouldn't fail here since
> > CLOCK_PROCESS_CPUTIME_ID is
> > -     supported since 2.6.12.  Check the return value anyway in
> > case the kernel
> > -     barfs on us for some reason.  */
> > -  if (__glibc_unlikely (__clock_gettime (CLOCK_PROCESS_CPUTIME_ID,
> > &ts) != 0))
> > +  if (__glibc_unlikely (__clock_gettime64
> > (CLOCK_PROCESS_CPUTIME_ID, &ts) != 0)) return (clock_t) -1;
> >  
> >    return (ts.tv_sec * CLOCKS_PER_SEC
> >   
> 
> 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] 24+ messages in thread

* Re: [PATCH v2 4/5] y2038: nscd: Modify nscd_helper to use __clock_gettime64
  2020-05-01 11:30     ` Lukasz Majewski
@ 2020-05-04 12:27       ` Adhemerval Zanella
  2020-05-04 15:26         ` Lukasz Majewski
  0 siblings, 1 reply; 24+ messages in thread
From: Adhemerval Zanella @ 2020-05-04 12:27 UTC (permalink / raw)
  To: Lukasz Majewski
  Cc: Joseph Myers, Alistair Francis, Alistair Francis, GNU C Library,
	Florian Weimer, Andreas Schwab


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



On 01/05/2020 08:30, Lukasz Majewski wrote:
> Hi Adhemerval,
> 
>> On 26/03/2020 05:06, Lukasz Majewski wrote:
>>> The nscd/nscd_helper.c uses __clock_gettime to get current time and
>>> on this basis calculate the relative timeout for poll.
>>> By using __clock_gettime64 on systems with __WORDSIZE == 32 &&
>>> __TIMESIZE != 64 the timeout is correctly calculated after time_t
>>> overflow.  
>>
>> LGTM, thanks.
>>
>>> ---
>>>  nscd/nscd_helper.c | 17 +++++++++--------
>>>  1 file changed, 9 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/nscd/nscd_helper.c b/nscd/nscd_helper.c
>>> index d2d7d15f26..a4f3312f90 100644
>>> --- a/nscd/nscd_helper.c
>>> +++ b/nscd/nscd_helper.c
>>> @@ -37,6 +37,7 @@
>>>  #include <not-cancel.h>
>>>  #include <kernel-features.h>
>>>  #include <nss.h>
>>> +#include <struct___timespec64.h>
>>>  
>>>  #include "nscd-client.h"
>>>  
>>> @@ -59,10 +60,10 @@ wait_on_socket (int sock, long int usectmo)
>>>        /* Handle the case where the poll() call is interrupted by a
>>>  	 signal.  We cannot just use TEMP_FAILURE_RETRY since it
>>> might lead to infinite loops.  */
>>> -      struct timespec now;
>>> -      __clock_gettime (CLOCK_REALTIME, &now);
>>> -      long int end = (now.tv_sec * 1000 + usectmo
>>> -                      + (now.tv_nsec + 500000) / 1000000);
>>> +      struct __timespec64 now;
>>> +      __clock_gettime64 (CLOCK_REALTIME, &now);
>>> +      int64_t end = (now.tv_sec * 1000 + usectmo
>>> +                     + (now.tv_nsec + 500000) / 1000000);
>>>        long int timeout = usectmo;
>>>        while (1)
>>>  	{  
>>
>> Ok. Maybe we could use ppoll instead here to simplify the timeout
>> calculation?
>>
> 
> I wanted to change as little as possible (to not introduce any extra
> bugs) to only replace __clock_gettime with __clock_gettime64. 

I don't have a strong opinion here, it is just that it might simplifies
a bit the timeout handling.

> 
>>> @@ -71,7 +72,7 @@ wait_on_socket (int sock, long int usectmo)
>>>  	    break;
>>>  
>>>  	  /* Recompute the timeout time.  */
>>> -          __clock_gettime (CLOCK_REALTIME, &now);
>>> +          __clock_gettime64 (CLOCK_REALTIME, &now);
>>>  	  timeout = end - ((now.tv_sec * 1000
>>>                              + (now.tv_nsec + 500000) / 1000000));
>>>  	}  
>>
>> Ok.
>>
>>> @@ -193,7 +194,7 @@ open_socket (request_type type, const char
>>> *key, size_t keylen) memcpy (reqdata->key, key, keylen);
>>>  
>>>    bool first_try = true;
>>> -  struct timespec tvend = { 0, 0 };
>>> +  struct __timespec64 tvend = { 0, 0 };
>>>    while (1)
>>>      {  
>>
>> Ok.
>>
>>>  #ifndef MSG_NOSIGNAL
>>> @@ -212,8 +213,8 @@ open_socket (request_type type, const char
>>> *key, size_t keylen) 
>>>        /* The daemon is busy wait for it.  */
>>>        int to;
>>> -      struct timespec now;
>>> -      __clock_gettime (CLOCK_REALTIME, &now);
>>> +      struct __timespec64 now;
>>> +      __clock_gettime64 (CLOCK_REALTIME, &now);
>>>        if (first_try)
>>>  	{
>>>  	  tvend.tv_nsec = now.tv_nsec;
>>>   
>>
>> 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: 833 bytes --]

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 5/5] y2038: Replace __clock_gettime with __clock_gettime64
  2020-05-01 11:56     ` Lukasz Majewski
@ 2020-05-04 14:04       ` Adhemerval Zanella
  2020-05-04 15:32         ` Lukasz Majewski
  2020-05-05 18:31         ` Lukasz Majewski
  0 siblings, 2 replies; 24+ messages in thread
From: Adhemerval Zanella @ 2020-05-04 14:04 UTC (permalink / raw)
  To: Lukasz Majewski
  Cc: Joseph Myers, Alistair Francis, Alistair Francis, GNU C Library,
	Florian Weimer, Andreas Schwab


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



On 01/05/2020 08:56, Lukasz Majewski wrote:
> Hi Adhemerval,
> 
>> On 26/03/2020 05:06, Lukasz Majewski wrote:
>>> diff --git a/nis/nis_call.c b/nis/nis_call.c
>>> index 92c70e97aa..9c6f62a753 100644
>>> --- a/nis/nis_call.c
>>> +++ b/nis/nis_call.c
>>> @@ -709,7 +709,7 @@ __nisfind_server (const_nis_name name, int
>>> search_parent, nis_error status;
>>>    directory_obj *obj;
>>>    struct timeval now;
>>> -  struct timespec ts;
>>> +  struct __timespec64 ts;
>>>    unsigned int server_used = ~0;
>>>    unsigned int current_ep = ~0;
>>>  
>>> @@ -719,8 +719,8 @@ __nisfind_server (const_nis_name name, int
>>> search_parent, if (*dir != NULL)
>>>      return NIS_SUCCESS;
>>>  
>>> -  __clock_gettime (CLOCK_REALTIME, &ts);
>>> -  TIMESPEC_TO_TIMEVAL (&now, &ts);
>>> +  __clock_gettime64 (CLOCK_REALTIME, &ts);
>>> +  now = valid_timespec64_to_timeval (ts);
>>>  
>>>    if ((flags & NO_CACHE) == 0)
>>>      *dir = nis_server_cache_search (name, search_parent,
>>> &server_used,  
>>
>> I think it would be simpler to just remove the timeval argument on
>> nis_server_cache_search and move the __clock_gettime64 call on the
>> function start. 
> 
> Have I understood you correctly that you recommend removing the "now"
> struct timeval argument and then call explicitly __clock_gettime64 on
> the beginning of nis_server_cache_search function?

Yes, the nis_server_cache_search is a static function used only once
at __nisfind_server.

> 
>>
>> Also, it would require to change nis_server_cache to use a
>> __time64_t for 'expires', otherwise this change won't help in
>> case of a time_t overflow.
>>
> 
> Ok. I will update this. Thanks for pointing this out.
> 
>>
>>> diff --git a/sysdeps/generic/hp-timing.h
>>> b/sysdeps/generic/hp-timing.h index e2d7447212..af9d92f7f7 100644
>>> --- a/sysdeps/generic/hp-timing.h
>>> +++ b/sysdeps/generic/hp-timing.h
>>> @@ -34,8 +34,8 @@ typedef uint64_t hp_timing_t;
>>>     vDSO symbol.  */
>>>  #define HP_TIMING_NOW(var) \
>>>  ({								\
>>> -  struct timespec tv;
>>> 	\
>>> -  __clock_gettime (CLOCK_MONOTONIC, &tv);			\
>>> +  struct __timespec64 tv;
>>> 	\
>>> +  __clock_gettime64 (CLOCK_MONOTONIC, &tv);
>>> \ (var) = (tv.tv_nsec + UINT64_C(1000000000) * tv.tv_sec);	\
>>>  })
>>>    
>>
>> Ok.
>>
>>> diff --git a/sysdeps/generic/memusage.h b/sysdeps/generic/memusage.h
>>> index a111864b0b..91e56d24de 100644
>>> --- a/sysdeps/generic/memusage.h
>>> +++ b/sysdeps/generic/memusage.h
>>> @@ -28,9 +28,9 @@
>>>  #ifndef GETTIME
>>>  # define GETTIME(low,high)
>>> 	   \ {
>>> 			   \
>>> -    struct timespec now;
>>> 	   \
>>> +    struct __timespec64 now;
>>> 		   \ uint64_t usecs;
>>> 			   \
>>> -    clock_gettime (CLOCK_REALTIME, &now);
>>> 	   \
>>> +    __clock_gettime64 (CLOCK_REALTIME, &now);
>>> 		   \ usecs = (uint64_t)now.tv_nsec / 1000 +
>>> (uint64_t)now.tv_sec * 1000000; \ low = usecs & 0xffffffff;
>>> 					   \ high = usecs >>
>>> 32;							   \  
>>
>> Is is the requirement to export __clock_gettime64 as a GLIBC_PRIVATE
>> symbol?
>>
> 
> The __clock_gettime is already exported as GLIBC_PRIVATE at
> ./time/Versions, so I'm following this pattern.
> 
> Moreover, the glibc will not build when __clock_gettime64 is not
> exported.
> 
>> In any case, I think we should try to avoid use internal symbols even
>> for distributed glibc libraries, so I think this change should go
>> once we start to export the clock_gettime64 as default symbol.
> 
> Am I correct that this is a preprocessor macro, which is in the
> exported header?

In fact __clock_gettime is used on other internal libraries and it
is required as is to avoid linknamespace pollution.  So it seems
that __clock_gettime64 should follow the same logic.

It might be misleading that for some ABI __clock_gettime and
for other __clock_gettime64 will be used internally, but it should
be ok nonetheless.


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

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 4/5] y2038: nscd: Modify nscd_helper to use __clock_gettime64
  2020-05-04 12:27       ` Adhemerval Zanella
@ 2020-05-04 15:26         ` Lukasz Majewski
  2020-05-04 16:36           ` Adhemerval Zanella
  0 siblings, 1 reply; 24+ messages in thread
From: Lukasz Majewski @ 2020-05-04 15:26 UTC (permalink / raw)
  To: Adhemerval Zanella
  Cc: Joseph Myers, Alistair Francis, Alistair Francis, GNU C Library,
	Florian Weimer, Andreas Schwab

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

Hi Adhemerval,

> On 01/05/2020 08:30, Lukasz Majewski wrote:
> > Hi Adhemerval,
> >   
> >> On 26/03/2020 05:06, Lukasz Majewski wrote:  
> >>> The nscd/nscd_helper.c uses __clock_gettime to get current time
> >>> and on this basis calculate the relative timeout for poll.
> >>> By using __clock_gettime64 on systems with __WORDSIZE == 32 &&
> >>> __TIMESIZE != 64 the timeout is correctly calculated after time_t
> >>> overflow.    
> >>
> >> LGTM, thanks.
> >>  
> >>> ---
> >>>  nscd/nscd_helper.c | 17 +++++++++--------
> >>>  1 file changed, 9 insertions(+), 8 deletions(-)
> >>>
> >>> diff --git a/nscd/nscd_helper.c b/nscd/nscd_helper.c
> >>> index d2d7d15f26..a4f3312f90 100644
> >>> --- a/nscd/nscd_helper.c
> >>> +++ b/nscd/nscd_helper.c
> >>> @@ -37,6 +37,7 @@
> >>>  #include <not-cancel.h>
> >>>  #include <kernel-features.h>
> >>>  #include <nss.h>
> >>> +#include <struct___timespec64.h>
> >>>  
> >>>  #include "nscd-client.h"
> >>>  
> >>> @@ -59,10 +60,10 @@ wait_on_socket (int sock, long int usectmo)
> >>>        /* Handle the case where the poll() call is interrupted by
> >>> a signal.  We cannot just use TEMP_FAILURE_RETRY since it
> >>> might lead to infinite loops.  */
> >>> -      struct timespec now;
> >>> -      __clock_gettime (CLOCK_REALTIME, &now);
> >>> -      long int end = (now.tv_sec * 1000 + usectmo
> >>> -                      + (now.tv_nsec + 500000) / 1000000);
> >>> +      struct __timespec64 now;
> >>> +      __clock_gettime64 (CLOCK_REALTIME, &now);
> >>> +      int64_t end = (now.tv_sec * 1000 + usectmo
> >>> +                     + (now.tv_nsec + 500000) / 1000000);
> >>>        long int timeout = usectmo;
> >>>        while (1)
> >>>  	{    
> >>
> >> Ok. Maybe we could use ppoll instead here to simplify the timeout
> >> calculation?
> >>  
> > 
> > I wanted to change as little as possible (to not introduce any extra
> > bugs) to only replace __clock_gettime with __clock_gettime64.   
> 
> I don't have a strong opinion here, it is just that it might
> simplifies a bit the timeout handling.

If you don't mind I would prefer to change as little as possible and
stick to the approach (changes) proposed in this patch.

Do you agree with such approach?

> 
> >   
> >>> @@ -71,7 +72,7 @@ wait_on_socket (int sock, long int usectmo)
> >>>  	    break;
> >>>  
> >>>  	  /* Recompute the timeout time.  */
> >>> -          __clock_gettime (CLOCK_REALTIME, &now);
> >>> +          __clock_gettime64 (CLOCK_REALTIME, &now);
> >>>  	  timeout = end - ((now.tv_sec * 1000
> >>>                              + (now.tv_nsec + 500000) / 1000000));
> >>>  	}    
> >>
> >> Ok.
> >>  
> >>> @@ -193,7 +194,7 @@ open_socket (request_type type, const char
> >>> *key, size_t keylen) memcpy (reqdata->key, key, keylen);
> >>>  
> >>>    bool first_try = true;
> >>> -  struct timespec tvend = { 0, 0 };
> >>> +  struct __timespec64 tvend = { 0, 0 };
> >>>    while (1)
> >>>      {    
> >>
> >> Ok.
> >>  
> >>>  #ifndef MSG_NOSIGNAL
> >>> @@ -212,8 +213,8 @@ open_socket (request_type type, const char
> >>> *key, size_t keylen) 
> >>>        /* The daemon is busy wait for it.  */
> >>>        int to;
> >>> -      struct timespec now;
> >>> -      __clock_gettime (CLOCK_REALTIME, &now);
> >>> +      struct __timespec64 now;
> >>> +      __clock_gettime64 (CLOCK_REALTIME, &now);
> >>>        if (first_try)
> >>>  	{
> >>>  	  tvend.tv_nsec = now.tv_nsec;
> >>>     
> >>
> >> 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 
> 




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] 24+ messages in thread

* Re: [PATCH v2 5/5] y2038: Replace __clock_gettime with __clock_gettime64
  2020-05-04 14:04       ` Adhemerval Zanella
@ 2020-05-04 15:32         ` Lukasz Majewski
  2020-05-04 16:37           ` Adhemerval Zanella
  2020-05-05 18:31         ` Lukasz Majewski
  1 sibling, 1 reply; 24+ messages in thread
From: Lukasz Majewski @ 2020-05-04 15:32 UTC (permalink / raw)
  To: Adhemerval Zanella
  Cc: Joseph Myers, Alistair Francis, Alistair Francis, GNU C Library,
	Florian Weimer, Andreas Schwab

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

Hi Adhemerval,

> On 01/05/2020 08:56, Lukasz Majewski wrote:
> > Hi Adhemerval,
> >   
> >> On 26/03/2020 05:06, Lukasz Majewski wrote:  
> >>> diff --git a/nis/nis_call.c b/nis/nis_call.c
> >>> index 92c70e97aa..9c6f62a753 100644
> >>> --- a/nis/nis_call.c
> >>> +++ b/nis/nis_call.c
> >>> @@ -709,7 +709,7 @@ __nisfind_server (const_nis_name name, int
> >>> search_parent, nis_error status;
> >>>    directory_obj *obj;
> >>>    struct timeval now;
> >>> -  struct timespec ts;
> >>> +  struct __timespec64 ts;
> >>>    unsigned int server_used = ~0;
> >>>    unsigned int current_ep = ~0;
> >>>  
> >>> @@ -719,8 +719,8 @@ __nisfind_server (const_nis_name name, int
> >>> search_parent, if (*dir != NULL)
> >>>      return NIS_SUCCESS;
> >>>  
> >>> -  __clock_gettime (CLOCK_REALTIME, &ts);
> >>> -  TIMESPEC_TO_TIMEVAL (&now, &ts);
> >>> +  __clock_gettime64 (CLOCK_REALTIME, &ts);
> >>> +  now = valid_timespec64_to_timeval (ts);
> >>>  
> >>>    if ((flags & NO_CACHE) == 0)
> >>>      *dir = nis_server_cache_search (name, search_parent,
> >>> &server_used,    
> >>
> >> I think it would be simpler to just remove the timeval argument on
> >> nis_server_cache_search and move the __clock_gettime64 call on the
> >> function start.   
> > 
> > Have I understood you correctly that you recommend removing the
> > "now" struct timeval argument and then call explicitly
> > __clock_gettime64 on the beginning of nis_server_cache_search
> > function?  
> 
> Yes, the nis_server_cache_search is a static function used only once
> at __nisfind_server.

Ok.

> 
> >   
> >>
> >> Also, it would require to change nis_server_cache to use a
> >> __time64_t for 'expires', otherwise this change won't help in
> >> case of a time_t overflow.
> >>  
> > 
> > Ok. I will update this. Thanks for pointing this out.
> >   
> >>  
> >>> diff --git a/sysdeps/generic/hp-timing.h
> >>> b/sysdeps/generic/hp-timing.h index e2d7447212..af9d92f7f7 100644
> >>> --- a/sysdeps/generic/hp-timing.h
> >>> +++ b/sysdeps/generic/hp-timing.h
> >>> @@ -34,8 +34,8 @@ typedef uint64_t hp_timing_t;
> >>>     vDSO symbol.  */
> >>>  #define HP_TIMING_NOW(var) \
> >>>  ({
> >>> 	\
> >>> -  struct timespec tv;
> >>> 	\
> >>> -  __clock_gettime (CLOCK_MONOTONIC, &tv);
> >>> \
> >>> +  struct __timespec64 tv;
> >>> 	\
> >>> +  __clock_gettime64 (CLOCK_MONOTONIC, &tv);
> >>> \ (var) = (tv.tv_nsec + UINT64_C(1000000000) * tv.tv_sec);
> >>> \ })
> >>>      
> >>
> >> Ok.
> >>  
> >>> diff --git a/sysdeps/generic/memusage.h
> >>> b/sysdeps/generic/memusage.h index a111864b0b..91e56d24de 100644
> >>> --- a/sysdeps/generic/memusage.h
> >>> +++ b/sysdeps/generic/memusage.h
> >>> @@ -28,9 +28,9 @@
> >>>  #ifndef GETTIME
> >>>  # define GETTIME(low,high)
> >>> 	   \ {
> >>> 			   \
> >>> -    struct timespec now;
> >>> 	   \
> >>> +    struct __timespec64 now;
> >>> 		   \ uint64_t usecs;
> >>> 			   \
> >>> -    clock_gettime (CLOCK_REALTIME, &now);
> >>> 	   \
> >>> +    __clock_gettime64 (CLOCK_REALTIME, &now);
> >>> 		   \ usecs = (uint64_t)now.tv_nsec / 1000 +
> >>> (uint64_t)now.tv_sec * 1000000; \ low = usecs & 0xffffffff;
> >>> 					   \ high = usecs >>
> >>> 32;							   \
> >>>  
> >>
> >> Is is the requirement to export __clock_gettime64 as a
> >> GLIBC_PRIVATE symbol?
> >>  
> > 
> > The __clock_gettime is already exported as GLIBC_PRIVATE at
> > ./time/Versions, so I'm following this pattern.
> > 
> > Moreover, the glibc will not build when __clock_gettime64 is not
> > exported.
> >   
> >> In any case, I think we should try to avoid use internal symbols
> >> even for distributed glibc libraries, so I think this change
> >> should go once we start to export the clock_gettime64 as default
> >> symbol.  
> > 
> > Am I correct that this is a preprocessor macro, which is in the
> > exported header?  
> 
> In fact __clock_gettime is used on other internal libraries and it
> is required as is to avoid linknamespace pollution.  So it seems
> that __clock_gettime64 should follow the same logic.

Ok, So then __clock_gettime64 shall be exported as well.

> 
> It might be misleading that for some ABI __clock_gettime and
> for other __clock_gettime64 will be used internally, but it should
> be ok nonetheless.
> 

All the occurrences of __clock_gettime shall be replaced by
__clock_gettime64 internally in glibc as __clock_gettime64 is Y2038
safe for e.g. ARM32 and is aliased to __clock_gettime for 64 bit archs
anyway.


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] 24+ messages in thread

* Re: [PATCH v2 4/5] y2038: nscd: Modify nscd_helper to use __clock_gettime64
  2020-05-04 15:26         ` Lukasz Majewski
@ 2020-05-04 16:36           ` Adhemerval Zanella
  0 siblings, 0 replies; 24+ messages in thread
From: Adhemerval Zanella @ 2020-05-04 16:36 UTC (permalink / raw)
  To: Lukasz Majewski
  Cc: Joseph Myers, Alistair Francis, Alistair Francis, GNU C Library,
	Florian Weimer, Andreas Schwab


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



On 04/05/2020 12:26, Lukasz Majewski wrote:
> Hi Adhemerval,
> 
>> On 01/05/2020 08:30, Lukasz Majewski wrote:
>>> Hi Adhemerval,
>>>   
>>>> On 26/03/2020 05:06, Lukasz Majewski wrote:  
>>>>> The nscd/nscd_helper.c uses __clock_gettime to get current time
>>>>> and on this basis calculate the relative timeout for poll.
>>>>> By using __clock_gettime64 on systems with __WORDSIZE == 32 &&
>>>>> __TIMESIZE != 64 the timeout is correctly calculated after time_t
>>>>> overflow.    
>>>>
>>>> LGTM, thanks.
>>>>  
>>>>> ---
>>>>>  nscd/nscd_helper.c | 17 +++++++++--------
>>>>>  1 file changed, 9 insertions(+), 8 deletions(-)
>>>>>
>>>>> diff --git a/nscd/nscd_helper.c b/nscd/nscd_helper.c
>>>>> index d2d7d15f26..a4f3312f90 100644
>>>>> --- a/nscd/nscd_helper.c
>>>>> +++ b/nscd/nscd_helper.c
>>>>> @@ -37,6 +37,7 @@
>>>>>  #include <not-cancel.h>
>>>>>  #include <kernel-features.h>
>>>>>  #include <nss.h>
>>>>> +#include <struct___timespec64.h>
>>>>>  
>>>>>  #include "nscd-client.h"
>>>>>  
>>>>> @@ -59,10 +60,10 @@ wait_on_socket (int sock, long int usectmo)
>>>>>        /* Handle the case where the poll() call is interrupted by
>>>>> a signal.  We cannot just use TEMP_FAILURE_RETRY since it
>>>>> might lead to infinite loops.  */
>>>>> -      struct timespec now;
>>>>> -      __clock_gettime (CLOCK_REALTIME, &now);
>>>>> -      long int end = (now.tv_sec * 1000 + usectmo
>>>>> -                      + (now.tv_nsec + 500000) / 1000000);
>>>>> +      struct __timespec64 now;
>>>>> +      __clock_gettime64 (CLOCK_REALTIME, &now);
>>>>> +      int64_t end = (now.tv_sec * 1000 + usectmo
>>>>> +                     + (now.tv_nsec + 500000) / 1000000);
>>>>>        long int timeout = usectmo;
>>>>>        while (1)
>>>>>  	{    
>>>>
>>>> Ok. Maybe we could use ppoll instead here to simplify the timeout
>>>> calculation?
>>>>  
>>>
>>> I wanted to change as little as possible (to not introduce any extra
>>> bugs) to only replace __clock_gettime with __clock_gettime64.   
>>
>> I don't have a strong opinion here, it is just that it might
>> simplifies a bit the timeout handling.
> 
> If you don't mind I would prefer to change as little as possible and
> stick to the approach (changes) proposed in this patch.
> 
> Do you agree with such approach?

Ack.


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

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 5/5] y2038: Replace __clock_gettime with __clock_gettime64
  2020-05-04 15:32         ` Lukasz Majewski
@ 2020-05-04 16:37           ` Adhemerval Zanella
  0 siblings, 0 replies; 24+ messages in thread
From: Adhemerval Zanella @ 2020-05-04 16:37 UTC (permalink / raw)
  To: Lukasz Majewski
  Cc: Joseph Myers, Alistair Francis, Alistair Francis, GNU C Library,
	Florian Weimer, Andreas Schwab


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



On 04/05/2020 12:32, Lukasz Majewski wrote:
> Hi Adhemerval,
> 
>> On 01/05/2020 08:56, Lukasz Majewski wrote:
>>> Hi Adhemerval,
>>>   
>>>> On 26/03/2020 05:06, Lukasz Majewski wrote:  
>>>>> diff --git a/nis/nis_call.c b/nis/nis_call.c
>>>>> index 92c70e97aa..9c6f62a753 100644
>>>>> --- a/nis/nis_call.c
>>>>> +++ b/nis/nis_call.c
>>>>> @@ -709,7 +709,7 @@ __nisfind_server (const_nis_name name, int
>>>>> search_parent, nis_error status;
>>>>>    directory_obj *obj;
>>>>>    struct timeval now;
>>>>> -  struct timespec ts;
>>>>> +  struct __timespec64 ts;
>>>>>    unsigned int server_used = ~0;
>>>>>    unsigned int current_ep = ~0;
>>>>>  
>>>>> @@ -719,8 +719,8 @@ __nisfind_server (const_nis_name name, int
>>>>> search_parent, if (*dir != NULL)
>>>>>      return NIS_SUCCESS;
>>>>>  
>>>>> -  __clock_gettime (CLOCK_REALTIME, &ts);
>>>>> -  TIMESPEC_TO_TIMEVAL (&now, &ts);
>>>>> +  __clock_gettime64 (CLOCK_REALTIME, &ts);
>>>>> +  now = valid_timespec64_to_timeval (ts);
>>>>>  
>>>>>    if ((flags & NO_CACHE) == 0)
>>>>>      *dir = nis_server_cache_search (name, search_parent,
>>>>> &server_used,    
>>>>
>>>> I think it would be simpler to just remove the timeval argument on
>>>> nis_server_cache_search and move the __clock_gettime64 call on the
>>>> function start.   
>>>
>>> Have I understood you correctly that you recommend removing the
>>> "now" struct timeval argument and then call explicitly
>>> __clock_gettime64 on the beginning of nis_server_cache_search
>>> function?  
>>
>> Yes, the nis_server_cache_search is a static function used only once
>> at __nisfind_server.
> 
> Ok.
> 
>>
>>>   
>>>>
>>>> Also, it would require to change nis_server_cache to use a
>>>> __time64_t for 'expires', otherwise this change won't help in
>>>> case of a time_t overflow.
>>>>  
>>>
>>> Ok. I will update this. Thanks for pointing this out.
>>>   
>>>>  
>>>>> diff --git a/sysdeps/generic/hp-timing.h
>>>>> b/sysdeps/generic/hp-timing.h index e2d7447212..af9d92f7f7 100644
>>>>> --- a/sysdeps/generic/hp-timing.h
>>>>> +++ b/sysdeps/generic/hp-timing.h
>>>>> @@ -34,8 +34,8 @@ typedef uint64_t hp_timing_t;
>>>>>     vDSO symbol.  */
>>>>>  #define HP_TIMING_NOW(var) \
>>>>>  ({
>>>>> 	\
>>>>> -  struct timespec tv;
>>>>> 	\
>>>>> -  __clock_gettime (CLOCK_MONOTONIC, &tv);
>>>>> \
>>>>> +  struct __timespec64 tv;
>>>>> 	\
>>>>> +  __clock_gettime64 (CLOCK_MONOTONIC, &tv);
>>>>> \ (var) = (tv.tv_nsec + UINT64_C(1000000000) * tv.tv_sec);
>>>>> \ })
>>>>>      
>>>>
>>>> Ok.
>>>>  
>>>>> diff --git a/sysdeps/generic/memusage.h
>>>>> b/sysdeps/generic/memusage.h index a111864b0b..91e56d24de 100644
>>>>> --- a/sysdeps/generic/memusage.h
>>>>> +++ b/sysdeps/generic/memusage.h
>>>>> @@ -28,9 +28,9 @@
>>>>>  #ifndef GETTIME
>>>>>  # define GETTIME(low,high)
>>>>> 	   \ {
>>>>> 			   \
>>>>> -    struct timespec now;
>>>>> 	   \
>>>>> +    struct __timespec64 now;
>>>>> 		   \ uint64_t usecs;
>>>>> 			   \
>>>>> -    clock_gettime (CLOCK_REALTIME, &now);
>>>>> 	   \
>>>>> +    __clock_gettime64 (CLOCK_REALTIME, &now);
>>>>> 		   \ usecs = (uint64_t)now.tv_nsec / 1000 +
>>>>> (uint64_t)now.tv_sec * 1000000; \ low = usecs & 0xffffffff;
>>>>> 					   \ high = usecs >>
>>>>> 32;							   \
>>>>>  
>>>>
>>>> Is is the requirement to export __clock_gettime64 as a
>>>> GLIBC_PRIVATE symbol?
>>>>  
>>>
>>> The __clock_gettime is already exported as GLIBC_PRIVATE at
>>> ./time/Versions, so I'm following this pattern.
>>>
>>> Moreover, the glibc will not build when __clock_gettime64 is not
>>> exported.
>>>   
>>>> In any case, I think we should try to avoid use internal symbols
>>>> even for distributed glibc libraries, so I think this change
>>>> should go once we start to export the clock_gettime64 as default
>>>> symbol.  
>>>
>>> Am I correct that this is a preprocessor macro, which is in the
>>> exported header?  
>>
>> In fact __clock_gettime is used on other internal libraries and it
>> is required as is to avoid linknamespace pollution.  So it seems
>> that __clock_gettime64 should follow the same logic.
> 
> Ok, So then __clock_gettime64 shall be exported as well.
> 
>>
>> It might be misleading that for some ABI __clock_gettime and
>> for other __clock_gettime64 will be used internally, but it should
>> be ok nonetheless.
>>
> 
> All the occurrences of __clock_gettime shall be replaced by
> __clock_gettime64 internally in glibc as __clock_gettime64 is Y2038
> safe for e.g. ARM32 and is aliased to __clock_gettime for 64 bit archs
> anyway.

I meant that some architectures does not provide __clock_gettime64 and
thus the internal symbol will be __clock_gettime. But it is not an
issue in fact.


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

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 5/5] y2038: Replace __clock_gettime with __clock_gettime64
  2020-05-04 14:04       ` Adhemerval Zanella
  2020-05-04 15:32         ` Lukasz Majewski
@ 2020-05-05 18:31         ` Lukasz Majewski
  1 sibling, 0 replies; 24+ messages in thread
From: Lukasz Majewski @ 2020-05-05 18:31 UTC (permalink / raw)
  To: Adhemerval Zanella
  Cc: Joseph Myers, Alistair Francis, Alistair Francis, GNU C Library,
	Florian Weimer, Andreas Schwab

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

Hi Adhemerval,

> On 01/05/2020 08:56, Lukasz Majewski wrote:
> > Hi Adhemerval,
> >   
> >> On 26/03/2020 05:06, Lukasz Majewski wrote:  
> >>> diff --git a/nis/nis_call.c b/nis/nis_call.c
> >>> index 92c70e97aa..9c6f62a753 100644
> >>> --- a/nis/nis_call.c
> >>> +++ b/nis/nis_call.c
> >>> @@ -709,7 +709,7 @@ __nisfind_server (const_nis_name name, int
> >>> search_parent, nis_error status;
> >>>    directory_obj *obj;
> >>>    struct timeval now;
> >>> -  struct timespec ts;
> >>> +  struct __timespec64 ts;
> >>>    unsigned int server_used = ~0;
> >>>    unsigned int current_ep = ~0;
> >>>  
> >>> @@ -719,8 +719,8 @@ __nisfind_server (const_nis_name name, int
> >>> search_parent, if (*dir != NULL)
> >>>      return NIS_SUCCESS;
> >>>  
> >>> -  __clock_gettime (CLOCK_REALTIME, &ts);
> >>> -  TIMESPEC_TO_TIMEVAL (&now, &ts);
> >>> +  __clock_gettime64 (CLOCK_REALTIME, &ts);
> >>> +  now = valid_timespec64_to_timeval (ts);
> >>>  
> >>>    if ((flags & NO_CACHE) == 0)
> >>>      *dir = nis_server_cache_search (name, search_parent,
> >>> &server_used,    
> >>
> >> I think it would be simpler to just remove the timeval argument on
> >> nis_server_cache_search and move the __clock_gettime64 call on the
> >> function start.   
> > 
> > Have I understood you correctly that you recommend removing the
> > "now" struct timeval argument and then call explicitly
> > __clock_gettime64 on the beginning of nis_server_cache_search
> > function?  
> 
> Yes, the nis_server_cache_search is a static function used only once
> at __nisfind_server.

I've looked into the code again and it seems like in the
__nisfind_server there are two static functions which use struct
timeval *now:

- nis_server_cache_search (...., &now)

- nis_server_cache_add (.... , &now)

Both of them are static functions. 

I would propose instead leaving the &now argument, but replace it with
struct timespec as in the above function we do compare expire with
tv_sec member of struct timeval (which can be easily changed to struct
timespec).

(I've just sent a v3 of this patch to mailing list)

> 
> >   
> >>
> >> Also, it would require to change nis_server_cache to use a
> >> __time64_t for 'expires', otherwise this change won't help in
> >> case of a time_t overflow.
> >>  
> > 
> > Ok. I will update this. Thanks for pointing this out.
> >   
> >>  
> >>> diff --git a/sysdeps/generic/hp-timing.h
> >>> b/sysdeps/generic/hp-timing.h index e2d7447212..af9d92f7f7 100644
> >>> --- a/sysdeps/generic/hp-timing.h
> >>> +++ b/sysdeps/generic/hp-timing.h
> >>> @@ -34,8 +34,8 @@ typedef uint64_t hp_timing_t;
> >>>     vDSO symbol.  */
> >>>  #define HP_TIMING_NOW(var) \
> >>>  ({
> >>> 	\
> >>> -  struct timespec tv;
> >>> 	\
> >>> -  __clock_gettime (CLOCK_MONOTONIC, &tv);
> >>> \
> >>> +  struct __timespec64 tv;
> >>> 	\
> >>> +  __clock_gettime64 (CLOCK_MONOTONIC, &tv);
> >>> \ (var) = (tv.tv_nsec + UINT64_C(1000000000) * tv.tv_sec);
> >>> \ })
> >>>      
> >>
> >> Ok.
> >>  
> >>> diff --git a/sysdeps/generic/memusage.h
> >>> b/sysdeps/generic/memusage.h index a111864b0b..91e56d24de 100644
> >>> --- a/sysdeps/generic/memusage.h
> >>> +++ b/sysdeps/generic/memusage.h
> >>> @@ -28,9 +28,9 @@
> >>>  #ifndef GETTIME
> >>>  # define GETTIME(low,high)
> >>> 	   \ {
> >>> 			   \
> >>> -    struct timespec now;
> >>> 	   \
> >>> +    struct __timespec64 now;
> >>> 		   \ uint64_t usecs;
> >>> 			   \
> >>> -    clock_gettime (CLOCK_REALTIME, &now);
> >>> 	   \
> >>> +    __clock_gettime64 (CLOCK_REALTIME, &now);
> >>> 		   \ usecs = (uint64_t)now.tv_nsec / 1000 +
> >>> (uint64_t)now.tv_sec * 1000000; \ low = usecs & 0xffffffff;
> >>> 					   \ high = usecs >>
> >>> 32;							   \
> >>>  
> >>
> >> Is is the requirement to export __clock_gettime64 as a
> >> GLIBC_PRIVATE symbol?
> >>  
> > 
> > The __clock_gettime is already exported as GLIBC_PRIVATE at
> > ./time/Versions, so I'm following this pattern.
> > 
> > Moreover, the glibc will not build when __clock_gettime64 is not
> > exported.
> >   
> >> In any case, I think we should try to avoid use internal symbols
> >> even for distributed glibc libraries, so I think this change
> >> should go once we start to export the clock_gettime64 as default
> >> symbol.  
> > 
> > Am I correct that this is a preprocessor macro, which is in the
> > exported header?  
> 
> In fact __clock_gettime is used on other internal libraries and it
> is required as is to avoid linknamespace pollution.  So it seems
> that __clock_gettime64 should follow the same logic.
> 
> It might be misleading that for some ABI __clock_gettime and
> for other __clock_gettime64 will be used internally, but it should
> be ok nonetheless.
> 




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] 24+ messages in thread

end of thread, other threads:[~2020-05-05 18:31 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-26  8:06 [PATCH v2 0/5] y2038: Replace __clock_gettime with __clock_gettime64 Lukasz Majewski
2020-03-26  8:06 ` [PATCH v2 1/5] y2038: Export __clock_gettime64 to be usable in other libraries Lukasz Majewski
2020-03-26  8:06 ` [PATCH v2 2/5] y2038: hurd: Provide __clock_gettime64 function Lukasz Majewski
2020-03-26  8:06 ` [PATCH v2 3/5] y2038: inet: Convert inet deadline to support 64 bit time Lukasz Majewski
2020-04-30 20:51   ` Adhemerval Zanella
2020-03-26  8:06 ` [PATCH v2 4/5] y2038: nscd: Modify nscd_helper to use __clock_gettime64 Lukasz Majewski
2020-04-30 20:54   ` Adhemerval Zanella
2020-05-01 11:30     ` Lukasz Majewski
2020-05-04 12:27       ` Adhemerval Zanella
2020-05-04 15:26         ` Lukasz Majewski
2020-05-04 16:36           ` Adhemerval Zanella
2020-03-26  8:06 ` [PATCH v2 5/5] y2038: Replace __clock_gettime with __clock_gettime64 Lukasz Majewski
2020-04-06 16:07   ` Florian Weimer
2020-04-06 21:03     ` Lukasz Majewski
2020-04-14 12:14       ` Lukasz Majewski
2020-04-21 13:48         ` Lukasz Majewski
2020-04-29 21:48           ` Lukasz Majewski
2020-04-30 21:03   ` Adhemerval Zanella
2020-05-01 11:56     ` Lukasz Majewski
2020-05-04 14:04       ` Adhemerval Zanella
2020-05-04 15:32         ` Lukasz Majewski
2020-05-04 16:37           ` Adhemerval Zanella
2020-05-05 18:31         ` Lukasz Majewski
2020-04-06 15:16 ` [PATCH v2 0/5] " Lukasz Majewski

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).