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: [RFC v4 24/24] timerfd_settime: Use 64-bit call if avaliable


On Fri, 9 Aug 2019, Alistair Francis wrote:

> diff --git a/sysdeps/unix/sysv/linux/sys/timerfd.h b/sysdeps/unix/sysv/linux/sys/timerfd.h
> index 5e5ad351a0d..51f63b357c1 100644
> --- a/sysdeps/unix/sysv/linux/sys/timerfd.h
> +++ b/sysdeps/unix/sysv/linux/sys/timerfd.h
> @@ -24,6 +24,13 @@
>  /* Get the platform-dependent flags.  */
>  #include <bits/timerfd.h>
>  
> +#if __TIMESIZE == 32
> +struct __itimerspec64
> +  {
> +    struct __timespec64 it_interval;
> +    struct __timespec64 it_value;
> +  };
> +#endif

This does not belong in an installed header (until we have actual 
_TIME_BITS support added to all installed headers).

>  enum
> @@ -40,16 +47,16 @@ __BEGIN_DECLS
>  /* Return file descriptor for new interval timer source.  */
>  extern int timerfd_create (__clockid_t __clock_id, int __flags) __THROW;
>  
> -/* Set next expiration time of interval timer source UFD to UTMR.  If
> -   FLAGS has the TFD_TIMER_ABSTIME flag set the timeout value is
> -   absolute.  Optionally return the old expiration time in OTMR.  */
> -extern int timerfd_settime (int __ufd, int __flags,
> -			    const struct itimerspec *__utmr,
> -			    struct itimerspec *__otmr) __THROW;
> -
>  /* Return the next expiration time of UFD.  */
>  extern int timerfd_gettime (int __ufd, struct itimerspec *__otmr) __THROW;
>  
>  __END_DECLS
>  
> +/* Set next expiration time of interval timer source UFD to UTMR.  If
> +   FLAGS has the TFD_TIMER_ABSTIME flag set the timeout value is
> +   absolute.  Optionally return the old expiration time in OTMR.  */
> +int timerfd_settime (int __ufd, int __flags,
> +        const struct itimerspec *__utmr,
> +        struct itimerspec *__otmr) __THROW;

This move is wrong.  All function declarations in installed headers must 
be between __BEGIN_DECLS and __END_DECLS so they have C linkage in C++.

This patch has no commit message explaining what it's supposed to be doing 
and why.  Please include a proper explanation with *every* patch of what 
it's doing, how it's been tested, etc.

> diff --git a/sysdeps/unix/sysv/linux/timerfd_settime.c b/sysdeps/unix/sysv/linux/timerfd_settime.c
> new file mode 100644
> index 00000000000..830faeada77
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/timerfd_settime.c
> @@ -0,0 +1,104 @@
> +/* Copyright (C) 2003-2019 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +   Contributed by Ulrich Drepper <drepper@redhat.com>, 2003.

We don't use "Contributed by" in new source files.

What file with copyrightable content dating from 2003 is this based on?  
You should state that in your ChangeLog entry, if you're copying 
significant content from another file (if not, of course the copyright 
years should just be 2019).

-- 
Joseph S. Myers
joseph@codesourcery.com


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