[v2] time: Fix compile error in itimer test affecting hurd
Checks
Commit Message
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(-)
Comments
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
* 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
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
@@ -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);
@@ -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);