Move _DIAGASSERT to include/assert.h and timespce_get form NetBSD.

[imp]

Summary

NetBSD defines this in <assert,h>. Define it now, but don't bring in
all the NetBSD code to do it since that's more extensive. Remove two
stray definitions we had in place before to cope.

Bring in timespce_get form NetBSD. It is the same on FreeBSD.

[cem]

include/time.h

210

Is this comment useful? We don't document the sections other standard C functions are defined.

lib/libc/gen/timespec_get.3

41

Hm, I don't think putting the specific value of TIME_UTC in the SYNOPSIS is beneficial. I think I'd just delete this line and leave defining valid base to the DESCRIPTION.

52

"supported" is redundant here

54

Since standard C leaves epoch implementation defined, it would be good to be explicit about the epoch chosen by our implementation.

63

Useless comment

65

.Xr time 3 is probably at least as relevant

73

.Fx 12

lib/libc/gen/timespec_get.c

50

This should probably _DIAGASSERT() as well. There is no good reason for`clock_gettime(CLOCK_REALTIME)` to fail here.

[imp]

include/time.h

210

It's verbatim form the NetBSD commit. I find it at least marginally useful, and so left it in. I don't plan on deleting it.

lib/libc/gen/timespec_get.3

41

This manual is verbatim from NetBSD. I don't plan on changing it unless there's a compelling reason, although this appears to be a bit of an outlier for FreeBSD practices.

54

I disagree. The epoch is universally 1970. There's no point in restating that everywhere.

65

nobody uses that old interface, why xref it?

73

Good point. I'll add a sentence here.

lib/libc/gen/timespec_get.c

50

No. I disagree. To be compliant we have to deal with failure by passing it back. asserts in library code for error cases (not programming errors) are bad.

[cem]

include/time.h

210

I don't see how it's even marginally useful.

lib/libc/gen/timespec_get.3

41

Yeah, understood this all was pulled directly from NetBSD. That makes it a good starting point, but doesn't mean it's perfect as-is.

54

1970 epoch is hardly universal. https://en.wikipedia.org/wiki/Epoch_(reference_date)#Notable_epoch_dates_in_computing

Just the other week I was told our manual pages document portability caveats of the standard C interface, in addition to stronger guarantees about our particular implementation.

I would be ok with just "unix epoch" if you feel specifying the full datetime is too verbose.

65

Because this is semantically the same interface (clock time since epoch), just with higher precision. This manual page's description of timespec_get even comes directly from C11's definition of time() — not timespec_get.

Nothing in this document explains how clock_gettime is related (i.e., the CLOCK_REALTIME connection).

lib/libc/gen/timespec_get.c

50

I don't follow. clock_gettime() returning failure is a programming error.

If we can't assert anything, we may as well drop the _DIAGASSERT addition and return 0 for ts == NULL as well.

(And if that's the route you really want to go, I'd suggest setting EINVAL for ts == NULL || base != TIME_UTC, unless there is some specific proviso against setting errno when the C standard does not specify it. And if there is such a provision, we need to save errno around clock_gettime.)

[imp]

include/time.h

210

noted.

lib/libc/gen/timespec_get.3

41

I prefer minimal changes to upstream since it makes taking later changes harder. I'll chat with the netbsd folks to see if we can harmonize this more, though I'll commit this as is pending those talks.

52

I'll add that to the list of changes I'll try to get upstream into NetBSD...

54

We're inconsistent when it comes to this. I'll commit this and do a followup commit to make the different places we talk about this more uniform. But all things POSIX the epoch is 1970.

65

True, i'll work with the netbsd folks to add this. Our clock_gettime man page needs a lot of work in this area as well. It's terrible at the moment.

lib/libc/gen/timespec_get.c

50

Since clock_gettime() is allowed to return an error, I don't think we should assert, or _DIAGASSERT until we bring in the other stuff from NetBSD for that. It's simply not a programming error on our part, though passing in a bogus value could trigger it. But that's not what asserts are for.

I'll check with the standard about errno and work with the NetBSD folks to get their stuff fixed.

[kib]

IMO it is very bad practice for the library functions to terminate the main program. Besides, what is wrong with returning 0/errno = EFAULT when NULL is passed ?

[imp]

In D16649#353911, @kib wrote:

IMO it is very bad practice for the library functions to terminate the main program. Besides, what is wrong with returning 0/errno = EFAULT when NULL is passed ?

I think I'll forego the __DIAGASSERT stuff, and just delete this one line from the import and get this committed so we have it in for 12.
I'll then work on the man page cleanup at a later time.

[kib]

In D16649#354034, @imp wrote:

In D16649#353911, @kib wrote:

IMO it is very bad practice for the library functions to terminate the main program. Besides, what is wrong with returning 0/errno = EFAULT when NULL is passed ?

I think I'll forego the __DIAGASSERT stuff, and just delete this one line from the import and get this committed so we have it in for 12.
I'll then work on the man page cleanup at a later time.

Sounds good. The versioning stuff for libc looks ok.

Note that freebsd style is to use 'return (x);'.

[imp]

In D16649#354035, @kib wrote:

Sounds good. The versioning stuff for libc looks ok.

Note that freebsd style is to use 'return (x);'.

Yea, I specifically didn't change that since we may have issues merging from NetBSD in the future. This may turn out not to be an issue, and if that turns out to be the case, I'll make this smell more like FreeBSD.

[imp]

Make changes suggested by kib. abandon the _DIAGASSERT update and
instead comment things out and then do a separate commit to do minimal
cleanup.

[imp]

lib/libc/gen/timespec_get.3

41

I've changed my mind on this, and will commit the cleanup as a separate commit... I think the changes would also help NetBSD...

54

I stole some verbage from gettimeofday(2) and noted FreeBSD specific details.

65

I've added some xrefs.

[kib]

include/time.h

211

Should there declarations guarded by 2011 C version check ?

#if (defined(__STDC_VERSION__) && __STDC_VERSION__ >= 201112L)

lib/libc/gen/timespec_get.3

72

I think this should reference the first version of FreeBSD. It might mention that the implementation was taken from NetBSD. But it is so trivial.

[imp]

include/time.h

211

Looks like this came in while / after I was committing things...

I never know when / how to guard namespace things. Is that the right way to do it? It seems a little odd, but I'm happy to do a followup commit to fix things so long as I know it's right...

[kib]

include/time.h

211

Well, I believe that this is the correct check, assuming that the function requirement was indeed introduced in C11.

Look for sys/cdefs.h for other uses of STDC_VERSION.

[imp]

include/time.h

211

It also appears to be in c++17. Do we have a standard way to flag that too?

[kib]

include/time.h

211

#if defined(cplusplus) && cplusplus >= 201703