This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH v2 06/10] Use clock_gettime to implement ftime; withdraw ftime.



On 28/08/2019 12:32, 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,
> and then the function is made a compatibility symbol.  The public
> header file that declared it, its result structure, and nothing else,
> is removed.
> 
> For some reason, the implementation of ftime in terms of gettimeofday
> was rounding rather than truncating microseconds to milliseconds.  In
> all the other places where we use a higher-resolution time function to
> implement a lower-resolution one, we truncate.  ftime is changed to
> match, just for tidiness' sake.

Sound reasonable.

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

Ok.

> 
> 	* time/tst-ftime.c
> 	* time/sys/timeb.h
> 	* sysdeps/unix/bsd/ftime.c
> 	* sysdeps/unix/sysv/linux/ftime.c: Delete file.
> 	* time/tst-ftime_l.c: Rename to tst-strftime_l.c.
> 
> 	* time/ftime.c (ftime): Make a compatibility-only symbol.
> 	Replace implementation with the code formerly in
> 	sysdeps/unix/bsd/ftime.c, then change that code to use
> 	__clock_gettime instead of __gettimeofday and to truncate
> 	rather than rounding.  Always set the timezone and dstflag
> 	fields of the timebuf argument to zero.
> 
> 	* time/Makefile (headers): Remove sys/timeb.h.
> 	(tests): Remove tst-ftime, change tst-ftime_l to
> 	tst-strftime_l.
> 
> 	* conform/Makefile: XFAIL all tests related to sys/timeb.h.
> ---
>  conform/Makefile                         |  9 ++++
>  sysdeps/unix/bsd/ftime.c                 | 40 ----------------
>  sysdeps/unix/sysv/linux/ftime.c          |  3 --
>  time/Makefile                            |  8 ++--
>  time/ftime.c                             | 40 +++++++++-------
>  time/sys/timeb.h                         | 43 ------------------
>  time/tst-ftime.c                         | 58 ------------------------
>  time/{tst-ftime_l.c => tst-strftime_l.c} |  0
>  8 files changed, 36 insertions(+), 165 deletions(-)
>  delete mode 100644 sysdeps/unix/bsd/ftime.c
>  delete mode 100644 sysdeps/unix/sysv/linux/ftime.c
>  delete mode 100644 time/sys/timeb.h
>  delete mode 100644 time/tst-ftime.c
>  rename time/{tst-ftime_l.c => tst-strftime_l.c} (100%)
> 
> diff --git a/conform/Makefile b/conform/Makefile
> index 59d569c4c5..8671c695e8 100644
> --- a/conform/Makefile
> +++ b/conform/Makefile
> @@ -241,3 +241,12 @@ test-xfail-XPG42/ndbm.h/linknamespace = yes
>  test-xfail-UNIX98/ndbm.h/linknamespace = yes
>  test-xfail-XOPEN2K/ndbm.h/linknamespace = yes
>  test-xfail-XOPEN2K8/ndbm.h/linknamespace = yes
> +
> +# Header no longer provided by glibc (obsoleted in newer POSIX
> +# standards).
> +test-xfail-UNIX98/sys/timeb.h/conform = yes
> +test-xfail-UNIX98/sys/timeb.h/linknamespace = yes
> +test-xfail-XOPEN2K/sys/timeb.h/conform = yes
> +test-xfail-XOPEN2K/sys/timeb.h/linknamespace = yes
> +test-xfail-XPG42/sys/timeb.h/conform = yes
> +test-xfail-XPG42/sys/timeb.h/linknamespace = yes

I think we will need to keep this header, at least one release, and mark is
as deprecated (maybe with a pragma warning) because some projects include
without configure checks (libsanitizer for instance).

> 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.  */
> -#include <sysdeps/unix/bsd/ftime.c>
> diff --git a/time/Makefile b/time/Makefile
> index 791db1c8e3..93c7cc28d4 100644
> --- a/time/Makefile
> +++ b/time/Makefile
> @@ -22,7 +22,7 @@ subdir	:= time
>  
>  include ../Makeconfig
>  
> -headers := time.h sys/time.h sys/timeb.h bits/time.h			\
> +headers := time.h sys/time.h bits/time.h				\
>  	   bits/types/clockid_t.h bits/types/clock_t.h			\
>  	   bits/types/struct_itimerspec.h				\
>  	   bits/types/struct_timespec.h bits/types/struct_timeval.h	\

As before.

> @@ -43,9 +43,9 @@ routines := offtime asctime clock ctime ctime_r difftime \
>  aux :=	    era alt_digit lc-time-cleanup
>  
>  tests	:= test_time clocktest tst-posixtz tst-strptime tst_wcsftime \
> -	   tst-getdate tst-mktime tst-mktime2 tst-ftime_l tst-strftime \
> +	   tst-getdate tst-mktime tst-mktime2 tst-strftime tst-strftime_l \
>  	   tst-mktime3 tst-strptime2 bug-asctime bug-asctime_r bug-mktime1 \
> -	   tst-strptime3 bug-getdate1 tst-strptime-whitespace tst-ftime \
> +	   tst-strptime3 bug-getdate1 tst-strptime-whitespace \
>  	   tst-tzname tst-y2039 bug-mktime4 tst-strftime2 tst-strftime3 \
>  	   tst-clock tst-clock2 tst-clock_nanosleep tst-cpuclock1
>  
> @@ -59,7 +59,7 @@ LOCALES := de_DE.ISO-8859-1 en_US.ISO-8859-1 ja_JP.EUC-JP fr_FR.UTF-8 \
>  	   nan_TW.UTF-8 lzh_TW.UTF-8
>  include ../gen-locales.mk
>  
> -$(objpfx)tst-ftime_l.out: $(gen-locales)
> +$(objpfx)tst-strftime_l.out: $(gen-locales)
>  $(objpfx)tst-strptime.out: $(gen-locales)
>  endif
>  
> diff --git a/time/ftime.c b/time/ftime.c
> index 6c2a256048..0db6b70adf 100644
> --- a/time/ftime.c
> +++ b/time/ftime.c
> @@ -15,27 +15,33 @@
>     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 <shlib-compat.h>
>  
> -int
> -ftime (struct timeb *timebuf)
> -{
> -  int save = errno;
> -  struct tm tp;
> +#if SHLIB_COMPAT (libc, GLIBC_2_0, GLIBC_2_31)
>  
> -  __set_errno (0);
> -  if (time (&timebuf->time) == (time_t) -1 && errno != 0)
> -    return -1;
> -  timebuf->millitm = 0;
> +#include <time.h>
>  
> -  if (__localtime_r (&timebuf->time, &tp) == NULL)
> -    return -1;
> +struct timeb
> +{
> +  time_t time;			/* Seconds since epoch, as from `time'.  */
> +  unsigned short int millitm;	/* Additional milliseconds.  */
> +  short int timezone;		/* Minutes west of GMT.  */
> +  short int dstflag;		/* Nonzero if Daylight Savings Time used.  */
> +};
>  
> -  timebuf->timezone = tp.tm_gmtoff / 60;
> -  timebuf->dstflag = tp.tm_isdst;
> +int
> +attribute_compat_text_section
> +__ftime (struct timeb *timebuf)
> +{
> +  struct timespec ts;
> +  __clock_gettime (CLOCK_REALTIME, &ts);
>  
> -  __set_errno (save);
> +  timebuf->time = ts.tv_sec;
> +  timebuf->millitm = ts.tv_nsec / 1000000;
> +  timebuf->timezone = 0;
> +  timebuf->dstflag = 0;
>    return 0;
>  }
> +compat_symbol (libc, __ftime, ftime, GLIBC_2_0);
> +
> +#endif

Ok.

> diff --git a/time/sys/timeb.h b/time/sys/timeb.h
> deleted file mode 100644
> index 6333e8074d..0000000000
> --- a/time/sys/timeb.h
> +++ /dev/null
> @@ -1,43 +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/>.  */
> -
> -#ifndef _SYS_TIMEB_H
> -#define _SYS_TIMEB_H	1
> -
> -#include <features.h>
> -
> -#include <bits/types/time_t.h>
> -
> -__BEGIN_DECLS
> -
> -/* Structure returned by the `ftime' function.  */
> -
> -struct timeb
> -  {
> -    time_t time;		/* Seconds since epoch, as from `time'.  */
> -    unsigned short int millitm;	/* Additional milliseconds.  */
> -    short int timezone;		/* Minutes west of GMT.  */
> -    short int dstflag;		/* Nonzero if Daylight Savings Time used.  */
> -  };
> -
> -/* Fill in TIMEBUF with information about the current time.  */
> -
> -extern int ftime (struct timeb *__timebuf);
> -
> -__END_DECLS
> -
> -#endif	/* sys/timeb.h */
> diff --git a/time/tst-ftime.c b/time/tst-ftime.c
> deleted file mode 100644
> index 65b753dec8..0000000000
> --- a/time/tst-ftime.c
> +++ /dev/null
> @@ -1,58 +0,0 @@
> -/* Verify that ftime is sane.
> -   Copyright (C) 2014-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 <stdio.h>
> -
> -static int
> -do_test (void)
> -{
> -  struct timeb prev, curr = {.time = 0, .millitm = 0};
> -  int sec = 0;
> -
> -  while (sec != 3)
> -    {
> -      prev = curr;
> -
> -      if (ftime (&curr))
> -        {
> -          printf ("ftime returned an error\n");
> -          return 1;
> -        }
> -
> -      if (curr.time < prev.time)
> -        {
> -          printf ("ftime's time flowed backwards\n");
> -          return 1;
> -        }
> -
> -      if (curr.time == prev.time
> -          && curr.millitm < prev.millitm)
> -        {
> -          printf ("ftime's millitm flowed backwards\n");
> -          return 1;
> -        }
> -
> -      if (curr.time > prev.time)
> -        sec ++;
> -    }
> -  return 0;
> -}
> -
> -#define TEST_FUNCTION do_test ()
> -#include "../test-skeleton.c"

We can make it a internal test using TEST_COMPAT instead.

> diff --git a/time/tst-ftime_l.c b/time/tst-strftime_l.c
> similarity index 100%
> rename from time/tst-ftime_l.c
> rename to time/tst-strftime_l.c
> 


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]