[07/12] Use clock_gettime to implement ftime.

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

-- 
2.23.0.rc1

Comments

Adhemerval Zanella Aug. 21, 2019, 3:18 p.m. | #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. | #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 --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;
 }