Re: [PATCH] RTEMS: Synchronize <sys/time.h> with FreeBSD

[Brian Inglis <Brian.Inglis@SystematicSw.ab.ca>]

On 2019-09-23 23:55, Sebastian Huber wrote:
> On 23/09/2019 22:06, Brian Inglis wrote:
>> On 2019-09-23 04:07, Sebastian Huber wrote:
>>> This change is based on the FreeBSD commit:
>>> Author: asomers <asomers@FreeBSD.org>
>>> Date:   Mon Jul 30 15:46:40 2018 +0000
>>> Make timespecadd(3) and friends public
>>> The timespecadd(3) family of macros were imported from NetBSD back in 
>>> r35029. However, they were initially guarded by #ifdef _KERNEL. In the 
>>> meantime, we have grown at least 28 syscalls that use timespecs in some 
>>> way, leading many programs both inside and outside of the base system to 
>>> redefine those macros. It's better just to make the definitions public. 
>>> Our kernel currently defines two-argument versions of timespecadd and
>>> timespecsub.  NetBSD, OpenBSD, and FreeDesktop.org's libbsd, however, 
>>> define three-argument versions.  Solaris also defines a three-argument
>>> version, but only in its kernel.  This revision changes our definition to
>>> match the common three-argument version.
>>> Bump _FreeBSD_version due to the breaking KPI change.
>>> Discussed with: cem, jilles, ian, bde
>>> Differential Revision:  https://reviews.freebsd.org/D14725
>>> ---
>>>   newlib/libc/sys/rtems/include/machine/_time.h | 42 +++++++++++++++++++++++++--
>>>   1 file changed, 39 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/newlib/libc/sys/rtems/include/machine/_time.h
>>> b/newlib/libc/sys/rtems/include/machine/_time.h
>>> index 13fa14afa..24e6c7398 100644
>>> --- a/newlib/libc/sys/rtems/include/machine/_time.h
>>> +++ b/newlib/libc/sys/rtems/include/machine/_time.h
>>> @@ -1,6 +1,9 @@
>>>   /*-
>>> + * SPDX-License-Identifier: BSD-3-Clause
>>> + *
>>> + * Copyright (c) 1982, 1986, 1993
>>> + *    The Regents of the University of California.  All rights reserved.
>>>    * Copyright (c) 2016 embedded brains GmbH
>>> - * All rights reserved.
>>>    *
>>>    * Redistribution and use in source and binary forms, with or without
>>>    * modification, are permitted provided that the following conditions
>>> @@ -10,11 +13,14 @@
>>>    * 2. Redistributions in binary form must reproduce the above copyright
>>>    *    notice, this list of conditions and the following disclaimer in the
>>>    *    documentation and/or other materials provided with the distribution.
>>> + * 3. Neither the name of the University nor the names of its contributors
>>> + *    may be used to endorse or promote products derived from this software
>>> + *    without specific prior written permission.
>>>    *
>>> - * THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND
>>> + * THIS SOFTWARE IS PROVIDED BY THE REGENTS AND CONTRIBUTORS ``AS IS'' AND
>>>    * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
>>>    * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
>>> - * ARE DISCLAIMED.  IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE
>>> + * ARE DISCLAIMED.  IN NO EVENT SHALL THE REGENTS OR CONTRIBUTORS BE LIABLE
>>>    * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
>>>    * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
>>>    * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
>>> @@ -22,12 +28,42 @@
>>>    * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
>>>    * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
>>>    * SUCH DAMAGE.
>>> + *
>>> + *    @(#)time.h    8.5 (Berkeley) 5/4/95
>>> + * $FreeBSD: head/sys/sys/time.h 346176 2019-04-13 04:46:35Z imp $
>>>    */
>>>     #ifndef _SYS_TIME_H_
>>>   #error "must be included via <sys/time.h>"
>>>   #endif /* !_SYS_TIME_H_ */
>>>   +/* Operations on timespecs */
>>> +#define    timespecclear(tvp)    ((tvp)->tv_sec = (tvp)->tv_nsec = 0)
>>> +#define    timespecisset(tvp)    ((tvp)->tv_sec || (tvp)->tv_nsec)
>>> +#define    timespeccmp(tvp, uvp, cmp)                \
>>> +    (((tvp)->tv_sec == (uvp)->tv_sec) ?                \
>>> +        ((tvp)->tv_nsec cmp (uvp)->tv_nsec) :            \
>>> +        ((tvp)->tv_sec cmp (uvp)->tv_sec))
>>> +
>>> +#define    timespecadd(tsp, usp, vsp)                \
>>> +    do {                                \
>>> +        (vsp)->tv_sec = (tsp)->tv_sec + (usp)->tv_sec;        \
>>> +        (vsp)->tv_nsec = (tsp)->tv_nsec + (usp)->tv_nsec;    \
>>> +        if ((vsp)->tv_nsec >= 1000000000L) {            \
>>> +            (vsp)->tv_sec++;                \
>>> +            (vsp)->tv_nsec -= 1000000000L;            \
>>> +        }                            \
>>> +    } while (0)
>>> +#define    timespecsub(tsp, usp, vsp)                \
>>> +    do {                                \
>>> +        (vsp)->tv_sec = (tsp)->tv_sec - (usp)->tv_sec;        \
>>> +        (vsp)->tv_nsec = (tsp)->tv_nsec - (usp)->tv_nsec;    \
>>> +        if ((vsp)->tv_nsec < 0) {                \
>>> +            (vsp)->tv_sec--;                \
>>> +            (vsp)->tv_nsec += 1000000000L;            \
>>> +        }                            \
>>> +    } while (0)
>>> +
>>>   #ifdef _KERNEL
>>>   /* Header file provided outside of Newlib */
>>>   #include <machine/_kernel_time.h>
>>
>> It would be good to make this more generally available for Cygwin, RTEMS, and
>> other systems, in a similar place to timeval, in
>> newlib/libc/include/sys/_timespec.h.
> 
> No, the purpose of this file is to provide only the type definitions and nothing
> extra.
> 
>> The timeval timer... macros are already defined in
>> newlib/libc/include/sys/_timeval.h under the __BSD_VISIBLE feature and #if*n*def
>> _KERNEL, 
> 
> That these macros are present in <sys/_timeval.h> looks like a bug which I
> produced. In FreeBSD, the macros are defined in <sys/time.h>.
> 
>> and these timespec macros are also available under most Linux distros,
>> standard on BSD.
> 
> The timespec macros above are not included in openSUSE 15.1 and the latest glibc
> Git master. On which Linux system did you see them?

They appear in packages libbsd-dev on Debian, Ubuntu, ... and libbsd-devel on
Centos, Fedora, openSuSE, ... installed by default on Fedora?
Macros in /usr/include/bsd/sys/time.h and manpages under 3bsd in DEBs and RPMs.

>> The conversion macros TIMEVAL_TO_TIMESPEC and TIMESPEC_TO_TIMEVAL are already
>> available in newlib/libc/include/sys/timespec.h.
>>
>> All the bintime... and sbintime... facilities are already available in
>> newlib/libc/include/sys/time.h guarded by __BSD_VISIBLE.
>>
>> As the existing facilities require _BSD_SOURCE be defined to make these public,
>> should the new additions also be guarded, or the guards removed in all cases?
>>
>> As I reinvented those wheels just last week, not knowing about these facilities,
>> could we please also make all the relevant BSD timeval...(3), timespec...(3),
>> and bintime...(9) man pages available also, perhaps under newlib/libc/time/?
> 
> I can change the patch to add these macros to <sys/time.h> under the
> __BSD_VISIBLE guard if this is fine for everyone.

You choose what to be compatible with or careful about.

-- 
Take care. Thanks, Brian Inglis, Calgary, Alberta, Canada

This email may be disturbing to some readers as it contains
too much technical detail. Reader discretion is advised.