[PATCH v2] time: Fix compile error in itimer test affecting hurd

[PATCH v2] time: Fix compile error in itimer test affecting hurd

[Stafford Horne]

The recent change to use __KERNEL_OLD_TIMEVAL_MATCHES_TIMEVAL64 to avoid
doing 64-bit checks on some platforms broke the test for hurd where
__KERNEL_OLD_TIMEVAL_MATCHES_TIMEVAL64 is not defined.  With error:

    tst-itimer.c: In function 'do_test':
    tst-itimer.c:103:11: error: '__KERNEL_OLD_TIMEVAL_MATCHES_TIMEVAL64' undeclared (first use in this function)
      103 |       if (__KERNEL_OLD_TIMEVAL_MATCHES_TIMEVAL64)
	  |           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    tst-itimer.c:103:11: note: each undeclared identifier is reported only once for each function it appears in

Define a support helper to detect when setitimer and getitimer support
64-bit time_t.

Fixes commit 6e8a0aac2f ("time: Fix overflow itimer tests on 32-bit
systems").

Cc: Adhemerval Zanella <adhemerval.zanella@linaro.org>
Cc: Joseph Myers <joseph@codesourcery.com>
---
Changes since v1:
 - Move macro to support helper function.

 support/support.h | 12 ++++++++++++
 time/tst-itimer.c |  5 +++--
 2 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/support/support.h b/support/support.h
index db264e3db7..7daa2c48c9 100644
--- a/support/support.h
+++ b/support/support.h
@@ -141,6 +141,18 @@ static __inline bool support_path_support_time64 (const char *path)
 					    0x80000002ULL);
 }
 
+/* Return true if the setitimer and getitimer syscalls support 64-bit time_t
+   values without resulting in overflow.  This is not true on some linux systems
+   which have 64-bit time_t due to legacy kernel API's.  */
+static __inline bool support_itimer_support_time64 (void)
+{
+#ifdef __KERNEL_OLD_TIMEVAL_MATCHES_TIMEVAL64
+  return __KERNEL_OLD_TIMEVAL_MATCHES_TIMEVAL64;
+#else
+  return (sizeof (__time_t) == 8);
+#endif
+}
+
 /* Return true if stat supports nanoseconds resolution.  */
 extern bool support_stat_nanoseconds (const char *path);
 
diff --git a/time/tst-itimer.c b/time/tst-itimer.c
index bd7d7afe83..c6d623cb19 100644
--- a/time/tst-itimer.c
+++ b/time/tst-itimer.c
@@ -21,6 +21,7 @@
 #include <stdlib.h>
 #include <sys/time.h>
 #include <support/check.h>
+#include <support/support.h>
 #include <support/xsignal.h>
 #include <unistd.h>
 #include <time.h>
@@ -100,7 +101,7 @@ do_test (void)
 
       /* Linux does not provide 64 bit time_t support for getitimer and
 	 setitimer on architectures with 32 bit time_t support.  */
-      if (__KERNEL_OLD_TIMEVAL_MATCHES_TIMEVAL64)
+      if (support_itimer_support_time64())
 	{
 	  TEST_COMPARE (setitimer (timers[i], &it, NULL), 0);
 	  TEST_COMPARE (setitimer (timers[i], &(struct itimerval) { 0 },
@@ -131,7 +132,7 @@ do_test (void)
       it.it_interval.tv_usec = 20;
       it.it_value.tv_sec = 30;
       it.it_value.tv_usec = 40;
-      if (__KERNEL_OLD_TIMEVAL_MATCHES_TIMEVAL64)
+      if (support_itimer_support_time64())
 	{
 	  TEST_COMPARE (setitimer (timers[i], &it, NULL), 0);
 
-- 
2.31.1

Re: [PATCH v2] time: Fix compile error in itimer test affecting hurd

[Stafford Horne]

On Fri, Aug 20, 2021 at 06:28:00AM +0900, Stafford Horne wrote:
> The recent change to use __KERNEL_OLD_TIMEVAL_MATCHES_TIMEVAL64 to avoid
> doing 64-bit checks on some platforms broke the test for hurd where
> __KERNEL_OLD_TIMEVAL_MATCHES_TIMEVAL64 is not defined.  With error:
> 
>     tst-itimer.c: In function 'do_test':
>     tst-itimer.c:103:11: error: '__KERNEL_OLD_TIMEVAL_MATCHES_TIMEVAL64' undeclared (first use in this function)
>       103 |       if (__KERNEL_OLD_TIMEVAL_MATCHES_TIMEVAL64)
> 	  |           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>     tst-itimer.c:103:11: note: each undeclared identifier is reported only once for each function it appears in
> 
> Define a support helper to detect when setitimer and getitimer support
> 64-bit time_t.
> 
> Fixes commit 6e8a0aac2f ("time: Fix overflow itimer tests on 32-bit
> systems").
> 
> Cc: Adhemerval Zanella <adhemerval.zanella@linaro.org>
> Cc: Joseph Myers <joseph@codesourcery.com>
> ---
> Changes since v1:
>  - Move macro to support helper function.
> 
>  support/support.h | 12 ++++++++++++
>  time/tst-itimer.c |  5 +++--
>  2 files changed, 15 insertions(+), 2 deletions(-)
> 
> diff --git a/support/support.h b/support/support.h
> index db264e3db7..7daa2c48c9 100644
> --- a/support/support.h
> +++ b/support/support.h
> @@ -141,6 +141,18 @@ static __inline bool support_path_support_time64 (const char *path)
>  					    0x80000002ULL);
>  }
>  
> +/* Return true if the setitimer and getitimer syscalls support 64-bit time_t
> +   values without resulting in overflow.  This is not true on some linux systems
> +   which have 64-bit time_t due to legacy kernel API's.  */
> +static __inline bool support_itimer_support_time64 (void)
> +{
> +#ifdef __KERNEL_OLD_TIMEVAL_MATCHES_TIMEVAL64
> +  return __KERNEL_OLD_TIMEVAL_MATCHES_TIMEVAL64;
> +#else
> +  return (sizeof (__time_t) == 8);
> +#endif
> +}
> +
>  /* Return true if stat supports nanoseconds resolution.  */
>  extern bool support_stat_nanoseconds (const char *path);
>  
> diff --git a/time/tst-itimer.c b/time/tst-itimer.c
> index bd7d7afe83..c6d623cb19 100644
> --- a/time/tst-itimer.c
> +++ b/time/tst-itimer.c
> @@ -21,6 +21,7 @@
>  #include <stdlib.h>
>  #include <sys/time.h>
>  #include <support/check.h>
> +#include <support/support.h>
>  #include <support/xsignal.h>
>  #include <unistd.h>
>  #include <time.h>
> @@ -100,7 +101,7 @@ do_test (void)
>  
>        /* Linux does not provide 64 bit time_t support for getitimer and
>  	 setitimer on architectures with 32 bit time_t support.  */
> -      if (__KERNEL_OLD_TIMEVAL_MATCHES_TIMEVAL64)
> +      if (support_itimer_support_time64())
>  	{
>  	  TEST_COMPARE (setitimer (timers[i], &it, NULL), 0);
>  	  TEST_COMPARE (setitimer (timers[i], &(struct itimerval) { 0 },
> @@ -131,7 +132,7 @@ do_test (void)
>        it.it_interval.tv_usec = 20;
>        it.it_value.tv_sec = 30;
>        it.it_value.tv_usec = 40;
> -      if (__KERNEL_OLD_TIMEVAL_MATCHES_TIMEVAL64)
> +      if (support_itimer_support_time64())
>  	{
>  	  TEST_COMPARE (setitimer (timers[i], &it, NULL), 0);
>  
> -- 
> 2.31.1

This is still outstanding.  OK?

-Stafford

Re: [PATCH v2] time: Fix compile error in itimer test affecting hurd

[Florian Weimer]

* Stafford Horne via Libc-alpha:

> +/* Return true if the setitimer and getitimer syscalls support 64-bit time_t
> +   values without resulting in overflow.  This is not true on some linux systems
> +   which have 64-bit time_t due to legacy kernel API's.  */
> +static __inline bool support_itimer_support_time64 (void)
> +{
> +#ifdef __KERNEL_OLD_TIMEVAL_MATCHES_TIMEVAL64
> +  return __KERNEL_OLD_TIMEVAL_MATCHES_TIMEVAL64;
> +#else
> +  return (sizeof (__time_t) == 8);
> +#endif
> +}

Extra parentheses for the second return.  I haven't followed development
in this area and I'm not sure if that's following the direction others
prefer.  I'd say wait a few more days for comments, and if there are
none, commit with the parentheses removed.

Thanks,
Florian

Re: [PATCH v2] time: Fix compile error in itimer test affecting hurd

[Stafford Horne]

On Tue, Sep 07, 2021 at 09:24:43AM +0200, Florian Weimer wrote:
> * Stafford Horne via Libc-alpha:
> 
> > +/* Return true if the setitimer and getitimer syscalls support 64-bit time_t
> > +   values without resulting in overflow.  This is not true on some linux systems
> > +   which have 64-bit time_t due to legacy kernel API's.  */
> > +static __inline bool support_itimer_support_time64 (void)
> > +{
> > +#ifdef __KERNEL_OLD_TIMEVAL_MATCHES_TIMEVAL64
> > +  return __KERNEL_OLD_TIMEVAL_MATCHES_TIMEVAL64;
> > +#else
> > +  return (sizeof (__time_t) == 8);
> > +#endif
> > +}
> 
> Extra parentheses for the second return.

Ah, yes that carried over fromt when I defined as a macro, will fix it.

> I haven't followed development
> in this area and I'm not sure if that's following the direction others
> prefer.  I'd say wait a few more days for comments, and if there are
> none, commit with the parentheses removed.

Thanks, I'll wait a few more days.

-Stafford