[tz] Minor (unimportant really) technical UB bug in strftime() ?
Jonathan Leffler
jonathan.leffler at gmail.com
Tue Nov 8 14:15:39 UTC 2022
The code has:
tm = *t;
mkt = mktime(&tm);
This makes a copy of the `const struct tm` that is the argument to
strftime(), and passes that copy to mktime(), which reads a few fields and
sets most fields. But since it is working on a copy of the argument to
strftime(), I don't see a problem. Can you elaborate?
Is there a good reason not to write this?
struct tm tm = *t;
time_t mkt = mktime(&tm);
It's a stylistic question wholly separate from the const-ness issue.
On Tue, Nov 8, 2022 at 6:07 AM Robert Elz via tz <tz at iana.org> wrote:
> The code for the %s strftime conversion starts as:
> (this taken from tzcode2022f strftime.c - but I suspect
> that many versions are the same)
>
> case 's':
> {
> struct tm tm;
> char
> buf[INT_STRLEN_MAXIMUM(
> time_t) +
> 1];
> time_t mkt;
>
> tm = *t;
> mkt = mktime(&tm);
>
> The "tm = *t;" line is the problem. It (or more correctly, something
> to replace that) is needed, as *t is const, strftime() is not permitted
> to modify it, yet the struct tm * passed to mktime() is not const, as
> mktime is sometimes required to modify that struct before returning.
>
> However, mktime() refers only to the fields tm_year tm_mon tm_mday
> tm_hour tm_min tm_sec and tm_isdst of the struct passed to it. The
> %s conversion is defined as producing (in a decimal string format) the
> result of mktime() applied to the struct tm passed to strftime.
> Hence an application which intends to use strftime(..., "%s", ...)
> need only initialise those 7 fields, the other 2 (currently) standard
> fields (tm_yday and tm_wday) are explicitly ignored by mktime() on
> input, so there is no need to assign those values, and any other
> fields that might exist in a particular implementation are out of
> scope for a portable (conforming) application to touch, so cannot
> be initialised.
>
> Since referring to an uninitialised variable is, I believe, technically
> undefined behaviour, doing a struct copy (copying all fields) can have
> that effect. (An application could memset() the entire struct before
> providing the values it must add, but is not required to do that).
>
> It seems likely that the simple fix is to replace the struct assignment
> with 7 individual assignment statements
>
> tm.tm_year = t->tm_year;
> tm.tm_mon = t->tm_mon;
> /* ... */
>
> On the other hand, hardware that balks at simply loading or copying,
> uninitialised values, without using them for any purpose is not exactly
> common (and half the world's code would probably break if an implementation
> that worked in such a manner was ever created) so I'd call this one of
> the less significant issues to worry about fixing.
>
> kre
>
>
--
Jonathan Leffler <jonathan.leffler at gmail.com> #include <disclaimer.h>
Guardian of DBD::Informix - v2018.1031 - http://dbi.perl.org
"Blessed are we who can laugh at ourselves, for we shall never cease to be
amused."
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mm.icann.org/pipermail/tz/attachments/20221108/e5e3ddb7/attachment.htm>
More information about the tz
mailing list