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 04/10] Use clock_settime to implement settimeofday.



On 28/08/2019 12:32, Zack Weinberg wrote:
> Unconditionally, on all ports, use clock_settime to implement
> settimeofday.  Remove sysdeps/unix/clock_settime.c, which implemented
> clock_settime by calling settimeofday; new OS ports must henceforth
> provide a real implementation of clock_settime.

I think it is a fair requirement.

> 
> Hurd had a real implementation of settimeofday but not of
> clock_settime; this patch converts it into an implementation of
> clock_settime.  It only supports CLOCK_REALTIME and microsecond
> resolution; Hurd/Mach does not appear to have any support for
> finer-resolution clocks.
> 
> The vestigial "set time zone" feature of settimeofday complicates the
> generic settimeofday implementation a little.  The only remaining uses
> of this feature that aren't just bugs, are using it to inform the
> Linux kernel of the offset between the hardware clock and UTC, on
> systems where the hardware clock doesn't run in UTC (usually because
> of dual-booting with Windows).  There currently isn't any other way to
> do this.  However, the callers that do this call settimeofday with
> _only_ the timezone argument non-NULL.  Therefore, glibc's new
> behavior is: callers of settimeofday must supply one and only one of
> the two arguments.  If both arguments are non-NULL, or both arguments
> are NULL, the call fails and sets errno to EINVAL.

We definitely need a NEWS entry for this change. 

> 
> When only the timeval argument is supplied, settimeofday calls
> __clock_settime(CLOCK_REALTIME), same as stime.
> 
> When only the timezone argument is supplied, settimeofday calls a new
> internal function called __settimezone.  On Linux, only, this function
> will pass the timezone structure to the settimeofday system call.  On
> all other operating systems, and on Linux architectures that don't
> define __NR_settimeofday, __settimezone is a stub that always sets
> errno to ENOSYS and returns -1.

It is worth to mention that settimeoday syscall is enabled on Linux by
the flag COMPAT_32BIT_TIME, which is an option to either 32-bits ABIs
or COMPAT builds (defined usually by 64-bit kernels that want to support
32-bit ABIs, such as x86).

So the idea is to future 64-bit time_t only ABIs to not provide
settimeofday syscall.

> 
> The same semantics are implemented for Linux/Alpha's GLIBC_2.0 compat
> symbol for settimeofday.
> 
> There are no longer any internal callers of __settimeofday, so the
> internal prototype is removed.
> 
> 	* time/settimeofday.c (settimeofday): No longer a stub
> 	implementation.  Call __clock_settime or __settimezone depending
> 	on arguments.  Optionally override the default symbol version for
> 	settimeofday.
> 	* include/sys/time.h: Remove prototype for __settimeofday.
> 	Add prototype for __settimezone.
> 	* sysdeps/unix/syscalls.list: Remove entry for settimeofday.
> 
> 	* time/settimezone.c: New file.
> 	(__settimezone): New stub implementation.
> 	* sysdeps/unix/sysv/linux/settimezone.c: New file.
> 	(__settimezone): Implement using settimeofday system call,
> 	if available.
> 	* time/Makefile (routines): Add settimezone.
> 
> 	* sysdeps/unix/sysv/linux/alpha/syscalls.list:
> 	Remove entries for settimeofday and osf_settimeofday.
> 	* sysdeps/unix/sysv/linux/alpha/osf_settimeofday.c
> 	New file, defines settimeofday@GLIBC_2.0.
> 	* sysdeps/unix/sysv/linux/alpha/settimeofday.c:
> 	New file, defines settimeofday@@GLIBC_2.1.
> 
> 	* sysdeps/unix/clock_gettime.c: Delete file.
> 	* sysdeps/mach/hurd/settimeofday.c: Rename to
> 	sysdeps/mach/hurd/clock_settime.c and convert into an
> 	implementation of clock_settime.
> ---
>  include/sys/time.h                            |  3 +-
>  sysdeps/{unix => mach/hurd}/clock_settime.c   | 51 ++++++++-----------
>  sysdeps/unix/syscalls.list                    |  1 -
>  .../unix/sysv/linux/alpha/osf_settimeofday.c  | 16 ++++--
>  sysdeps/unix/sysv/linux/alpha/settimeofday.c  | 22 ++++++++
>  sysdeps/unix/sysv/linux/alpha/syscalls.list   |  1 -
>  sysdeps/unix/sysv/linux/settimezone.c         | 39 ++++++++++++++
>  time/Makefile                                 |  8 +--
>  time/settimeofday.c                           | 24 +++++++--
>  .../hurd/settimeofday.c => time/settimezone.c | 34 ++-----------
>  10 files changed, 126 insertions(+), 73 deletions(-)
>  rename sysdeps/{unix => mach/hurd}/clock_settime.c (65%)
>  create mode 100644 sysdeps/unix/sysv/linux/alpha/settimeofday.c
>  create mode 100644 sysdeps/unix/sysv/linux/settimezone.c
>  rename sysdeps/mach/hurd/settimeofday.c => time/settimezone.c (52%)
> 
> diff --git a/include/sys/time.h b/include/sys/time.h
> index 7ba0ca7c2d..a57752e8c7 100644
> --- a/include/sys/time.h
> +++ b/include/sys/time.h
> @@ -24,8 +24,7 @@ extern int __gettimeofday (struct timeval *__tv,
>  			   struct timezone *__tz);
>  libc_hidden_proto (__gettimeofday)
>  libc_hidden_proto (gettimeofday)
> -extern int __settimeofday (const struct timeval *__tv,
> -			   const struct timezone *__tz)
> +extern int __settimezone (const struct timezone *__tz)
>  	attribute_hidden;
>  extern int __adjtime (const struct timeval *__delta,
>  		      struct timeval *__olddelta);

Ok.

> diff --git a/sysdeps/unix/clock_settime.c b/sysdeps/mach/hurd/clock_settime.c
> similarity index 65%
> rename from sysdeps/unix/clock_settime.c
> rename to sysdeps/mach/hurd/clock_settime.c
> index 5b398491ab..0a32f8d1a0 100644
> --- a/sysdeps/unix/clock_settime.c
> +++ b/sysdeps/mach/hurd/clock_settime.c
> @@ -1,4 +1,4 @@
> -/* Copyright (C) 1999-2019 Free Software Foundation, Inc.
> +/* Copyright (C) 1991-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
> @@ -17,38 +17,31 @@
>  
>  #include <errno.h>
>  #include <time.h>
> -#include <sys/time.h>
> +#include <hurd.h>
> +#include <hurd/port.h>
>  #include <shlib-compat.h>
>  
> -/* Set CLOCK to value TP.  */
> +/* Set the current time of day.
> +   This call is restricted to the super-user.  */
>  int
> -__clock_settime (clockid_t clock_id, const struct timespec *tp)
> +__clock_settime (clockid_t clock_id, const struct timespec *ts)
>  {
> -  int retval = -1;
> -
> -  /* Make sure the time cvalue is OK.  */
> -  if (tp->tv_nsec < 0 || tp->tv_nsec >= 1000000000)
> -    {
> -      __set_errno (EINVAL);
> -      return -1;
> -    }
> -
> -  switch (clock_id)
> -    {
> -    case CLOCK_REALTIME:
> -      {
> -	struct timeval tv;
> -	TIMESPEC_TO_TIMEVAL (&tv, tp);
> -	retval = __settimeofday (&tv, NULL);
> -      }
> -      break;
> -
> -    default:
> -      __set_errno (EINVAL);
> -      break;
> -    }
> -
> -  return retval;
> +  error_t err;
> +  mach_port_t hostpriv;
> +  time_value_t tv;
> +
> +  if (clock_id != CLOCK_REALTIME)
> +    return __hurd_fail (EINVAL);
> +
> +  err = __get_privileged_ports (&hostpriv, NULL);
> +  if (err)
> +    return __hurd_fail (EPERM);
> +
> +  TIMESPEC_TO_TIME_VALUE (&tv, ts);
> +  err = __host_set_time (hostpriv, tv);
> +  __mach_port_deallocate (__mach_task_self (), hostpriv);
> +
> +  return __hurd_fail (err);
>  }
>  libc_hidden_def (__clock_settime)
>  

Looks ok, although I can't voucher for Hurd modifications.

> diff --git a/sysdeps/unix/syscalls.list b/sysdeps/unix/syscalls.list
> index 61e5360b4d..5fedd5733d 100644
> --- a/sysdeps/unix/syscalls.list
> +++ b/sysdeps/unix/syscalls.list
> @@ -76,7 +76,6 @@ setreuid	-	setreuid	i:ii	__setreuid	setreuid
>  setrlimit	-	setrlimit	i:ip	__setrlimit setrlimit
>  setsid		-	setsid		i:	__setsid	setsid
>  setsockopt	-	setsockopt	i:iiibn	setsockopt	__setsockopt
> -settimeofday	-	settimeofday	i:PP	__settimeofday	settimeofday
>  setuid		-	setuid		i:i	__setuid	setuid
>  shutdown	-	shutdown	i:ii	shutdown
>  sigaction	-	sigaction	i:ipp	__sigaction	sigaction

Ok.

> diff --git a/sysdeps/unix/sysv/linux/alpha/osf_settimeofday.c b/sysdeps/unix/sysv/linux/alpha/osf_settimeofday.c
> index 475e0fd656..9221deb80f 100644
> --- a/sysdeps/unix/sysv/linux/alpha/osf_settimeofday.c
> +++ b/sysdeps/unix/sysv/linux/alpha/osf_settimeofday.c
> @@ -32,9 +32,19 @@ attribute_compat_text_section
>  __settimeofday_tv32 (const struct timeval32 *tv32,
>                       const struct timezone *tz)
>  {
> -  struct timeval tv;
> -  TV32_TO_TV64 (&tv, tv32);
> -  return __settimeofday (&tv, tz);
> +  if (__glibc_unlikely (tz != 0))
> +    {
> +      if (tv32 != 0)
> +	{
> +	  __set_errno (EINVAL);
> +	  return -1;
> +	}
> +      return __settimezone (tz);
> +    }
> +
> +  struct timespec ts;
> +  TV32_TO_TS64 (&ts, tv32);
> +  return __clock_settime (CLOCK_REALTIME, &ts);
>  }
>  
>  compat_symbol (libc, __settimeofday_tv32, settimeofday, GLIBC_2_0);

Ok.

> diff --git a/sysdeps/unix/sysv/linux/alpha/settimeofday.c b/sysdeps/unix/sysv/linux/alpha/settimeofday.c
> new file mode 100644
> index 0000000000..36a6901e4e
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/alpha/settimeofday.c
> @@ -0,0 +1,22 @@
> +/* settimeofday -- Set the current time of day.  Linux/Alpha/tv64 version.
> +   Copyright (C) 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/>.  */
> +
> +/* We can use the generic implementation, but we have to override its
> +   default symbol version.  */
> +#define VERSION_settimeofday GLIBC_2.1
> +#include <time/settimeofday.c>

Ok.

> diff --git a/sysdeps/unix/sysv/linux/alpha/syscalls.list b/sysdeps/unix/sysv/linux/alpha/syscalls.list
> index c786aa751e..95a27e18e8 100644
> --- a/sysdeps/unix/sysv/linux/alpha/syscalls.list
> +++ b/sysdeps/unix/sysv/linux/alpha/syscalls.list
> @@ -24,7 +24,6 @@ pciconfig_iobase EXTRA	pciconfig_iobase 3	__pciconfig_iobase pciconfig_iobase
>  
>  # timeval64 entry points (see osf_*.c for GLIBC_2.0 timeval32 equivalents)
>  gettimeofday	-	gettimeofday	i:pP	__GI___gettimeofday gettimeofday@@GLIBC_2.1 __gettimeofday@@GLIBC_2.1
> -settimeofday	-	settimeofday	i:PP	__settimeofday	settimeofday@@GLIBC_2.1
>  getitimer	-	getitimer	i:ip	__getitimer	getitimer@@GLIBC_2.1
>  setitimer	-	setitimer	i:ipP	__setitimer	setitimer@@GLIBC_2.1
>  utimes		-	utimes		i:sp	__utimes	utimes@@GLIBC_2.1

Ok.

> diff --git a/sysdeps/unix/sysv/linux/settimezone.c b/sysdeps/unix/sysv/linux/settimezone.c
> new file mode 100644
> index 0000000000..823f4fe42f
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/settimezone.c
> @@ -0,0 +1,39 @@

One line comment.

> +/* Copyright (C) 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 <errno.h>
> +#include <sys/time.h>
> +#include <sysdep.h>
> +
> +/* Set the system-wide timezone.
> +   This call is restricted to the super-user.
> +   This operation is considered obsolete, kernel support may not be
> +   available on all architectures.  */
> +
> +#ifdef __NR_settimeofday
> +
> +int
> +__settimezone (const struct timezone *tz)
> +{
> +  return INLINE_SYSCALL_CALL (settimeofday, NULL, tz);
> +}
> +
> +#else
> +
> +#include <time/settimezone.c>
> +
> +#endif

I think it would be simpler to just add the expected behaviour instead of
include the default implementation to make it more clearly:

int
__settimezone (const struct timezone *tz)
{
#ifdef __NR_settimeofday
  return INLINE_SYSCALL_CALL (settimeofday, NULL, tz);
#else
  __set_errno (ENOSYS);
  return -1
#endif
}

> diff --git a/time/Makefile b/time/Makefile
> index 85a62c0376..791db1c8e3 100644
> --- a/time/Makefile
> +++ b/time/Makefile
> @@ -31,13 +31,13 @@ headers := time.h sys/time.h sys/timeb.h bits/time.h			\
>  
>  routines := offtime asctime clock ctime ctime_r difftime \
>  	    gmtime localtime mktime time		 \
> -	    gettimeofday settimeofday adjtime tzset	 \
> -	    tzfile getitimer setitimer			 \
> +	    gettimeofday settimeofday settimezone	 \
> +	    adjtime tzset tzfile getitimer setitimer	 \
>  	    stime dysize timegm ftime			 \
>  	    getdate strptime strptime_l			 \
>  	    strftime wcsftime strftime_l wcsftime_l	 \
> -	    timespec_get 	 			 \
> -	    clock_getcpuclockid clock_getres 		 \
> +	    timespec_get				 \
> +	    clock_getcpuclockid clock_getres		 \
>  	    clock_gettime clock_settime clock_nanosleep
>  
>  aux :=	    era alt_digit lc-time-cleanup

Ok.

> diff --git a/time/settimeofday.c b/time/settimeofday.c
> index 4620559652..78f666e352 100644
> --- a/time/settimeofday.c
> +++ b/time/settimeofday.c
> @@ -16,6 +16,7 @@
>     <http://www.gnu.org/licenses/>.  */
>  
>  #include <errno.h>
> +#include <time.h>
>  #include <sys/time.h>
>  
>  /* Set the current time of day and timezone information.
> @@ -23,9 +24,24 @@
>  int
>  __settimeofday (const struct timeval *tv, const struct timezone *tz)
>  {
> -  __set_errno (ENOSYS);
> -  return -1;
> +  if (__glibc_unlikely (tz != 0))
> +    {
> +      if (tv != 0)
> +	{
> +	  __set_errno (EINVAL);
> +	  return -1;
> +	}
> +      return __settimezone (tz);
> +    }
> +
> +  struct timespec ts;
> +  TIMEVAL_TO_TIMESPEC (tv, &ts);
> +  return __clock_settime (CLOCK_REALTIME, &ts);
>  }
> -stub_warning (settimeofday)
>  
> -weak_alias (__settimeofday, settimeofday)
> +#ifdef VERSION_settimeofday
> +weak_alias (__settimeofday, __settimeofday_w);
> +default_symbol_version (__settimeofday_w, settimeofday, VERSION_settimeofday);
> +#else
> +weak_alias (__settimeofday, settimeofday);
> +#endif

Ok.

> diff --git a/sysdeps/mach/hurd/settimeofday.c b/time/settimezone.c
> similarity index 52%
> rename from sysdeps/mach/hurd/settimeofday.c
> rename to time/settimezone.c
> index bd0ffd64ac..eb005da213 100644
> --- a/sysdeps/mach/hurd/settimeofday.c
> +++ b/time/settimezone.c
> @@ -1,4 +1,4 @@
> -/* Copyright (C) 1991-2019 Free Software Foundation, Inc.
> +/* Copyright (C) 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
> @@ -17,36 +17,12 @@
>  
>  #include <errno.h>
>  #include <sys/time.h>
> -#include <hurd.h>
> -#include <hurd/port.h>
>  
> -/* Set the current time of day and timezone information.
> +/* Set the system-wide timezone.
>     This call is restricted to the super-user.  */
>  int
> -__settimeofday (const struct timeval *tv, const struct timezone *tz)
> +__settimezone (const struct timezone *tz)
>  {
> -  error_t err;
> -  mach_port_t hostpriv;
> -
> -  if (tz != NULL)
> -    {
> -      errno = ENOSYS;
> -      return -1;
> -    }
> -
> -  err = __get_privileged_ports (&hostpriv, NULL);
> -  if (err)
> -    return __hurd_fail (EPERM);
> -
> -  /* `time_value_t' and `struct timeval' are in fact identical with the
> -     names changed.  */
> -  err = __host_set_time (hostpriv, *(time_value_t *) tv);
> -  __mach_port_deallocate (__mach_task_self (), hostpriv);
> -
> -  if (err)
> -    return __hurd_fail (err);
> -
> -  return 0;
> +  __set_errno (ENOSYS);
> +  return -1;
>  }
> -
> -weak_alias (__settimeofday, settimeofday)
> 

Ok.


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