[07/12] Use clock_gettime to implement ftime.

  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
  

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

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