[PATCHv2,1/4] Cleanup clock_*time includes
diff mbox series

Message ID DB5PR08MB10300F4B507034FBE758D011839B0@DB5PR08MB1030.eurprd08.prod.outlook.com
State New
Headers show
Series
  • [PATCHv2,1/4] Cleanup clock_*time includes
Related show

Commit Message

Wilco Dijkstra Jan. 25, 2019, 4:15 p.m. UTC
(resend patch as part of hp-timing cleanup)

Clock_gettime, settime and getres implementations are unnecessarily
complex due to using many defines and C file inclusion.  Simplify the
code by replacing the redundant defines and removing the inclusion,
making it much easier to understand. No functional changes.

OK for commit (for 2.30)?

ChangeLog:
2019-01-14  Wilco Dijkstra  <wdijkstr@arm.com>

	* sysdeps/posix/clock_getres.c (__clock_getres): Cleanup.
	* sysdeps/unix/clock_gettime.c (__clock_gettime): Cleanup.
	* sysdeps/unix/clock_settime.c (__clock_settime): Cleanup.
	* sysdeps/unix/sysv/linux/clock_getres.c (__clock_getres): Cleanup.
	* sysdeps/unix/sysv/linux/clock_gettime.c (__clock_gettime): Cleanup.
	* sysdeps/unix/sysv/linux/clock_settime.c (__clock_settime): Cleanup.

--

Comments

Adhemerval Zanella Jan. 28, 2019, 5:08 p.m. UTC | #1
On 25/01/2019 14:15, Wilco Dijkstra wrote:
> (resend patch as part of hp-timing cleanup)
> 
> Clock_gettime, settime and getres implementations are unnecessarily
> complex due to using many defines and C file inclusion.  Simplify the
> code by replacing the redundant defines and removing the inclusion,
> making it much easier to understand. No functional changes.
> 
> OK for commit (for 2.30)?

https://sourceware.org/ml/libc-alpha/2019-01/msg00364.html

> 
> ChangeLog:
> 2019-01-14  Wilco Dijkstra  <wdijkstr@arm.com>
> 
> 	* sysdeps/posix/clock_getres.c (__clock_getres): Cleanup.
> 	* sysdeps/unix/clock_gettime.c (__clock_gettime): Cleanup.
> 	* sysdeps/unix/clock_settime.c (__clock_settime): Cleanup.
> 	* sysdeps/unix/sysv/linux/clock_getres.c (__clock_getres): Cleanup.
> 	* sysdeps/unix/sysv/linux/clock_gettime.c (__clock_gettime): Cleanup.
> 	* sysdeps/unix/sysv/linux/clock_settime.c (__clock_settime): Cleanup.
> 
> --
> diff --git a/sysdeps/posix/clock_getres.c b/sysdeps/posix/clock_getres.c
> index f47715ed6b48a54248fb375fbbeb55008b995104..dac4761fcc2c86cb0d53425cab4e88d587696540 100644
> --- a/sysdeps/posix/clock_getres.c
> +++ b/sysdeps/posix/clock_getres.c
> @@ -82,20 +82,11 @@ __clock_getres (clockid_t clock_id, struct timespec *res)
>  
>    switch (clock_id)
>      {
> -#ifdef SYSDEP_GETRES
> -      SYSDEP_GETRES;
> -#endif
> -
> -#ifndef HANDLED_REALTIME
>      case CLOCK_REALTIME:
>        retval = realtime_getres (res);
>        break;
> -#endif	/* handled REALTIME */
>  
>      default:
> -#ifdef SYSDEP_GETRES_CPU
> -      SYSDEP_GETRES_CPU;
> -#endif
>  #if HP_TIMING_AVAIL
>        if ((clock_id & ((1 << CLOCK_IDFIELD_SIZE) - 1))
>  	  == CLOCK_THREAD_CPUTIME_ID)
> @@ -105,7 +96,7 @@ __clock_getres (clockid_t clock_id, struct timespec *res)
>  	__set_errno (EINVAL);
>        break;
>  
> -#if HP_TIMING_AVAIL && !defined HANDLED_CPUTIME
> +#if HP_TIMING_AVAIL
>      case CLOCK_PROCESS_CPUTIME_ID:
>      case CLOCK_THREAD_CPUTIME_ID:
>        retval = hp_timing_getres (res);
> diff --git a/sysdeps/unix/clock_gettime.c b/sysdeps/unix/clock_gettime.c
> index 138991d1bc081cea004e94defe361580dfb022dc..33a1f3335c269eec2b4f01fb927e3836a191b5eb 100644
> --- a/sysdeps/unix/clock_gettime.c
> +++ b/sysdeps/unix/clock_gettime.c
> @@ -95,11 +95,6 @@ __clock_gettime (clockid_t clock_id, struct timespec *tp)
>  
>    switch (clock_id)
>      {
> -#ifdef SYSDEP_GETTIME
> -      SYSDEP_GETTIME;
> -#endif
> -
> -#ifndef HANDLED_REALTIME
>      case CLOCK_REALTIME:
>        {
>  	struct timeval tv;
> @@ -108,12 +103,8 @@ __clock_gettime (clockid_t clock_id, struct timespec *tp)
>  	  TIMEVAL_TO_TIMESPEC (&tv, tp);
>        }
>        break;
> -#endif
>  
>      default:
> -#ifdef SYSDEP_GETTIME_CPU
> -      SYSDEP_GETTIME_CPU (clock_id, tp);
> -#endif
>  #if HP_TIMING_AVAIL
>        if ((clock_id & ((1 << CLOCK_IDFIELD_SIZE) - 1))
>  	  == CLOCK_THREAD_CPUTIME_ID)
> @@ -123,7 +114,7 @@ __clock_gettime (clockid_t clock_id, struct timespec *tp)
>  	__set_errno (EINVAL);
>        break;
>  
> -#if HP_TIMING_AVAIL && !defined HANDLED_CPUTIME
> +#if HP_TIMING_AVAIL
>      case CLOCK_PROCESS_CPUTIME_ID:
>        retval = hp_timing_gettime (clock_id, tp);
>        break;
> diff --git a/sysdeps/unix/clock_settime.c b/sysdeps/unix/clock_settime.c
> index b589e875cf2dfb5fc083550e01e774a6f4d0cae6..24d73b048c1b891906dacdf0c199544b85d924d2 100644
> --- a/sysdeps/unix/clock_settime.c
> +++ b/sysdeps/unix/clock_settime.c
> @@ -21,7 +21,7 @@
>  #include <ldsodefs.h>
>  
>  
> -#if HP_TIMING_AVAIL && !defined HANDLED_CPUTIME
> +#if HP_TIMING_AVAIL
>  /* Clock frequency of the processor.  We make it a 64-bit variable
>     because some jokers are already playing with processors with more
>     than 4GHz.  */
> @@ -84,29 +84,15 @@ __clock_settime (clockid_t clock_id, const struct timespec *tp)
>  
>    switch (clock_id)
>      {
> -#define HANDLE_REALTIME \
> -      do {								      \
> -	struct timeval tv;						      \
> -	TIMESPEC_TO_TIMEVAL (&tv, tp);					      \
> -									      \
> -	retval = __settimeofday (&tv, NULL);				      \
> -      } while (0)
> -
> -#ifdef SYSDEP_SETTIME
> -      SYSDEP_SETTIME;
> -#endif
> -
> -#ifndef HANDLED_REALTIME
>      case CLOCK_REALTIME:
> -      HANDLE_REALTIME;
> +      do {
> +	struct timeval tv;
> +	TIMESPEC_TO_TIMEVAL (&tv, tp);
> +	retval = __settimeofday (&tv, NULL);
> +      } while (0);
>        break;
> -#endif
>  
>      default:
> -#ifdef SYSDEP_SETTIME_CPU
> -      SYSDEP_SETTIME_CPU;
> -#endif
> -#ifndef HANDLED_CPUTIME
>  # if HP_TIMING_AVAIL
>        if (CPUCLOCK_WHICH (clock_id) == CLOCK_PROCESS_CPUTIME_ID
>  	  || CPUCLOCK_WHICH (clock_id) == CLOCK_THREAD_CPUTIME_ID)
> @@ -117,7 +103,6 @@ __clock_settime (clockid_t clock_id, const struct timespec *tp)
>  	  __set_errno (EINVAL);
>  	  retval = -1;
>  	}
> -#endif
>        break;
>      }
>  
> diff --git a/sysdeps/unix/sysv/linux/clock_getres.c b/sysdeps/unix/sysv/linux/clock_getres.c
> index a24532888fd3f3e256f34fb5a7636cee8b0b7904..bb2ee109bd66f916cc790899fbc205c8890098ae 100644
> --- a/sysdeps/unix/sysv/linux/clock_getres.c
> +++ b/sysdeps/unix/sysv/linux/clock_getres.c
> @@ -26,26 +26,11 @@
>  #endif
>  #include <sysdep-vdso.h>
>  
> -#define SYSCALL_GETRES \
> -  retval = INLINE_VSYSCALL (clock_getres, 2, clock_id, res); \
> -  break
> +/* Get resolution of clock.  */
> +int
> +__clock_getres (clockid_t clock_id, struct timespec *res)
> +{
> +  return INLINE_VSYSCALL (clock_getres, 2, clock_id, res);
> +}
> +weak_alias (__clock_getres, clock_getres)
>  
> -/* The REALTIME and MONOTONIC clock are definitely supported in the
> -   kernel.  */
> -#define SYSDEP_GETRES							      \
> -  SYSDEP_GETRES_CPUTIME							      \
> -  case CLOCK_REALTIME:							      \
> -  case CLOCK_MONOTONIC:							      \
> -  case CLOCK_MONOTONIC_RAW:						      \
> -  case CLOCK_REALTIME_COARSE:						      \
> -  case CLOCK_MONOTONIC_COARSE:						      \
> -    SYSCALL_GETRES
> -
> -/* We handled the REALTIME clock here.  */
> -#define HANDLED_REALTIME	1
> -#define HANDLED_CPUTIME		1
> -
> -#define SYSDEP_GETRES_CPU SYSCALL_GETRES
> -#define SYSDEP_GETRES_CPUTIME	/* Default catches them too.  */
> -
> -#include <sysdeps/posix/clock_getres.c>
> diff --git a/sysdeps/unix/sysv/linux/clock_gettime.c b/sysdeps/unix/sysv/linux/clock_gettime.c
> index 9af806f76ef34ad711a9f94de77599d6e17fe421..2ad7846a4d811635c40daab62bc0eae9d7d3b7ac 100644
> --- a/sysdeps/unix/sysv/linux/clock_gettime.c
> +++ b/sysdeps/unix/sysv/linux/clock_gettime.c
> @@ -26,22 +26,12 @@
>  #endif
>  #include <sysdep-vdso.h>
>  
> -/* The REALTIME and MONOTONIC clock are definitely supported in the
> -   kernel.  */
> -#define SYSDEP_GETTIME \
> -  SYSDEP_GETTIME_CPUTIME;						      \
> -  case CLOCK_REALTIME:							      \
> -  case CLOCK_MONOTONIC:							      \
> -    retval = INLINE_VSYSCALL (clock_gettime, 2, clock_id, tp);		      \
> -    break
> +/* Get current value of CLOCK and store it in TP.  */
> +int
> +__clock_gettime (clockid_t clock_id, struct timespec *tp)
> +{
> +  return INLINE_VSYSCALL (clock_gettime, 2, clock_id, tp);
> +}
> +weak_alias (__clock_gettime, clock_gettime)
> +libc_hidden_def (__clock_gettime)
>  
> -/* We handled the REALTIME clock here.  */
> -#define HANDLED_REALTIME	1
> -#define HANDLED_CPUTIME	1
> -
> -#define SYSDEP_GETTIME_CPU(clock_id, tp) \
> -  retval = INLINE_VSYSCALL (clock_gettime, 2, clock_id, tp); \
> -  break
> -#define SYSDEP_GETTIME_CPUTIME	/* Default catches them too.  */
> -
> -#include <sysdeps/unix/clock_gettime.c>
> diff --git a/sysdeps/unix/sysv/linux/clock_settime.c b/sysdeps/unix/sysv/linux/clock_settime.c
> index 7e2432b244a40ac620c39010a7e0a556f73a9c79..c5668012e0819059f9bfdd684616e0c9add2b115 100644
> --- a/sysdeps/unix/sysv/linux/clock_settime.c
> +++ b/sysdeps/unix/sysv/linux/clock_settime.c
> @@ -21,18 +21,18 @@
>  
>  #include "kernel-posix-cpu-timers.h"
>  
> +/* Set CLOCK to value TP.  */
> +int
> +__clock_settime (clockid_t clock_id, const struct timespec *tp)
> +{
> +  /* Make sure the time cvalue is OK.  */
> +  if (tp->tv_nsec < 0 || tp->tv_nsec >= 1000000000)
> +    {
> +      __set_errno (EINVAL);
> +      return -1;
> +    }
> +
> +  return INLINE_SYSCALL (clock_settime, 2, clock_id, tp);
> +}
> +weak_alias (__clock_settime, clock_settime)
>  
> -/* The REALTIME clock is definitely supported in the kernel.  */
> -#define SYSDEP_SETTIME \
> -  case CLOCK_REALTIME:							      \
> -    retval = INLINE_SYSCALL (clock_settime, 2, clock_id, tp);		      \
> -    break
> -
> -/* We handled the REALTIME clock here.  */
> -#define HANDLED_REALTIME	1
> -
> -#define HANDLED_CPUTIME 1
> -#define SYSDEP_SETTIME_CPU \
> -  retval = INLINE_SYSCALL (clock_settime, 2, clock_id, tp)
> -
> -#include <sysdeps/unix/clock_settime.c>
>
Wilco Dijkstra Jan. 29, 2019, 2:07 p.m. UTC | #2
Hi Adhemerval,

> https://sourceware.org/ml/libc-alpha/2019-01/msg00364.html

Thanks, I missed your review. Here is the updated version:

Clock_gettime, settime and getres implementations are unnecessarily
complex due to using many defines and C file inclusion.  Simplify the
code by replacing the redundant defines and removing the inclusion,
making it much easier to understand. No functional changes.

OK for commit?

ChangeLog:
2019-01-29  Wilco Dijkstra  <wdijkstr@arm.com>

	* sysdeps/posix/clock_getres.c (__clock_getres): Cleanup.
	* sysdeps/unix/clock_gettime.c (__clock_gettime): Cleanup.
	* sysdeps/unix/clock_settime.c (__clock_settime): Cleanup.
	* sysdeps/unix/sysv/linux/clock_getres.c (__clock_getres): Cleanup.
	* sysdeps/unix/sysv/linux/clock_gettime.c (__clock_gettime): Cleanup.
	* sysdeps/unix/sysv/linux/clock_settime.c (__clock_settime): Cleanup.

--

diff --git a/sysdeps/posix/clock_getres.c b/sysdeps/posix/clock_getres.c
index f47715ed6b48a54248fb375fbbeb55008b995104..dac4761fcc2c86cb0d53425cab4e88d587696540 100644
--- a/sysdeps/posix/clock_getres.c
+++ b/sysdeps/posix/clock_getres.c
@@ -82,20 +82,11 @@ __clock_getres (clockid_t clock_id, struct timespec *res)
 
   switch (clock_id)
     {
-#ifdef SYSDEP_GETRES
-      SYSDEP_GETRES;
-#endif
-
-#ifndef HANDLED_REALTIME
     case CLOCK_REALTIME:
       retval = realtime_getres (res);
       break;
-#endif	/* handled REALTIME */
 
     default:
-#ifdef SYSDEP_GETRES_CPU
-      SYSDEP_GETRES_CPU;
-#endif
 #if HP_TIMING_AVAIL
       if ((clock_id & ((1 << CLOCK_IDFIELD_SIZE) - 1))
 	  == CLOCK_THREAD_CPUTIME_ID)
@@ -105,7 +96,7 @@ __clock_getres (clockid_t clock_id, struct timespec *res)
 	__set_errno (EINVAL);
       break;
 
-#if HP_TIMING_AVAIL && !defined HANDLED_CPUTIME
+#if HP_TIMING_AVAIL
     case CLOCK_PROCESS_CPUTIME_ID:
     case CLOCK_THREAD_CPUTIME_ID:
       retval = hp_timing_getres (res);
diff --git a/sysdeps/unix/clock_gettime.c b/sysdeps/unix/clock_gettime.c
index 138991d1bc081cea004e94defe361580dfb022dc..33a1f3335c269eec2b4f01fb927e3836a191b5eb 100644
--- a/sysdeps/unix/clock_gettime.c
+++ b/sysdeps/unix/clock_gettime.c
@@ -95,11 +95,6 @@ __clock_gettime (clockid_t clock_id, struct timespec *tp)
 
   switch (clock_id)
     {
-#ifdef SYSDEP_GETTIME
-      SYSDEP_GETTIME;
-#endif
-
-#ifndef HANDLED_REALTIME
     case CLOCK_REALTIME:
       {
 	struct timeval tv;
@@ -108,12 +103,8 @@ __clock_gettime (clockid_t clock_id, struct timespec *tp)
 	  TIMEVAL_TO_TIMESPEC (&tv, tp);
       }
       break;
-#endif
 
     default:
-#ifdef SYSDEP_GETTIME_CPU
-      SYSDEP_GETTIME_CPU (clock_id, tp);
-#endif
 #if HP_TIMING_AVAIL
       if ((clock_id & ((1 << CLOCK_IDFIELD_SIZE) - 1))
 	  == CLOCK_THREAD_CPUTIME_ID)
@@ -123,7 +114,7 @@ __clock_gettime (clockid_t clock_id, struct timespec *tp)
 	__set_errno (EINVAL);
       break;
 
-#if HP_TIMING_AVAIL && !defined HANDLED_CPUTIME
+#if HP_TIMING_AVAIL
     case CLOCK_PROCESS_CPUTIME_ID:
       retval = hp_timing_gettime (clock_id, tp);
       break;
diff --git a/sysdeps/unix/clock_settime.c b/sysdeps/unix/clock_settime.c
index b589e875cf2dfb5fc083550e01e774a6f4d0cae6..dcf9ff660ad9a7ad4022aa695ee92575c8875b5c 100644
--- a/sysdeps/unix/clock_settime.c
+++ b/sysdeps/unix/clock_settime.c
@@ -21,7 +21,7 @@
 #include <ldsodefs.h>
 
 
-#if HP_TIMING_AVAIL && !defined HANDLED_CPUTIME
+#if HP_TIMING_AVAIL
 /* Clock frequency of the processor.  We make it a 64-bit variable
    because some jokers are already playing with processors with more
    than 4GHz.  */
@@ -84,29 +84,15 @@ __clock_settime (clockid_t clock_id, const struct timespec *tp)
 
   switch (clock_id)
     {
-#define HANDLE_REALTIME \
-      do {								      \
-	struct timeval tv;						      \
-	TIMESPEC_TO_TIMEVAL (&tv, tp);					      \
-									      \
-	retval = __settimeofday (&tv, NULL);				      \
-      } while (0)
-
-#ifdef SYSDEP_SETTIME
-      SYSDEP_SETTIME;
-#endif
-
-#ifndef HANDLED_REALTIME
     case CLOCK_REALTIME:
-      HANDLE_REALTIME;
+      {
+	struct timeval tv;
+	TIMESPEC_TO_TIMEVAL (&tv, tp);
+	retval = __settimeofday (&tv, NULL);
+      }
       break;
-#endif
 
     default:
-#ifdef SYSDEP_SETTIME_CPU
-      SYSDEP_SETTIME_CPU;
-#endif
-#ifndef HANDLED_CPUTIME
 # if HP_TIMING_AVAIL
       if (CPUCLOCK_WHICH (clock_id) == CLOCK_PROCESS_CPUTIME_ID
 	  || CPUCLOCK_WHICH (clock_id) == CLOCK_THREAD_CPUTIME_ID)
@@ -117,7 +103,6 @@ __clock_settime (clockid_t clock_id, const struct timespec *tp)
 	  __set_errno (EINVAL);
 	  retval = -1;
 	}
-#endif
       break;
     }
 
diff --git a/sysdeps/unix/sysv/linux/clock_getres.c b/sysdeps/unix/sysv/linux/clock_getres.c
index a24532888fd3f3e256f34fb5a7636cee8b0b7904..bb2ee109bd66f916cc790899fbc205c8890098ae 100644
--- a/sysdeps/unix/sysv/linux/clock_getres.c
+++ b/sysdeps/unix/sysv/linux/clock_getres.c
@@ -26,26 +26,11 @@
 #endif
 #include <sysdep-vdso.h>
 
-#define SYSCALL_GETRES \
-  retval = INLINE_VSYSCALL (clock_getres, 2, clock_id, res); \
-  break
+/* Get resolution of clock.  */
+int
+__clock_getres (clockid_t clock_id, struct timespec *res)
+{
+  return INLINE_VSYSCALL (clock_getres, 2, clock_id, res);
+}
+weak_alias (__clock_getres, clock_getres)
 
-/* The REALTIME and MONOTONIC clock are definitely supported in the
-   kernel.  */
-#define SYSDEP_GETRES							      \
-  SYSDEP_GETRES_CPUTIME							      \
-  case CLOCK_REALTIME:							      \
-  case CLOCK_MONOTONIC:							      \
-  case CLOCK_MONOTONIC_RAW:						      \
-  case CLOCK_REALTIME_COARSE:						      \
-  case CLOCK_MONOTONIC_COARSE:						      \
-    SYSCALL_GETRES
-
-/* We handled the REALTIME clock here.  */
-#define HANDLED_REALTIME	1
-#define HANDLED_CPUTIME		1
-
-#define SYSDEP_GETRES_CPU SYSCALL_GETRES
-#define SYSDEP_GETRES_CPUTIME	/* Default catches them too.  */
-
-#include <sysdeps/posix/clock_getres.c>
diff --git a/sysdeps/unix/sysv/linux/clock_gettime.c b/sysdeps/unix/sysv/linux/clock_gettime.c
index 9af806f76ef34ad711a9f94de77599d6e17fe421..2ad7846a4d811635c40daab62bc0eae9d7d3b7ac 100644
--- a/sysdeps/unix/sysv/linux/clock_gettime.c
+++ b/sysdeps/unix/sysv/linux/clock_gettime.c
@@ -26,22 +26,12 @@
 #endif
 #include <sysdep-vdso.h>
 
-/* The REALTIME and MONOTONIC clock are definitely supported in the
-   kernel.  */
-#define SYSDEP_GETTIME \
-  SYSDEP_GETTIME_CPUTIME;						      \
-  case CLOCK_REALTIME:							      \
-  case CLOCK_MONOTONIC:							      \
-    retval = INLINE_VSYSCALL (clock_gettime, 2, clock_id, tp);		      \
-    break
+/* Get current value of CLOCK and store it in TP.  */
+int
+__clock_gettime (clockid_t clock_id, struct timespec *tp)
+{
+  return INLINE_VSYSCALL (clock_gettime, 2, clock_id, tp);
+}
+weak_alias (__clock_gettime, clock_gettime)
+libc_hidden_def (__clock_gettime)
 
-/* We handled the REALTIME clock here.  */
-#define HANDLED_REALTIME	1
-#define HANDLED_CPUTIME	1
-
-#define SYSDEP_GETTIME_CPU(clock_id, tp) \
-  retval = INLINE_VSYSCALL (clock_gettime, 2, clock_id, tp); \
-  break
-#define SYSDEP_GETTIME_CPUTIME	/* Default catches them too.  */
-
-#include <sysdeps/unix/clock_gettime.c>
diff --git a/sysdeps/unix/sysv/linux/clock_settime.c b/sysdeps/unix/sysv/linux/clock_settime.c
index 7e2432b244a40ac620c39010a7e0a556f73a9c79..729a4b20e5d208b9618d2b5a4a682f377c446b2b 100644
--- a/sysdeps/unix/sysv/linux/clock_settime.c
+++ b/sysdeps/unix/sysv/linux/clock_settime.c
@@ -21,18 +21,18 @@
 
 #include "kernel-posix-cpu-timers.h"
 
+/* Set CLOCK to value TP.  */
+int
+__clock_settime (clockid_t clock_id, const struct timespec *tp)
+{
+  /* Make sure the time cvalue is OK.  */
+  if (tp->tv_nsec < 0 || tp->tv_nsec >= 1000000000)
+    {
+      __set_errno (EINVAL);
+      return -1;
+    }
+
+  return INLINE_SYSCALL_CALL (clock_settime, clock_id, tp);
+}
+weak_alias (__clock_settime, clock_settime)
 
-/* The REALTIME clock is definitely supported in the kernel.  */
-#define SYSDEP_SETTIME \
-  case CLOCK_REALTIME:							      \
-    retval = INLINE_SYSCALL (clock_settime, 2, clock_id, tp);		      \
-    break
-
-/* We handled the REALTIME clock here.  */
-#define HANDLED_REALTIME	1
-
-#define HANDLED_CPUTIME 1
-#define SYSDEP_SETTIME_CPU \
-  retval = INLINE_SYSCALL (clock_settime, 2, clock_id, tp)
-
-#include <sysdeps/unix/clock_settime.c>
Adhemerval Zanella Jan. 29, 2019, 7:11 p.m. UTC | #3
On 29/01/2019 12:07, Wilco Dijkstra wrote:
> Hi Adhemerval,
> 
>> https://sourceware.org/ml/libc-alpha/2019-01/msg00364.html
> 
> Thanks, I missed your review. Here is the updated version:
> 
> Clock_gettime, settime and getres implementations are unnecessarily
> complex due to using many defines and C file inclusion.  Simplify the
> code by replacing the redundant defines and removing the inclusion,
> making it much easier to understand. No functional changes.
> 
> OK for commit?

LGTM when 2.30 opens, thanks.

> 
> ChangeLog:
> 2019-01-29  Wilco Dijkstra  <wdijkstr@arm.com>
> 
> 	* sysdeps/posix/clock_getres.c (__clock_getres): Cleanup.
> 	* sysdeps/unix/clock_gettime.c (__clock_gettime): Cleanup.
> 	* sysdeps/unix/clock_settime.c (__clock_settime): Cleanup.
> 	* sysdeps/unix/sysv/linux/clock_getres.c (__clock_getres): Cleanup.
> 	* sysdeps/unix/sysv/linux/clock_gettime.c (__clock_gettime): Cleanup.
> 	* sysdeps/unix/sysv/linux/clock_settime.c (__clock_settime): Cleanup.
> 
> --
> 
> diff --git a/sysdeps/posix/clock_getres.c b/sysdeps/posix/clock_getres.c
> index f47715ed6b48a54248fb375fbbeb55008b995104..dac4761fcc2c86cb0d53425cab4e88d587696540 100644
> --- a/sysdeps/posix/clock_getres.c
> +++ b/sysdeps/posix/clock_getres.c
> @@ -82,20 +82,11 @@ __clock_getres (clockid_t clock_id, struct timespec *res)
>  
>    switch (clock_id)
>      {
> -#ifdef SYSDEP_GETRES
> -      SYSDEP_GETRES;
> -#endif
> -
> -#ifndef HANDLED_REALTIME
>      case CLOCK_REALTIME:
>        retval = realtime_getres (res);
>        break;
> -#endif	/* handled REALTIME */
>  
>      default:
> -#ifdef SYSDEP_GETRES_CPU
> -      SYSDEP_GETRES_CPU;
> -#endif
>  #if HP_TIMING_AVAIL
>        if ((clock_id & ((1 << CLOCK_IDFIELD_SIZE) - 1))
>  	  == CLOCK_THREAD_CPUTIME_ID)
> @@ -105,7 +96,7 @@ __clock_getres (clockid_t clock_id, struct timespec *res)
>  	__set_errno (EINVAL);
>        break;
>  
> -#if HP_TIMING_AVAIL && !defined HANDLED_CPUTIME
> +#if HP_TIMING_AVAIL
>      case CLOCK_PROCESS_CPUTIME_ID:
>      case CLOCK_THREAD_CPUTIME_ID:
>        retval = hp_timing_getres (res);
> diff --git a/sysdeps/unix/clock_gettime.c b/sysdeps/unix/clock_gettime.c
> index 138991d1bc081cea004e94defe361580dfb022dc..33a1f3335c269eec2b4f01fb927e3836a191b5eb 100644
> --- a/sysdeps/unix/clock_gettime.c
> +++ b/sysdeps/unix/clock_gettime.c
> @@ -95,11 +95,6 @@ __clock_gettime (clockid_t clock_id, struct timespec *tp)
>  
>    switch (clock_id)
>      {
> -#ifdef SYSDEP_GETTIME
> -      SYSDEP_GETTIME;
> -#endif
> -
> -#ifndef HANDLED_REALTIME
>      case CLOCK_REALTIME:
>        {
>  	struct timeval tv;
> @@ -108,12 +103,8 @@ __clock_gettime (clockid_t clock_id, struct timespec *tp)
>  	  TIMEVAL_TO_TIMESPEC (&tv, tp);
>        }
>        break;
> -#endif
>  
>      default:
> -#ifdef SYSDEP_GETTIME_CPU
> -      SYSDEP_GETTIME_CPU (clock_id, tp);
> -#endif
>  #if HP_TIMING_AVAIL
>        if ((clock_id & ((1 << CLOCK_IDFIELD_SIZE) - 1))
>  	  == CLOCK_THREAD_CPUTIME_ID)
> @@ -123,7 +114,7 @@ __clock_gettime (clockid_t clock_id, struct timespec *tp)
>  	__set_errno (EINVAL);
>        break;
>  
> -#if HP_TIMING_AVAIL && !defined HANDLED_CPUTIME
> +#if HP_TIMING_AVAIL
>      case CLOCK_PROCESS_CPUTIME_ID:
>        retval = hp_timing_gettime (clock_id, tp);
>        break;
> diff --git a/sysdeps/unix/clock_settime.c b/sysdeps/unix/clock_settime.c
> index b589e875cf2dfb5fc083550e01e774a6f4d0cae6..dcf9ff660ad9a7ad4022aa695ee92575c8875b5c 100644
> --- a/sysdeps/unix/clock_settime.c
> +++ b/sysdeps/unix/clock_settime.c
> @@ -21,7 +21,7 @@
>  #include <ldsodefs.h>
>  
>  
> -#if HP_TIMING_AVAIL && !defined HANDLED_CPUTIME
> +#if HP_TIMING_AVAIL
>  /* Clock frequency of the processor.  We make it a 64-bit variable
>     because some jokers are already playing with processors with more
>     than 4GHz.  */
> @@ -84,29 +84,15 @@ __clock_settime (clockid_t clock_id, const struct timespec *tp)
>  
>    switch (clock_id)
>      {
> -#define HANDLE_REALTIME \
> -      do {								      \
> -	struct timeval tv;						      \
> -	TIMESPEC_TO_TIMEVAL (&tv, tp);					      \
> -									      \
> -	retval = __settimeofday (&tv, NULL);				      \
> -      } while (0)
> -
> -#ifdef SYSDEP_SETTIME
> -      SYSDEP_SETTIME;
> -#endif
> -
> -#ifndef HANDLED_REALTIME
>      case CLOCK_REALTIME:
> -      HANDLE_REALTIME;
> +      {
> +	struct timeval tv;
> +	TIMESPEC_TO_TIMEVAL (&tv, tp);
> +	retval = __settimeofday (&tv, NULL);
> +      }
>        break;
> -#endif
>  
>      default:
> -#ifdef SYSDEP_SETTIME_CPU
> -      SYSDEP_SETTIME_CPU;
> -#endif
> -#ifndef HANDLED_CPUTIME
>  # if HP_TIMING_AVAIL
>        if (CPUCLOCK_WHICH (clock_id) == CLOCK_PROCESS_CPUTIME_ID
>  	  || CPUCLOCK_WHICH (clock_id) == CLOCK_THREAD_CPUTIME_ID)
> @@ -117,7 +103,6 @@ __clock_settime (clockid_t clock_id, const struct timespec *tp)
>  	  __set_errno (EINVAL);
>  	  retval = -1;
>  	}
> -#endif
>        break;
>      }
>  
> diff --git a/sysdeps/unix/sysv/linux/clock_getres.c b/sysdeps/unix/sysv/linux/clock_getres.c
> index a24532888fd3f3e256f34fb5a7636cee8b0b7904..bb2ee109bd66f916cc790899fbc205c8890098ae 100644
> --- a/sysdeps/unix/sysv/linux/clock_getres.c
> +++ b/sysdeps/unix/sysv/linux/clock_getres.c
> @@ -26,26 +26,11 @@
>  #endif
>  #include <sysdep-vdso.h>
>  
> -#define SYSCALL_GETRES \
> -  retval = INLINE_VSYSCALL (clock_getres, 2, clock_id, res); \
> -  break
> +/* Get resolution of clock.  */
> +int
> +__clock_getres (clockid_t clock_id, struct timespec *res)
> +{
> +  return INLINE_VSYSCALL (clock_getres, 2, clock_id, res);
> +}
> +weak_alias (__clock_getres, clock_getres)
>  
> -/* The REALTIME and MONOTONIC clock are definitely supported in the
> -   kernel.  */
> -#define SYSDEP_GETRES							      \
> -  SYSDEP_GETRES_CPUTIME							      \
> -  case CLOCK_REALTIME:							      \
> -  case CLOCK_MONOTONIC:							      \
> -  case CLOCK_MONOTONIC_RAW:						      \
> -  case CLOCK_REALTIME_COARSE:						      \
> -  case CLOCK_MONOTONIC_COARSE:						      \
> -    SYSCALL_GETRES
> -
> -/* We handled the REALTIME clock here.  */
> -#define HANDLED_REALTIME	1
> -#define HANDLED_CPUTIME		1
> -
> -#define SYSDEP_GETRES_CPU SYSCALL_GETRES
> -#define SYSDEP_GETRES_CPUTIME	/* Default catches them too.  */
> -
> -#include <sysdeps/posix/clock_getres.c>
> diff --git a/sysdeps/unix/sysv/linux/clock_gettime.c b/sysdeps/unix/sysv/linux/clock_gettime.c
> index 9af806f76ef34ad711a9f94de77599d6e17fe421..2ad7846a4d811635c40daab62bc0eae9d7d3b7ac 100644
> --- a/sysdeps/unix/sysv/linux/clock_gettime.c
> +++ b/sysdeps/unix/sysv/linux/clock_gettime.c
> @@ -26,22 +26,12 @@
>  #endif
>  #include <sysdep-vdso.h>
>  
> -/* The REALTIME and MONOTONIC clock are definitely supported in the
> -   kernel.  */
> -#define SYSDEP_GETTIME \
> -  SYSDEP_GETTIME_CPUTIME;						      \
> -  case CLOCK_REALTIME:							      \
> -  case CLOCK_MONOTONIC:							      \
> -    retval = INLINE_VSYSCALL (clock_gettime, 2, clock_id, tp);		      \
> -    break
> +/* Get current value of CLOCK and store it in TP.  */
> +int
> +__clock_gettime (clockid_t clock_id, struct timespec *tp)
> +{
> +  return INLINE_VSYSCALL (clock_gettime, 2, clock_id, tp);
> +}
> +weak_alias (__clock_gettime, clock_gettime)
> +libc_hidden_def (__clock_gettime)
>  
> -/* We handled the REALTIME clock here.  */
> -#define HANDLED_REALTIME	1
> -#define HANDLED_CPUTIME	1
> -
> -#define SYSDEP_GETTIME_CPU(clock_id, tp) \
> -  retval = INLINE_VSYSCALL (clock_gettime, 2, clock_id, tp); \
> -  break
> -#define SYSDEP_GETTIME_CPUTIME	/* Default catches them too.  */
> -
> -#include <sysdeps/unix/clock_gettime.c>
> diff --git a/sysdeps/unix/sysv/linux/clock_settime.c b/sysdeps/unix/sysv/linux/clock_settime.c
> index 7e2432b244a40ac620c39010a7e0a556f73a9c79..729a4b20e5d208b9618d2b5a4a682f377c446b2b 100644
> --- a/sysdeps/unix/sysv/linux/clock_settime.c
> +++ b/sysdeps/unix/sysv/linux/clock_settime.c
> @@ -21,18 +21,18 @@
>  
>  #include "kernel-posix-cpu-timers.h"
>  
> +/* Set CLOCK to value TP.  */
> +int
> +__clock_settime (clockid_t clock_id, const struct timespec *tp)
> +{
> +  /* Make sure the time cvalue is OK.  */
> +  if (tp->tv_nsec < 0 || tp->tv_nsec >= 1000000000)
> +    {
> +      __set_errno (EINVAL);
> +      return -1;
> +    }
> +
> +  return INLINE_SYSCALL_CALL (clock_settime, clock_id, tp);
> +}
> +weak_alias (__clock_settime, clock_settime)
>  
> -/* The REALTIME clock is definitely supported in the kernel.  */
> -#define SYSDEP_SETTIME \
> -  case CLOCK_REALTIME:							      \
> -    retval = INLINE_SYSCALL (clock_settime, 2, clock_id, tp);		      \
> -    break
> -
> -/* We handled the REALTIME clock here.  */
> -#define HANDLED_REALTIME	1
> -
> -#define HANDLED_CPUTIME 1
> -#define SYSDEP_SETTIME_CPU \
> -  retval = INLINE_SYSCALL (clock_settime, 2, clock_id, tp)
> -
> -#include <sysdeps/unix/clock_settime.c>
>

Patch
diff mbox series

diff --git a/sysdeps/posix/clock_getres.c b/sysdeps/posix/clock_getres.c
index f47715ed6b48a54248fb375fbbeb55008b995104..dac4761fcc2c86cb0d53425cab4e88d587696540 100644
--- a/sysdeps/posix/clock_getres.c
+++ b/sysdeps/posix/clock_getres.c
@@ -82,20 +82,11 @@  __clock_getres (clockid_t clock_id, struct timespec *res)
 
   switch (clock_id)
     {
-#ifdef SYSDEP_GETRES
-      SYSDEP_GETRES;
-#endif
-
-#ifndef HANDLED_REALTIME
     case CLOCK_REALTIME:
       retval = realtime_getres (res);
       break;
-#endif	/* handled REALTIME */
 
     default:
-#ifdef SYSDEP_GETRES_CPU
-      SYSDEP_GETRES_CPU;
-#endif
 #if HP_TIMING_AVAIL
       if ((clock_id & ((1 << CLOCK_IDFIELD_SIZE) - 1))
 	  == CLOCK_THREAD_CPUTIME_ID)
@@ -105,7 +96,7 @@  __clock_getres (clockid_t clock_id, struct timespec *res)
 	__set_errno (EINVAL);
       break;
 
-#if HP_TIMING_AVAIL && !defined HANDLED_CPUTIME
+#if HP_TIMING_AVAIL
     case CLOCK_PROCESS_CPUTIME_ID:
     case CLOCK_THREAD_CPUTIME_ID:
       retval = hp_timing_getres (res);
diff --git a/sysdeps/unix/clock_gettime.c b/sysdeps/unix/clock_gettime.c
index 138991d1bc081cea004e94defe361580dfb022dc..33a1f3335c269eec2b4f01fb927e3836a191b5eb 100644
--- a/sysdeps/unix/clock_gettime.c
+++ b/sysdeps/unix/clock_gettime.c
@@ -95,11 +95,6 @@  __clock_gettime (clockid_t clock_id, struct timespec *tp)
 
   switch (clock_id)
     {
-#ifdef SYSDEP_GETTIME
-      SYSDEP_GETTIME;
-#endif
-
-#ifndef HANDLED_REALTIME
     case CLOCK_REALTIME:
       {
 	struct timeval tv;
@@ -108,12 +103,8 @@  __clock_gettime (clockid_t clock_id, struct timespec *tp)
 	  TIMEVAL_TO_TIMESPEC (&tv, tp);
       }
       break;
-#endif
 
     default:
-#ifdef SYSDEP_GETTIME_CPU
-      SYSDEP_GETTIME_CPU (clock_id, tp);
-#endif
 #if HP_TIMING_AVAIL
       if ((clock_id & ((1 << CLOCK_IDFIELD_SIZE) - 1))
 	  == CLOCK_THREAD_CPUTIME_ID)
@@ -123,7 +114,7 @@  __clock_gettime (clockid_t clock_id, struct timespec *tp)
 	__set_errno (EINVAL);
       break;
 
-#if HP_TIMING_AVAIL && !defined HANDLED_CPUTIME
+#if HP_TIMING_AVAIL
     case CLOCK_PROCESS_CPUTIME_ID:
       retval = hp_timing_gettime (clock_id, tp);
       break;
diff --git a/sysdeps/unix/clock_settime.c b/sysdeps/unix/clock_settime.c
index b589e875cf2dfb5fc083550e01e774a6f4d0cae6..24d73b048c1b891906dacdf0c199544b85d924d2 100644
--- a/sysdeps/unix/clock_settime.c
+++ b/sysdeps/unix/clock_settime.c
@@ -21,7 +21,7 @@ 
 #include <ldsodefs.h>
 
 
-#if HP_TIMING_AVAIL && !defined HANDLED_CPUTIME
+#if HP_TIMING_AVAIL
 /* Clock frequency of the processor.  We make it a 64-bit variable
    because some jokers are already playing with processors with more
    than 4GHz.  */
@@ -84,29 +84,15 @@  __clock_settime (clockid_t clock_id, const struct timespec *tp)
 
   switch (clock_id)
     {
-#define HANDLE_REALTIME \
-      do {								      \
-	struct timeval tv;						      \
-	TIMESPEC_TO_TIMEVAL (&tv, tp);					      \
-									      \
-	retval = __settimeofday (&tv, NULL);				      \
-      } while (0)
-
-#ifdef SYSDEP_SETTIME
-      SYSDEP_SETTIME;
-#endif
-
-#ifndef HANDLED_REALTIME
     case CLOCK_REALTIME:
-      HANDLE_REALTIME;
+      do {
+	struct timeval tv;
+	TIMESPEC_TO_TIMEVAL (&tv, tp);
+	retval = __settimeofday (&tv, NULL);
+      } while (0);
       break;
-#endif
 
     default:
-#ifdef SYSDEP_SETTIME_CPU
-      SYSDEP_SETTIME_CPU;
-#endif
-#ifndef HANDLED_CPUTIME
 # if HP_TIMING_AVAIL
       if (CPUCLOCK_WHICH (clock_id) == CLOCK_PROCESS_CPUTIME_ID
 	  || CPUCLOCK_WHICH (clock_id) == CLOCK_THREAD_CPUTIME_ID)
@@ -117,7 +103,6 @@  __clock_settime (clockid_t clock_id, const struct timespec *tp)
 	  __set_errno (EINVAL);
 	  retval = -1;
 	}
-#endif
       break;
     }
 
diff --git a/sysdeps/unix/sysv/linux/clock_getres.c b/sysdeps/unix/sysv/linux/clock_getres.c
index a24532888fd3f3e256f34fb5a7636cee8b0b7904..bb2ee109bd66f916cc790899fbc205c8890098ae 100644
--- a/sysdeps/unix/sysv/linux/clock_getres.c
+++ b/sysdeps/unix/sysv/linux/clock_getres.c
@@ -26,26 +26,11 @@ 
 #endif
 #include <sysdep-vdso.h>
 
-#define SYSCALL_GETRES \
-  retval = INLINE_VSYSCALL (clock_getres, 2, clock_id, res); \
-  break
+/* Get resolution of clock.  */
+int
+__clock_getres (clockid_t clock_id, struct timespec *res)
+{
+  return INLINE_VSYSCALL (clock_getres, 2, clock_id, res);
+}
+weak_alias (__clock_getres, clock_getres)
 
-/* The REALTIME and MONOTONIC clock are definitely supported in the
-   kernel.  */
-#define SYSDEP_GETRES							      \
-  SYSDEP_GETRES_CPUTIME							      \
-  case CLOCK_REALTIME:							      \
-  case CLOCK_MONOTONIC:							      \
-  case CLOCK_MONOTONIC_RAW:						      \
-  case CLOCK_REALTIME_COARSE:						      \
-  case CLOCK_MONOTONIC_COARSE:						      \
-    SYSCALL_GETRES
-
-/* We handled the REALTIME clock here.  */
-#define HANDLED_REALTIME	1
-#define HANDLED_CPUTIME		1
-
-#define SYSDEP_GETRES_CPU SYSCALL_GETRES
-#define SYSDEP_GETRES_CPUTIME	/* Default catches them too.  */
-
-#include <sysdeps/posix/clock_getres.c>
diff --git a/sysdeps/unix/sysv/linux/clock_gettime.c b/sysdeps/unix/sysv/linux/clock_gettime.c
index 9af806f76ef34ad711a9f94de77599d6e17fe421..2ad7846a4d811635c40daab62bc0eae9d7d3b7ac 100644
--- a/sysdeps/unix/sysv/linux/clock_gettime.c
+++ b/sysdeps/unix/sysv/linux/clock_gettime.c
@@ -26,22 +26,12 @@ 
 #endif
 #include <sysdep-vdso.h>
 
-/* The REALTIME and MONOTONIC clock are definitely supported in the
-   kernel.  */
-#define SYSDEP_GETTIME \
-  SYSDEP_GETTIME_CPUTIME;						      \
-  case CLOCK_REALTIME:							      \
-  case CLOCK_MONOTONIC:							      \
-    retval = INLINE_VSYSCALL (clock_gettime, 2, clock_id, tp);		      \
-    break
+/* Get current value of CLOCK and store it in TP.  */
+int
+__clock_gettime (clockid_t clock_id, struct timespec *tp)
+{
+  return INLINE_VSYSCALL (clock_gettime, 2, clock_id, tp);
+}
+weak_alias (__clock_gettime, clock_gettime)
+libc_hidden_def (__clock_gettime)
 
-/* We handled the REALTIME clock here.  */
-#define HANDLED_REALTIME	1
-#define HANDLED_CPUTIME	1
-
-#define SYSDEP_GETTIME_CPU(clock_id, tp) \
-  retval = INLINE_VSYSCALL (clock_gettime, 2, clock_id, tp); \
-  break
-#define SYSDEP_GETTIME_CPUTIME	/* Default catches them too.  */
-
-#include <sysdeps/unix/clock_gettime.c>
diff --git a/sysdeps/unix/sysv/linux/clock_settime.c b/sysdeps/unix/sysv/linux/clock_settime.c
index 7e2432b244a40ac620c39010a7e0a556f73a9c79..c5668012e0819059f9bfdd684616e0c9add2b115 100644
--- a/sysdeps/unix/sysv/linux/clock_settime.c
+++ b/sysdeps/unix/sysv/linux/clock_settime.c
@@ -21,18 +21,18 @@ 
 
 #include "kernel-posix-cpu-timers.h"
 
+/* Set CLOCK to value TP.  */
+int
+__clock_settime (clockid_t clock_id, const struct timespec *tp)
+{
+  /* Make sure the time cvalue is OK.  */
+  if (tp->tv_nsec < 0 || tp->tv_nsec >= 1000000000)
+    {
+      __set_errno (EINVAL);
+      return -1;
+    }
+
+  return INLINE_SYSCALL (clock_settime, 2, clock_id, tp);
+}
+weak_alias (__clock_settime, clock_settime)
 
-/* The REALTIME clock is definitely supported in the kernel.  */
-#define SYSDEP_SETTIME \
-  case CLOCK_REALTIME:							      \
-    retval = INLINE_SYSCALL (clock_settime, 2, clock_id, tp);		      \
-    break
-
-/* We handled the REALTIME clock here.  */
-#define HANDLED_REALTIME	1
-
-#define HANDLED_CPUTIME 1
-#define SYSDEP_SETTIME_CPU \
-  retval = INLINE_SYSCALL (clock_settime, 2, clock_id, tp)
-
-#include <sysdeps/unix/clock_settime.c>