time: Add EXTERROR for all EINVAL returns

[imp]

Summary

Sponsored by: Netflix

[markj]

sys/kern/kern_time.c

386

430

1291

This is spelled "clock_id invalid" elsewhere.

[kib]

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.

[imp]

reviews

[kib]

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 ↗

(On Diff #175860)

This and related bits are unrelated, right?

[imp]

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 ↗

(On Diff #175860)

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

[kib]

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.

[kib]

sys/kern/kern_time.c

801

It is documented in exterror(9)

[imp]

helps to read exterror(9) carefully <blush>

[kib]

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.

[imp]

really update w/o the extra

[imp]

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.

[mckusick]

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));
}