[07/12] Use clock_gettime to implement ftime.
diff mbox series

Message ID 20190820132152.24100-8-zackw@panix.com
State New
Headers show
Series
  • Y2038 preparation: use clock_[gs]ettime to implement the other time-getting and -setting functions
Related show

Commit Message

Zack Weinberg Aug. 20, 2019, 1:21 p.m. UTC
ftime is an obsolete variation on gettimeofday, offering only
millisecond time resolution; it was probably a system call in ooold
versions of BSD Unix.  For historic reasons, we had three
implementations of it.  These are all consolidated into time/ftime.c.

Like gettimeofday, ftime tries to report the time zone, and using that
information is always a bug.  This patch dummies out the reported
timezone information; the ‘timezone’ and ‘dstflag’ fields of the
returned ‘struct timeb’ will always be zero.

(There is an argument for turning this function into a compat symbol,
and not installing sys/timeb.h anymore.  Thoughts?)

	* time/ftime.c (ftime): Replace implementation with the code
	formerly in sysdeps/unix/bsd/ftime.c, then change that code to use
	__clock_gettime instead of __gettimeofday.  Always set the
	timezone and dstflag fields of the ‘timebuf’ argument to zero.

	* sysdeps/unix/bsd/ftime.c
	* sysdeps/unix/sysv/linux/ftime.c: Delete file.
---
 sysdeps/unix/bsd/ftime.c        | 40 ---------------------------------
 sysdeps/unix/sysv/linux/ftime.c |  3 ---
 time/ftime.c                    | 26 ++++++++++-----------
 3 files changed, 12 insertions(+), 57 deletions(-)
 delete mode 100644 sysdeps/unix/bsd/ftime.c
 delete mode 100644 sysdeps/unix/sysv/linux/ftime.c

Comments

Adhemerval Zanella Aug. 21, 2019, 3:18 p.m. UTC | #1
On 20/08/2019 10:21, Zack Weinberg wrote:
> ftime is an obsolete variation on gettimeofday, offering only
> millisecond time resolution; it was probably a system call in ooold
> versions of BSD Unix.  For historic reasons, we had three
> implementations of it.  These are all consolidated into time/ftime.c.
> 
> Like gettimeofday, ftime tries to report the time zone, and using that
> information is always a bug.  This patch dummies out the reported
> timezone information; the ‘timezone’ and ‘dstflag’ fields of the
> returned ‘struct timeb’ will always be zero.
> 
> (There is an argument for turning this function into a compat symbol,
> and not installing sys/timeb.h anymore.  Thoughts?)

It was removed in POSIX.1-2008 and moving it to a compat symbol it one less
symbol we would need to adapt to y2038.  I think it is reasonable.

> 
> 	* time/ftime.c (ftime): Replace implementation with the code
> 	formerly in sysdeps/unix/bsd/ftime.c, then change that code to use
> 	__clock_gettime instead of __gettimeofday.  Always set the
> 	timezone and dstflag fields of the ‘timebuf’ argument to zero.
> 
> 	* sysdeps/unix/bsd/ftime.c
> 	* sysdeps/unix/sysv/linux/ftime.c: Delete file.

LGTM with a nit below regarding clock_gettime (CLOCK_REALTIME).

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

> ---
>  sysdeps/unix/bsd/ftime.c        | 40 ---------------------------------
>  sysdeps/unix/sysv/linux/ftime.c |  3 ---
>  time/ftime.c                    | 26 ++++++++++-----------
>  3 files changed, 12 insertions(+), 57 deletions(-)
>  delete mode 100644 sysdeps/unix/bsd/ftime.c
>  delete mode 100644 sysdeps/unix/sysv/linux/ftime.c
> 
> diff --git a/sysdeps/unix/bsd/ftime.c b/sysdeps/unix/bsd/ftime.c
> deleted file mode 100644
> index 3a1c6e9b01..0000000000
> --- a/sysdeps/unix/bsd/ftime.c
> +++ /dev/null
> @@ -1,40 +0,0 @@
> -/* Copyright (C) 1994-2019 Free Software Foundation, Inc.
> -   This file is part of the GNU C Library.
> -
> -   The GNU C Library is free software; you can redistribute it and/or
> -   modify it under the terms of the GNU Lesser General Public
> -   License as published by the Free Software Foundation; either
> -   version 2.1 of the License, or (at your option) any later version.
> -
> -   The GNU C Library is distributed in the hope that it will be useful,
> -   but WITHOUT ANY WARRANTY; without even the implied warranty of
> -   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> -   Lesser General Public License for more details.
> -
> -   You should have received a copy of the GNU Lesser General Public
> -   License along with the GNU C Library; if not, see
> -   <http://www.gnu.org/licenses/>.  */
> -
> -#include <sys/timeb.h>
> -#include <sys/time.h>
> -
> -int
> -ftime (struct timeb *timebuf)
> -{
> -  struct timeval tv;
> -  struct timezone tz;
> -
> -  if (__gettimeofday (&tv, &tz) < 0)
> -    return -1;
> -
> -  timebuf->time = tv.tv_sec;
> -  timebuf->millitm = (tv.tv_usec + 500) / 1000;
> -  if (timebuf->millitm == 1000)
> -    {
> -      ++timebuf->time;
> -      timebuf->millitm = 0;
> -    }
> -  timebuf->timezone = tz.tz_minuteswest;
> -  timebuf->dstflag = tz.tz_dsttime;
> -  return 0;
> -}

Ok.

> diff --git a/sysdeps/unix/sysv/linux/ftime.c b/sysdeps/unix/sysv/linux/ftime.c
> deleted file mode 100644
> index 5a5949f608..0000000000
> --- a/sysdeps/unix/sysv/linux/ftime.c
> +++ /dev/null
> @@ -1,3 +0,0 @@
> -/* Linux defines the ftime system call but doesn't actually implement
> -   it.  Use the BSD implementation.  */

Ok.

> -#include <sysdeps/unix/bsd/ftime.c>
> diff --git a/time/ftime.c b/time/ftime.c
> index 6c2a256048..de8d043893 100644
> --- a/time/ftime.c
> +++ b/time/ftime.c
> @@ -15,27 +15,25 @@
>     License along with the GNU C Library; if not, see
>     <http://www.gnu.org/licenses/>.  */
>  
> -#include <errno.h>
> -#include <time.h>
>  #include <sys/timeb.h>
> +#include <time.h>
>  
>  int
>  ftime (struct timeb *timebuf)
>  {
> -  int save = errno;
> -  struct tm tp;
> +  struct timespec ts;
>  
> -  __set_errno (0);
> -  if (time (&timebuf->time) == (time_t) -1 && errno != 0)
> +  if (__clock_gettime (CLOCK_REALTIME, &ts) < 0)
>      return -1;

I think we can assume CLOCK_REALTIME is always supported and &ts is always
valid, so we can skip the test here.

> -  timebuf->millitm = 0;
> -
> -  if (__localtime_r (&timebuf->time, &tp) == NULL)
> -    return -1;
> -
> -  timebuf->timezone = tp.tm_gmtoff / 60;
> -  timebuf->dstflag = tp.tm_isdst;
>  
> -  __set_errno (save);
> +  timebuf->time = ts.tv_sec;
> +  timebuf->millitm = (ts.tv_nsec + 500000) / 1000000;

I think it is a fair assumption that kernel won't play foul here and give
us invalid tv_nsec.

> +  if (timebuf->millitm == 1000)
> +    {
> +      ++timebuf->time;
> +      timebuf->millitm = 0;
> +    }
> +  timebuf->timezone = 0;
> +  timebuf->dstflag = 0;
>    return 0;
>  }
>
Zack Weinberg Aug. 22, 2019, 12:59 p.m. UTC | #2
On 8/21/19 11:18 AM, Adhemerval Zanella wrote:
>> -  __set_errno (0);
>> -  if (time (&timebuf->time) == (time_t) -1 && errno != 0)
>> +  if (__clock_gettime (CLOCK_REALTIME, &ts) < 0)
>>      return -1;
> 
> I think we can assume CLOCK_REALTIME is always supported and &ts is always
> valid, so we can skip the test here.

Will change.

>> +  timebuf->time = ts.tv_sec;
>> +  timebuf->millitm = (ts.tv_nsec + 500000) / 1000000;
> 
> I think it is a fair assumption that kernel won't play foul here and give
> us invalid tv_nsec.

Yes, we already assume this in many other places.

zw

Patch
diff mbox series

diff --git a/sysdeps/unix/bsd/ftime.c b/sysdeps/unix/bsd/ftime.c
deleted file mode 100644
index 3a1c6e9b01..0000000000
--- a/sysdeps/unix/bsd/ftime.c
+++ /dev/null
@@ -1,40 +0,0 @@ 
-/* Copyright (C) 1994-2019 Free Software Foundation, Inc.
-   This file is part of the GNU C Library.
-
-   The GNU C Library is free software; you can redistribute it and/or
-   modify it under the terms of the GNU Lesser General Public
-   License as published by the Free Software Foundation; either
-   version 2.1 of the License, or (at your option) any later version.
-
-   The GNU C Library is distributed in the hope that it will be useful,
-   but WITHOUT ANY WARRANTY; without even the implied warranty of
-   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
-   Lesser General Public License for more details.
-
-   You should have received a copy of the GNU Lesser General Public
-   License along with the GNU C Library; if not, see
-   <http://www.gnu.org/licenses/>.  */
-
-#include <sys/timeb.h>
-#include <sys/time.h>
-
-int
-ftime (struct timeb *timebuf)
-{
-  struct timeval tv;
-  struct timezone tz;
-
-  if (__gettimeofday (&tv, &tz) < 0)
-    return -1;
-
-  timebuf->time = tv.tv_sec;
-  timebuf->millitm = (tv.tv_usec + 500) / 1000;
-  if (timebuf->millitm == 1000)
-    {
-      ++timebuf->time;
-      timebuf->millitm = 0;
-    }
-  timebuf->timezone = tz.tz_minuteswest;
-  timebuf->dstflag = tz.tz_dsttime;
-  return 0;
-}
diff --git a/sysdeps/unix/sysv/linux/ftime.c b/sysdeps/unix/sysv/linux/ftime.c
deleted file mode 100644
index 5a5949f608..0000000000
--- a/sysdeps/unix/sysv/linux/ftime.c
+++ /dev/null
@@ -1,3 +0,0 @@ 
-/* Linux defines the ftime system call but doesn't actually implement
-   it.  Use the BSD implementation.  */
-#include <sysdeps/unix/bsd/ftime.c>
diff --git a/time/ftime.c b/time/ftime.c
index 6c2a256048..de8d043893 100644
--- a/time/ftime.c
+++ b/time/ftime.c
@@ -15,27 +15,25 @@ 
    License along with the GNU C Library; if not, see
    <http://www.gnu.org/licenses/>.  */
 
-#include <errno.h>
-#include <time.h>
 #include <sys/timeb.h>
+#include <time.h>
 
 int
 ftime (struct timeb *timebuf)
 {
-  int save = errno;
-  struct tm tp;
+  struct timespec ts;
 
-  __set_errno (0);
-  if (time (&timebuf->time) == (time_t) -1 && errno != 0)
+  if (__clock_gettime (CLOCK_REALTIME, &ts) < 0)
     return -1;
-  timebuf->millitm = 0;
-
-  if (__localtime_r (&timebuf->time, &tp) == NULL)
-    return -1;
-
-  timebuf->timezone = tp.tm_gmtoff / 60;
-  timebuf->dstflag = tp.tm_isdst;
 
-  __set_errno (save);
+  timebuf->time = ts.tv_sec;
+  timebuf->millitm = (ts.tv_nsec + 500000) / 1000000;
+  if (timebuf->millitm == 1000)
+    {
+      ++timebuf->time;
+      timebuf->millitm = 0;
+    }
+  timebuf->timezone = 0;
+  timebuf->dstflag = 0;
   return 0;
 }