Dead code in strftime()?

strftime() in the 2024b tzcode distribution includes if (!p) { errno = EOVERFLOW; return 0; } AFAICS this is dead code, except maybe in the case that the call passes s = NULL, maxsize = 0, which doesn't seem like a case that the code intends to support (and EOVERFLOW would be a rather odd choice of error code if that's the idea). newstrftime.3.txt offers a clue about the intent: This function may fail if: [EOVERFLOW] The format includes an %s conversion and the number of seconds since the Epoch cannot be represented in a time_t. But the %s conversion code never returns zero. There's a suggestive comment there: /* If mktime fails, %s expands to the value of (time_t) -1 as a failure marker; this is better in practice than strftime failing. */ So it looks to me like somebody changed the error handling in the %s stanza but didn't bother to remove the now-dead path in the caller nor update the man page. Am I missing something? regards, tom lane

Date: Tue, 12 Nov 2024 12:54:34 -0500 From: Tom Lane via tz <tz@iana.org> Message-ID: <1212112.1731434074@sss.pgh.pa.us> | strftime() in the 2024b tzcode distribution includes | | if (!p) { | errno = EOVERFLOW; | return 0; | } | | AFAICS this is dead code, It shouldn't be. | except maybe in the case that the call | passes s = NULL, maxsize = 0, which doesn't seem like a case that | the code intends to support (and EOVERFLOW would be a rather odd | choice of error code if that's the idea). POSIX requires ERANGE in that case (maxsize == 0 regardless of anything else, since there would not be room for even the terminating '\0'). (The C standard just requires an error, since it doesn't have errno or any of its values). | newstrftime.3.txt | offers a clue about the intent: | | This function may fail if: | | [EOVERFLOW] | The format includes an %s conversion and the number of seconds | since the Epoch cannot be represented in a time_t. That can happen whenever sizeof(int) >= sizeof(time_t) (and in some cases wen sizeof(int) < sizeof(time_t) but not by very much - which are extermely unlikely in any practical system). It doesn't happen on most modern systems with 64 bit time_t and 32 bit int -- but not everything is like that. | But the %s conversion code never returns zero. That sounds to be what is broken. | There's a suggestive comment there: | | /* If mktime fails, %s expands to the | value of (time_t) -1 as a failure | marker; this is better in practice | than strftime failing. */ Whoever thought that is, frankly, a fool. Pandering to broken application code with "anything will do" is never the right solution, the broken code doesn't get fixed, and its users just get nonsense results. That should be undone. POSIX actually says: [EOVERFLOW] The format string includes a %s conversion and the number of seconds since the Epoch cannot be represented in a time_t. Doing that is required. C doesn't have a %s conversion, so it says nothing about this case, but certainly allows strftime to return 0 in error cases. It isn't even as if this was a case like asctime() or ctime() used to be, where they returned a pointer to the resulting string, so apps would just do things like printf("%s", ctime(&time)); (or asctime(&tm)) since these "never fail" (which they do, or at least ctime() could). Code like that doesn't work with strftime() as it doesn't return the buffer, but the length - so there's no reason at all for applications to fail to check it, after all it can also fail if the supplied buffer just isn't big enough. kre

On 2024-11-12 11:59, Robert Elz via tz wrote:
| except maybe in the case that the call | passes s = NULL, maxsize = 0, which doesn't seem like a case that | the code intends to support (and EOVERFLOW would be a rather odd | choice of error code if that's the idea).
POSIX requires ERANGE in that case (maxsize == 0 regardless of anything else
No, because the current POSIX and C standards both require s to be nonnull. Behavior is undefined otherwise. This is true regardless of whether maxsize is zero. As mentioned in "Allow zero length operations on null pointers", WG14 N3322 (2024-08-28) <https://www.open-std.org/jtc1/sc22/wg14/www/docs/n3322.pdf>, the next C standard may relax this to allow null s with maxsize = 0; although strftime is not explicitly called out in N3322 it may be added to the list. If this happens, the just-fixed tzcode will set errno=ERANGE and return 0, which should be conforming behavior assuming POSIX follows C2y's lead.
| /* If mktime fails, %s expands to the | value of (time_t) -1 as a failure | marker; this is better in practice | than strftime failing. */
Whoever thought that is, frankly, a fool. Pandering to broken application code with "anything will do" is never the right solution,
Foolish or not, the longstanding practice in tzcode is to pander to broken application code. For example, strftime with the format "%H:%M:%S %?" gives you the hour, minute, second and a "?" rather than failing and returning 0. Another example: if you set TZ="BAD" strftime with "%H:%M:%S" will give you UTC hour, minute and second rather than failing and returning 0.
POSIX actually says:
[EOVERFLOW] The format string includes a %s conversion and the number of seconds since the Epoch cannot be represented in a time_t.
Doing that is required.
It's not required. It's in a "may fail" section, not a "must fail" section, and this means POSIX does not require an implementation to fail when the number of seconds does not fit in time_t. For example, the glibc implementation on the platform I'm using to write this email generates the correct decimal representation without reporting an error, and POSIX is fine with that. (If someone wrote a tzcode patch to do that, it'd be a welcome improvement on what it currently does.) As an aside, this issue is not that important in practice, as it comes up mostly on obsolescent platforms that will stop working in the year 2038 for other reasons.

Date: Tue, 12 Nov 2024 14:47:58 -0800 From: Paul Eggert <eggert@cs.ucla.edu> Message-ID: <f1f22ba6-0814-4bee-a1e2-6de0a321c717@cs.ucla.edu> | No, because the current POSIX and C standards both require s to be | nonnull. Behavior is undefined otherwise. This is true regardless of | whether maxsize is zero. OK, this issue wasn't the real point of the message. | Foolish or not, the longstanding practice in tzcode is to pander to | broken application code. That is a pity. The %? thing is a mistake as well, as someday C or posix might define a %? conversion, and then any code getting away with strings like that is going to break. Better to break them now while there are less of them (people copy code that seems to work.) | It's not required. It's in a "may fail" section, not a "must fail" Sure, but surely the intent of the option is to allow the implementation to write the correct value which would be the time_t if only a time_t had enough bits to contain it - not to simply generate any random value (even if you don't consider it random) and write that. They also allow EINVAL for dates before the epoch - that doesn't mean that you can just generate any random thing for the conversions instead of returning EINVAL in that case. | glibc generates the correct decimal representation without reporting an | error, and POSIX is fine with that. Yes, that's fine. No issue with that at all. That's exactly why it needs to be may fail, not must fail - not so you can generate nonsense results. | As an aside, this issue is not that important in practice, as it comes | up mostly on obsolescent platforms that will stop working in the year | 2038 for other reasons. Not necessarily, there are tricks that 32 bit time_t systems can play to increase the range way beyond 2038 (by moving the lower limit closer to the epoch). The extreme of making time_t unsigned means things run until 21xx (for some small xx I forget and am too lazy to calculate). By the time that is no longer plausible, none of us are likely to care any more (and quite probably those systems will be long extinct). But apart from that, 64 bit time_t is as big as we will ever need (the range of times it represents go back & forward beyond the known expected life of the universe) but 32 bit int forever is no certainty. Systems with 64 bit int are not at all impossible - not all that far into the future (quite possibly before 2038). As soon as that happens, struct tm values that cannot be represented as a time_t are alive again (and we go back to never having localtime() fail because the year is out of range). kre

On 2024-11-12 16:42, Robert Elz wrote:
That is a pity. The %? thing is a mistake as well, as someday C or posix might define a %? conversion, and then any code getting away with strings like that is going to break.
Come to think of it, it would be better if "%?" generated "%?" rather than "?". Both are common practice, but "%?" is easier to debug, particularly if you extend it to weirder strings like "%9?". I just now checked by running the shell command "date '+%?'", and GNU/Linux, AIX and Solaris output "%?" whereas macOS and the BSDs (output "?". (musl outputs "" which is less helpful.) Would there be any objection to changing tzcode strftime so that "%?" and other invalid formats output themselves? That would match GNU/Linux, AIX, and Solaris instead of matching the BSDs.
surely the intent of the option is to allow the implementation to write the correct value
That's not how POSIX usually works. A "may fail" error means that the implementation can do what it likes (as an extension) if it decides not to fail. To take an extreme example, on a machine with 32-bit signed time_t, POSIX allows time(0) to wrap around after 2038, and return negative time_t values; this is because EOVERFLOW is a "may fail" for "time".
That's exactly why it needs to be may fail, not must fail - not so you can generate nonsense results.
As I hope the "time (0)" example illustrates, whether something is nonsense is in the eye of the beholder. POSIX doesn't have an opinion on that.
32 bit int forever is no certainty.
Yes of course, and that's why it'd be good to fix tzcode to do the right thing here, which would be to generate the correct value regardless of whether it fits into time_t - something that glibc already does. Although this is doable without resorting anything fancy like GMP, I'm a bit crunched for time right now so I hope someone else does it. (It's not high priority or else somebody would have done it already....)

Date: Tue, 12 Nov 2024 20:54:47 -0800 From: Paul Eggert <eggert@cs.ucla.edu> Message-ID: <0a79acab-8509-488e-ba69-287bb92e4c97@cs.ucla.edu> | Would there be any objection to changing tzcode strftime so that "%?" | and other invalid formats output themselves? That would match GNU/Linux, | AIX, and Solaris instead of matching the BSDs. The BSDs largely run tzcode, so do what they do because tzcode does. Were I to alter that on NetBSD (that is, no longer do what tzcode does) it would be to return an error in that case. As long as we're copying tzcode here, what incorrect thing you choose to do, amongst several choices (another would be to omit both the % and the ? and any intervening pretend to be flags/widths/...) doesn't matter very much, applications should not be doing that anyway. | > surely the intent of the option is to allow the implementation | > to write the correct value | | That's not how POSIX usually works. It actually is, it just isn't the way you want to interpret it. See section 2.3 (Error Numbers) where it says: The ERRORS section on each reference page specifies which error conditions shall be detected by all implementations (``shall fail'') and which may be optionally detected by an implementation (``may fail''). There's nothing new there, we all knew that. It continues: If no error condition is detected, the action requested shall be successful. "shall be successful". | To take an extreme example, on a machine with 32-bit signed | time_t, POSIX allows time(0) to wrap around after 2038, and return | negative time_t values; this is because EOVERFLOW is a "may fail" for | "time". Nonsense, that is not what it is allowing at all. On a system with 32 bit time_t (which would include NetBSD running an app compiled on NetBSD 4 or earlier, which we still support) there are two possibilities. Either we simply act as if the system really had a 32 bit time_t, and return the 32 low order bits of the time, which definitely will wrap around after 2038 (unless we alter the 32->64 bit mapping, which for present purposes, assume we won't do). That correctly emulates a true 32 bit time_t system, which is what NetBSD 4 (and earlier) were. In that case, that is all that is possible - that value must be returned, and no EOVERFLOW is possible - the system cannot tell the difference between Tue Jan 19 03:14:08 UTC 2038 (time_t == 0x8000000 on a 64 bit time_t) and Fri Dec 13 20:45:52 UTC 1901 (time_t == 0x8000000 on a 32 bit time_t), both in UTC for simplicity, otherwise the actual times vary by timezone. You might believe it impossible for 1901 to be the correct year, but it certainly can be - if I decide I want to run my system with the date set to back then, perhaps as an emulation of events at that time, if only they had modern computers to record them. It is simply impossible to tell the difference between a 32 bit time_t that has wrapped, and one that legitimately has that value (unless you're doing the calculation that moves from 0x7FFFFFFF to 0x80000000 which the time() system call, or library function more likely these days, certainly is not). Whatever system call actually is executed to return the 32 bit time_t (or storing the value into a mapped memory page or however it is done) -- and remember everything but the kernel only knows 32 bit time_t values in this situation -- could have detected that the overflow happened and returned an error instead (even a genuine 32 bit time_t kernel could have done that). But if that didn't happen we have no choice but to return the value the kernel gave us back to the application. The kernel says X is the number of seconds since the epoch. We trust that, we have no reason, or justification, to do otherwise. However, given that modern NetBSD has 64 bit time_t's, when we map that to the 32 bit value for old system emulation purposes, we can tell the difference between 2038 and 2001 being the actual system time. In the latter case simply return that, in the former case POSIX allows us to set EOVERFLOW as we know we are unable to do as the spec for time requires, which is: The time() function shall return the value of time in seconds since the Epoch. There is no option there, either we do that (and be successful) or (unless some other error occurs, such as the pointer supplied being invalid) we must return EOVERFLOW. However, if (as best as we are able to determine) the time really is 1901 then the time_t that says that must be returned since even in a 32 bit (signed) time_t, that fits (well, from late Dec 13 1901 onwards it fits). | As I hope the "time (0)" example illustrates, whether something is | nonsense is in the eye of the beholder. POSIX doesn't have an opinion on | that. You're wrong, it does. | Yes of course, and that's why it'd be good to fix tzcode to do the right | thing here, which would be to generate the correct value regardless of | whether it fits into time_t - something that glibc already does. There's an obvious intermediate step, which is trivial to program, which could be done, which isn't as good as handling any random sized time_t and any random sized int in the struct tm, but would be adequate for practical purposes to handle the cases that actually arise. That is, make an __mktime() function with a signature intmax_t __mktime(struct tm *); (that can take a const struct tm * perhaps, see later). Then time_t mktime(struct tm *tm) { const int sverr = errno; time_t t; /* * here normalise the values in the struct if * __mktime()'s arg is const, so it cannot do it/ */ // code to do that omitted errno -; intmax_t result = __mktime(tm); if (result == -1 && errno != 0) return (time_t) -1; if (result < TIMET_MIN || result > TIMET_MAX) { errno = EOVERFLOW; return (time_t) -1; } t = result; // do *tm = localtime(&t); if needed return t; } and in strftime() simply call __mktime to get the result as an intmax_t and then format that (whether it would fit in a time_t or not) unless __mktime() returns an error, in which case strftime() simply returns 0, leaving errno from __mktime(). The implementation of __mktime() is simply the current mktime() adapted to use intmax_t instead of time_t. But you're right, this isn't high priority, and having systems where strftime(%s) can fail is good to help achieve better code portability. But as POSIX said above: If no error condition is detected, the action requested shall be successful. and for strftime's %s conversion, "successful" means: Replaced by the number of seconds since the Epoch as a decimal number, calculated as described for mktime(). Nothing at all about fitting in a time_t or actually calling mktime(), just calculates by the same method as described for mktime(). mktime() itself has a limitation, it can only return values that fit in a time_t so is required to return EOVERFLOW if the value doesn't fit. Similarly, implementations of strftime() are allowed to call mktime() to do the calculation "as described for mktime()" (not required to do it that way, but permitted) and then if they do, and mktime() returns EOVERFLOW then strftime() is as well - but in that case it must return 0 as its result (and what is in the buffer is unspecified in that case). There really are no options here beyond that if you want to conform. Either you return the result the standard says you must return, and a "success" return value (> 0 in this case) or you return 0 (failed) and set errno. Nothing else conforms. kre

Date: Thu, 14 Nov 2024 02:20:21 +0700 From: Robert Elz via tz <tz@iana.org> Message-ID: <8899.1731525621@jacaranda.noi.kre.to> | what incorrect thing you choose to do I should point out that by that I meant IMO, or perhaps, IMO suboptimal thing that you choose to do - this is a case where it clearly is undefined in the standard what happens (%? and similar unspecified translations) so implementations can do whatever they prefer. I believe that it is best to cause the application to fail (even calling abort() would be acceptable, though returning 0 immediately - not processing any more of the format string - is probably a better approach). kre

On 11/13/24 11:20, Robert Elz wrote:
If no error condition is detected, the action requested shall be successful.
"shall be successful".
Yes, of course, and the current tzcode succeeds by outputting the textual equivalent of (time_t) -1. It would be better if it output the numerically correct value, and the implementation strategy you suggested had already occurred to me. However, it hasn't been coded up and tested yet.

Date: Wed, 13 Nov 2024 22:54:06 -0700 From: Paul Eggert <eggert@cs.ucla.edu> Message-ID: <49958d54-4e0e-4556-9ed3-571617c437b4@cs.ucla.edu> | Yes, of course, and the current tzcode succeeds by outputting the | textual equivalent of (time_t) -1. That isn't the number of seconds since the epoch calculated as specified for mktime() unless the struct tm happened to contain the local timezone equiv of Dec 31 1969 23:59:59 UTC. Since that number is what it is specified to produce, it isn't successful unless it does. | It would be better if it output the numerically correct value, That, or return an error, either is acceptable - except for a tm which is in fact (time_t)-1 inserting "-1" is failing. kre

On 11/14/24 00:53, Robert Elz wrote:
Since that number is what it is specified to produce, it isn't successful unless it does.
Not quite following what you mean here. Here's a different example to illustrate the point. If an application calls open(".", -1) where the "-1" contains bits that aren't valid for oflag, EINVAL is a "may fail" error so POSIX does not require 'open' to fail with EINVAL. An implementation is allowed to use the oflag bits it understands while ignoring the flags it doesn't. In other words, for a "may fail" error, the implementation is allowed to not fail, and to support an extension instead. Different people may disagree on what extensions are reasonable; POSIX itself doesn't take a position on that.

Date: Thu, 14 Nov 2024 10:19:00 -0700 From: Paul Eggert <eggert@cs.ucla.edu> Message-ID: <5ee435ad-7d6a-4e11-82a2-97931bd56b7a@cs.ucla.edu> | Here's a different example to illustrate the point. If an application | calls open(".", -1) where the "-1" contains bits that aren't valid for | oflag, EINVAL is a "may fail" error so POSIX does not require 'open' to | fail with EINVAL. An implementation is allowed to use the oflag bits it | understands while ignoring the flags it doesn't. Yes. | In other words, for a "may fail" error, the implementation is allowed to | not fail, and to support an extension instead. Provided that when it doesn't fail it does everything it is required to do, as it is specified, if the application doesn't actually make use of some not standard extension (such as by setting a non-standard bit in the flags for open, as one example). Once the application starts indulging in non-standard behaviour the implementation has much more latitude, the standard only applies as much as the implementation chooses to make it apply. | Different people may disagree on what extensions are reasonable; | POSIX itself doesn't take a position on that. Sure, and that's exactly the position wrt things like %? in strftime's format string. You think one thing, I think a different thing, neither of us is right or wrong as judged by the standard (but in real terms of course, I am right!;) But unless the user actually presents input in a form that is different than the standard says is defined, the implementation is required to make output in the form (or one of the forms) that the standard says it can, otherwise there's no way a portable application can predict what is going to happen. When it was written it might never have heard of your implementation, or its differences, it obeys the standard, you should return what the standard says you should (that or stop pretending that your implementation in any way is standards conforming.) When the user uses a local documented extension, then the implementation should do what it documents for that extension to do. If the user does something defined to produce unspecified results, or worse, undefined behaviour, then you get to do what you like. So, for example, with strftime, if the user passes a NULL pointer instead of the struct tm *, then you can largely do whatever the hell you please. (You could make that a shorthand for using the current time with the format string for example, or you could dump core with a segmentation violation). But you don't get to not produce a decimal representation of seconds since the epoch in place of %s (with no non-standard flags, or widths, etc) just because the result doesn't fit in a time_t. You either generate the correct result anyway, or return 0 and EOVERFLOW. No other options there. kre

On 2024-11-14 13:34, Robert Elz wrote:
you don't get to not produce a decimal representation of seconds since the epoch in place of %s (with no non-standard flags, or widths, etc) just because the result doesn't fit in a time_t.
This issue is moot for tzcode, now that I've fixed its strftime %s to generate the correct string even for counts outside time_t range. https://lists.iana.org/hyperkitty/list/tz@iana.org/message/UZPM7SJJSOX4PNJFP...

On 11/13/24 12:20, Robert Elz wrote:
amongst several choices (another would be to omit both the % and the ? and any intervening pretend to be flags/widths/...) doesn't matter very much, applications should not be doing that anyway.
Yes, musl omits everything which isn't very helpful. Since it doesn't matter much I installed the attached, which makes tzcode agree with glibc, AIX, Solaris, and slightly simplifies tzcode to boot. I'll add the wider-int %s fix to my list of things to do.
participants (3)
-
Paul Eggert
-
Robert Elz
-
Tom Lane