Page MenuHomeFreeBSD

Put guards around timespec_get() decleration.
AbandonedPublic

Authored by brooks on Sep 14 2018, 7:04 PM.
Tags
None
Referenced Files
Unknown Object (File)
Tue, Nov 19, 3:19 AM
Unknown Object (File)
Fri, Nov 1, 2:32 AM
Unknown Object (File)
Fri, Nov 1, 2:32 AM
Unknown Object (File)
Fri, Nov 1, 2:31 AM
Unknown Object (File)
Fri, Nov 1, 2:31 AM
Unknown Object (File)
Fri, Nov 1, 2:31 AM
Unknown Object (File)
Fri, Nov 1, 2:08 AM
Unknown Object (File)
Oct 23 2024, 11:39 AM
Subscribers

Details

Summary

With a sufficiently low C standard version, struct timespec isn't
defined in time.h causing warnings. By adding guards for the version
in which timespec_get() was introduced we ensure that struct timespec
is defined.

These guards are based on the discussion in the origonal review:
https://reviews.freebsd.org/D16649

Test Plan

it builds and timespec_get() is new in 12

Diff Detail

Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 19633
Build 19204: arc lint + arc unit

Event Timeline

In CheriBSD, we have a copy of libpng in base that compiled with a low enough C standard that time.h breaks without this. I'm quite surprised the original change doesn't break some ports.

I do not see when struct timespec could be not defined. Can you show an example of the program and compiler invocation to reproduce ?

Timespec defined by the include of sys/timespec.h IFF:

#if __POSIX_VISIBLE >= 199309

I don't fully understand how this can happen (and it wasn't obvious from the failing command line), but it did happen in our tree so I added the previously discussed guards.

Timespec defined by the include of sys/timespec.h IFF:

#if __POSIX_VISIBLE >= 199309

Where is this guard ? From my look at sys/time.h, sys/timespec.h is always included, and then sys/_timespec.h is also.

I don't fully understand how this can happen (and it wasn't obvious from the failing command line), but it did happen in our tree so I added the previously discussed guards.

This block of include/time.h at line 76:

#if __POSIX_VISIBLE >= 199309
/*
 * New in POSIX 1003.1b-1993.
 */
#ifndef _CLOCKID_T_DECLARED
typedef __clockid_t     clockid_t;
#define _CLOCKID_T_DECLARED
#endif

#ifndef _TIMER_T_DECLARED
typedef __timer_t       timer_t;
#define _TIMER_T_DECLARED
#endif

#include <sys/timespec.h>
#endif /* __POSIX_VISIBLE >= 199309 */

This block of include/time.h at line 76:

Now I understand, I looked at sys/time.h, sorry.

I think that STDC_VERSION should be replaced by ISO_C_VISIBLE check. And, instead of WANT_TIMESPEC_GET, wouldn't __BSD_VISIBLE check work ?

And in fact our include/time.h is not C11 compatible, since C11 requires struct timespec to be defined.

  • Use ISO_C_VISIBLE and BSD_VISIBLE per @kib's suggestion.
include/time.h
210

I do not see a need to check for the __ISO_C_VISIBLE defined. It should be there always, because sys/cdefs.h is included. This is how other headers use it as well.

  • Check __ISO_C_VISIBLE unconditionally and rewrap.
This revision is now accepted and ready to land.Sep 17 2018, 10:44 PM
This revision was automatically updated to reflect the committed changes.
mjg added a subscriber: mjg.

Looks like these guards should be __cplusplus instead?

Spotted by Sascha Wildner <swildner@dragonflybsd.org>

FWIW,
It looks like we should update
contrib/libc++/include/__config around line 348.

head/include/time.h
211 ↗(On Diff #48168)

Somebody pointed out that this should be spelled as __cplusplus.