Cleanup clock_*time includes

Message ID DB5PR08MB103032CD99D614F18FC2D15383800@DB5PR08MB1030.eurprd08.prod.outlook.com
State Superseded
Headers show
Series
  • Cleanup clock_*time includes
Related show

Commit Message

Wilco Dijkstra Jan. 14, 2019, 3:36 p.m.
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. 15, 2019, 10:10 p.m. | #1
On 14/01/2019 13:36, Wilco Dijkstra wrote:
> 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.


LGTM with two nits below.

> 

> --

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


Ok.

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


Ok.

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


No need to use 'do ... while (0)' here, just add the brackets.

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

>      }

>  


Ok.

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


Ok.

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


Ok.

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


Use INLINE_SYSCALL_CALL (clock_settime, clock_id, tp).

>  

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

> 


Ok.

Patch

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>