ctime.3: Add a note about a possible return value to the description of localtime(3)

[gbe]

Summary

The localtime(3) function returns a NULL pointer, if the passed in time
translates to a year that will not fit in an integer type. It is stricly
recommended to check the return value to avoid garage output.

Reported by: mckusick
MFC after: 1 week

Test Plan

mandoc output review and 'mandoc -Tlint' checks

[mckusick]

I think that it would be better to just add a sentence in the description of localtime that notes that it may return NULL since that is where it is most likely to be seen by readers of the manual page.

diff --git a/contrib/tzcode/stdtime/ctime.3 b/contrib/tzcode/stdtime/ctime.3
index 771027fa62ec..c577073a9a2a 100644
--- a/contrib/tzcode/stdtime/ctime.3
+++ b/contrib/tzcode/stdtime/ctime.3
@@ -94,6 +94,10 @@ and returns a pointer to a
 (described below) which contains
 the broken-out time information for the value after adjusting for the current
 time zone (and any other factors such as Daylight Saving Time).
+When the passed in time translates to a year that will not fit
+in an integer type,
+.Fn localtime
+returns NULL.
 Time zone adjustments are performed as specified by the
 .Ev TZ
 environment variable (see

Otherwise add it as a standard section as noted in my inline comment.

I also recommend that you do the formatting changes as a separate commit so that people looking at this commit do not have to sift through it to find the very important addition.

contrib/tzcode/stdtime/ctime.3

373–378

If you add a new section, I propose that you use a more standard section header:

.Sh RETURN VALUES
When successful, the
.Fn localtime
function return a pointer to a
.Dq Fa struct tm .
When the passed in time translates to a year that will not fit in an integer type,
.Fn localtime
returns NULL.

[imp]

contrib/tzcode/stdtime/ctime.3

373–378

Is it a 'integral type' or is it specifically time_t?

I'd be tempted to add some vague statement about 'the standard allows any error to cause a NULL return. FreeBSD will return an error when...'

387

Should these reference the specific POSIX standard?
I know that's likely for another commit since this is just cleaning up .Tn, but seeing the change makes me wonder as we've often moved that way in other man pages.

[gbe]

• Don't add a CAVEATS section, add a note the description of localtime(3)

[imp]

contrib/tzcode/stdtime/ctime.3

98

Please specify what kind of integer. Is it an int, time_t or ????
This is too vague because nearly all times can fit into an integral type if it's a long long, but only a limited subset if it is a int32_t.

[mckusick]

Proposed new wording.

contrib/tzcode/stdtime/ctime.3

98

The field for year in the tm structure is defined as int'. So it depends on how the architecture defines int'. Perhaps the wording should be:

When the passed in time translates to a year that will not fit in in an int' for architectures that define int' to be int32_t,

[imp]

contrib/tzcode/stdtime/ctime.3

98

When the passed in time translates to a year that overflows the hosts'
.Dv int
type.

is likely fine. And it makes perfect sense now where we go awry and what the impact will be (in practical terms, not much).

[gbe]

• Refine wording about the possible return value of locatime(3)

[gbe]

Update wording to address comments.

[imp]

contrib/tzcode/stdtime/ctime.3

101

"for architectures where
.Dv int
is 32-bits,"

instead, because int32_t is defined in terms of things like int, long, etc so this may confuse people.

[gbe]

• Use a more generic term for int32_t

[gbe]

• Grammar

[rpokala]

Is the problem when the year won't fit into an int, or specifically when it won't fit into a 32-bit int?

I would think that the problem is when it won't fit into an int regardless of size, but that we would only run into that on architectures that use 32-bit ints.

contrib/tzcode/stdtime/ctime.3

97

"passed-in"

[imp]

In D36515#830781, @rpokala wrote:

Is the problem when the year won't fit into an int, or specifically when it won't fit into a 32-bit int?

I would think that the problem is when it won't fit into an int regardless of size, but that we would only run into that on architectures that use 32-bit ints.

I just realized all our architectures an int is 32-bits, so I've suggested a simpler wording above. But that's only possible on !i386 archtectures with the 8 byte time_t since i386 can't specify a time that's 2 billion years in the future/past.

contrib/tzcode/stdtime/ctime.3

99–100

I hate to keep picking nits, please forgive me for not realizing this comment sooner.

I think we can just remove the 'for architectures where int is 32-bits' entirely and have it read OK.

When the specified time translates to a year that will not fit in an
.Dv int ,
.Fn localtime
returns NULL.

[gbe]

• Address comments

[imp]

I'm happy now. Thanks for putting up with what must have been frustrating comments as I circled in on this wording.

[gbe]

In D36515#831121, @imp wrote:

I'm happy now. Thanks for putting up with what must have been frustrating comments as I circled in on this wording.

No problem, that are what differentials are for.

[mckusick]

It took us a long time to figure out one sentence, but I am happy with the result. Thanks Gordon for your patience.

I fixed ls(1) and find(1). There are only about a hundred more applications and libraries to go (see https://reviews.freebsd.org/D36474).