Page MenuHomeFreeBSD

time: Add EXTERROR for all EINVAL returns
AcceptedPublic

Authored by imp on Apr 18 2026, 2:50 AM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Jun 5, 10:34 PM
Unknown Object (File)
Thu, Jun 4, 5:13 PM
Unknown Object (File)
Fri, May 29, 1:51 PM
Unknown Object (File)
Mon, May 18, 1:36 PM
Unknown Object (File)
Mon, May 18, 12:26 PM
Unknown Object (File)
Mon, May 18, 12:07 PM
Unknown Object (File)
Mon, May 18, 12:03 PM
Unknown Object (File)
Sun, May 17, 6:23 PM
Subscribers

Details

Summary

Sponsored by: Netflix

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped
Build Status
Buildable 72338
Build 69221: arc lint + arc unit

Event Timeline

imp requested review of this revision.Apr 18 2026, 2:50 AM
sys/kern/kern_time.c
386
430
1291

This is spelled "clock_id invalid" elsewhere.

sys/kern/kern_time.c
530

In fact all cases could benefit from providing the wrong value that triggered the EINVAL, but this is optional.

sys/kern/kern_time.c
801

The format needs to be %ju. I.e. all format specifiers must be for (u)intmax_t. On the other hand, I do not think that the casts of the arguments to intmax are needed.

sys/kern/syscalls.master
55

This and related bits are unrelated, right?

sys/kern/kern_time.c
801

OK. I'm super confused about how that could possibly work. Why aren't the casts needed, and why does this have to be %ju? I missed something, somewhere I think.

sys/kern/syscalls.master
55

oh my! Yes, that's a leakage of something else I'm working on.

sys/kern/kern_time.c
801

The format string and the arguments are copied intact from kernel to usermode. Usermode, libc specifically, does the formatting. Arguments are marshaled through uint64_t objects, and are cast to uintmax_t for the formatting.

This is done so no memory allocation happens in kernel on error, so that EXTERROR() works in almost any context that has a valid struct thread.

sys/kern/kern_time.c
801

It is documented in exterror(9)

helps to read exterror(9) carefully <blush>

In D56490#1293494, @imp wrote:

helps to read exterror(9) carefully <blush>

I did not intended to shame, sorry if it can be read this way. I tried to note that this is specified and is not a tribal knowledge to posses.

really update w/o the extra

In D56490#1293526, @kib wrote:
In D56490#1293494, @imp wrote:

helps to read exterror(9) carefully <blush>

I did not intended to shame, sorry if it can be read this way. I tried to note that this is specified and is not a tribal knowledge to posses.

What you said was fine. exterror(9) is explicit which is great! I jsut was confused.

Always glad to see more use of EXTERROR.

sys/kern/kern_time.c
430

Suggest being more specific:

 if (!timespecvalid_interval(ats)) {
        if (ats->tv_sec < 0)
                return (EXTERROR(EINVAL, "Backward clock jump"));
        return (EXTERROR(EINVAL, "Out of range tv_nsec %jd",
            ats->tv_nsec));
}

Your title on this phabricator review needs to be more specific. There are about 11,000 instances of EINVAL returns and you are not adding EXTERROR for all of them. So being a bit more specific about which subsystem you are doing is needed.

This revision is now accepted and ready to land.Tue, Jun 9, 6:40 PM