sys/time: rework saturation ifdef to avoid direct arch ref

[rlibby]

Summary

Suggested by: kib
Fixes: 00dccc3164c6 ("sys/time: appease gcc -Wtype-limits")

Test Plan

make tinderbox

[kib]

Did you considered using SIZEOF_LONG and SIZEOF_INT instead of 8/4?

[rlibby]

In D56401#1291100, @kib wrote:

Did you considered using SIZEOF_LONG and SIZEOF_INT instead of 8/4?

I hadn't, but looking at it now...

It seems directly defining __SIZEOF_TIME_T in terms of __SIZEOF_LONG__ and __SIZEOF_INT__ would require also knowing how int64_t is actually defined for a given arch. For example if some arch (maybe arm?) uses int64_t for time_t but where __SIZEOF_LONG__ might be 4, we would then need to test for that and then in that case actually make it __SIZEOF_LONG_LONG__, right?

We could add __SIZEOF_INT64_T etc to sys/sys/_types.h and define them in terms of __SIZEOF_LONG__ etc, and then #define __SIZEOF_TIME_T __SIZEOF_INT64. Maybe that would make sense if there were some other use or consumer?

[kib]

In D56401#1291388, @rlibby wrote:

In D56401#1291100, @kib wrote:

Did you considered using SIZEOF_LONG and SIZEOF_INT instead of 8/4?

I hadn't, but looking at it now...

It seems directly defining __SIZEOF_TIME_T in terms of __SIZEOF_LONG__ and __SIZEOF_INT__ would require also knowing how int64_t is actually defined for a given arch. For example if some arch (maybe arm?) uses int64_t for time_t but where __SIZEOF_LONG__ might be 4, we would then need to test for that and then in that case actually make it __SIZEOF_LONG_LONG__, right?

We could add __SIZEOF_INT64_T etc to sys/sys/_types.h and define them in terms of __SIZEOF_LONG__ etc, and then #define __SIZEOF_TIME_T __SIZEOF_INT64. Maybe that would make sense if there were some other use or consumer?

I mean add __SIZEOF_TIME_T in the same location in machine/_type.h where time_t is defined. Of course it must use the same __SIZEOF_<uppercase type name> as the type used for time_t.
But I do not insist.

[rlibby]

In D56401#1291397, @kib wrote:

Of course it must use the same __SIZEOF_<uppercase type name> as the type used for time_t.

But which type is that? The basic type for int64_t is defined in sys/sys/_types.h according to __SIZEOF_LONG__, not directly in machine/_types.h. Maybe the problem I'm imagining doesn't exist, but my worry is that on some arch we might have to

#if __SIZEOF_LONG__ == 8
#define __SIZEOF_TIME_T __SIZEOF_LONG__
#else
#define __SIZEOF_TIME_T __SIZEOF_LONG_LONG__
#endif

as sys/sys/_types.h does, and if we're going to do that, should we just define __SIZEOF_INT64_T in sys/sys/types.h?

Do you see this as potentially being useful for more CPP definitions?

[kib]

In D56401#1291442, @rlibby wrote:

In D56401#1291397, @kib wrote:

Of course it must use the same __SIZEOF_<uppercase type name> as the type used for time_t.

But which type is that? The basic type for int64_t is defined in sys/sys/_types.h according to __SIZEOF_LONG__, not directly in machine/_types.h. Maybe the problem I'm imagining doesn't exist, but my worry is that on some arch we might have to

#if __SIZEOF_LONG__ == 8
#define __SIZEOF_TIME_T __SIZEOF_LONG__
#else
#define __SIZEOF_TIME_T __SIZEOF_LONG_LONG__
#endif

as sys/sys/_types.h does, and if we're going to do that, should we just define __SIZEOF_INT64_T in sys/sys/types.h?

It is per arch. E.g. it is SIZEOF_LONG on amd64, but SIZEOF_LONG_LONG on arm, and SIZEOF_INT on i386.

Do you see this as potentially being useful for more CPP definitions?

In fact I might want SIZEOF_TIME32_T as well, for compat32.

[rlibby]

What do you think of this? https://github.com/rlibby/freebsd/commit/73605b3a9268d5ea7ad52650ba9ba7c808842dc5

Same is attached if you prefer: {F152561163}

[imp]

So what's wrong with the #ifdef i386 that we're going to tear down in the near future?
This seems to be way over-engineering a solution to a bogus gcc warning.
I'd really rather have #ifdef _KERNEL around the two sat functions instead of this over-engineering.

[imp]

sys/sys/time.h

359

Or maybe just _ts.tv_sec >= INT_MAX and <= INT_MIN here, and accept the issue markj pointed out that you could have up to a second of truncation when _ts.tv_sec == INT_MAX/INT_MIN.

[rlibby]

I don't feel strongly about it, beyond wanting the gcc build to work. If @kib and @imp you guys don't agree on the direction here, my inclination is just to leave it as it is after 00dccc3164c6dff38350a1baeeea7238acf2efc3 and move on.

[kib]

I would much prefer the SIZEOF approach. It is much cleaner IMO, and it makes some more things possible with compat32, after addition of SIZEOF_TIME32_T.
I will not block any kind of commit, of course.