Date: Sat, 8 Aug 2020 12:29:23 -0700 From: Paul Eggert <eggert@cs.ucla.edu> Message-ID: <20200808192923.23487-2-eggert@cs.ucla.edu> | a reliable way | to determine whether 0 represents failure or buffer exhaustion This is not necessary. The cases can be distinguished by setting errno to 0 before the call, and checking afterwards (if 0 is returned) - if errno is set there was an error, if it is still 0, it is something else. A 0 return can indicate a successful return with no data, not just conversion failure or buffer exhaustion (eg: strftime(buf, sizeof buf, "%Z", tm); in a timezone where there are no timezone names). This is easy to avoid by simply using res = strftime(buf, sizeof buf, " %Z", tm); where any successful return must be at least 1, so 0 always indicates some kind of error (including when sizeof(buf) == 0, though that's not a case worth worrying about). Then the code simply uses buf+1 as the result (and res-1 as the length) after it has been determined that there was no error (ie: res >= 1). kre

Date: Sat, 8 Aug 2020 22:25:47 +0100 From: "Clive D.W. Feather" <clive@davros.org> Message-ID: <20200808212547.GK1036@davros.org> | C has no requirement to leave errno unchanged on success. Yes, I know, but functions can be defined to work that way. It turns out not to matter here really, as it is trivial to arrange that a successful call always returns >= 1, leaving 0 to always mean that an error occurred (after which errno can be checked). | strftime is not documented as using errno, so that wording applies. It is going to be specified to use errno in the next POSIX update. This is already in the draft: ERRORS CX These functions shall fail if: [EOVERFLOW] The format string includes a %s conversion and the number of seconds since the Epoch cannot be represented in a time_t. These functions may fail if: [EINVAL] The format string includes a %s conversion and the number of seconds since the Epoch would be negative. kre

Date: Sun, 9 Aug 2020 00:03:55 -0700 From: Paul Eggert <eggert@cs.ucla.edu> Message-ID: <b07884cb-f690-54db-4127-459b9ad8c502@cs.ucla.edu> | The tzdb's date.c program, This is for handling date +fmt ? right What we do is simply pass the "+fmt" arg to strftime, unaltered. The '+' that we know must be there, or we wouldn't be calling strftime() from date at all, serves as the extra dummy padding character. It doesn't matter what that char is, as long as it isn't '\0' or '%', so the '+' is fine. if (*argv && **argv == '+') format = *argv; [...] while (strftime(buf, bufsiz, format, tm) == 0) /* make buffer bigger, etc, if we can */ The '+' then gets dropped from the result. (void)printf("%s\n", buf + 1); | One can work around the problem by allocating memory for a larger format | (take the original format and prepend a space, then strip a leading space | from the output), One certainly could, but as the format already has that '+' in it, one doesn't really need that complication, it all just works. There might be some cases where something like that is actually needed, but those seem rare, I don't think I have encountered one. | I submitted a bug report to POSIX about this, here: | https://www.austingroupbugs.net/view.php?id=1386 Yes, I saw, I added a note to it similar to my e-mail here. | which proposes that errno should be preserved on successful returns | of 0 from strftime. I have no problem with that part of it. We just don't need the ERANGE stuff added. kre

Date: Sun, 9 Aug 2020 11:00:35 -0700 From: Paul Eggert <eggert@cs.ucla.edu> Message-ID: <35abe9d1-18c3-dd9b-cd6b-5d46866f66df@cs.ucla.edu> | I installed the attached patch to do it. The only comment I'd make about that, and this is just style, is that instead of ... | + timeout(stdout, format ? format : "+%+", tmp); we simply init format before (actually during) the arg processing, as ... format = "+%a %b %e %H:%M:%S %Z %Y"; (other than the leading '+' (which could be a space there, or other) the content of the string isn't important here). and then allow a '+' arg to override it. There's actually a bit more to the arg processing than I showed, but nothing important for this issue. | General-purpose routines don't always have the luxury of knowing that | format[-1] is available. No, they don't, but someone designing a general purpose routine to use strftime (or for that matter, anything) should design it around the characteristics of strftime (or whatever). Do it with that in mind, and whatever extra needs doing to handle strftime (etc) properly becomes much easier, than if one ignores the properties of the interfaces to be used to implement the routine, and has to coerce a poorly designed API to fit. kre ps: to get the ERANGE accepted by posix, it needs to be the standard (or at least common) behaviour first, POSIX specifies what the standard is, it doesn't (usually anyway) invent it (or adopt someone else's invention). It is very rare for something to be added which would make a formerly conforming implementation become non-conforming.

Date: Tue, 11 Aug 2020 19:05:39 -0700 From: Paul Eggert <eggert@cs.ucla.edu> Message-ID: <85aad8e3-99dd-d428-947b-3f53b30be64f@cs.ucla.edu> | It's too late for that. Many APIs have already been designed. APIs that are ever used in a way that the ambiguous case can arise?? I think it might be just %Z (rarely) and %p (sometimes) which ever produce an empty string. How many of those other APIs would ever be given either "%Z" or "%p" as their arg? If that doesn't happen they don't have a problem. | The POSIX draft is already doing that to strftime, by requiring EOVERFLOW | where many implementations don't do EOVERFLOW, As I'm sure you have seen on the Austin list, I agree with your point there, that should not be required. But to add it as a "may fail" all that is needed is any one implementation that does that. | and similarly for EINVAL for the odd | implementations that don't support negative time_t. Same there. | As long as it's specifying | errno de novo anyway, it might as well fix this longtime botch. The problem with this one is that all implementations need to support it, or it is worthless. If code has to deal with the possibility that the implementation doesn't return ERANGE (and of course, most don't currently) then that it might happen to be running on one that does is largely immaterial. The burden to add this kind of change is considerably greater. | I have a similar beef with mktime, by the way: there's not a | well-designed way to distinguish success from failure. It isn't trivial, but if -1 is returned, it isn't all that difficult to check whether there's any possibility the tm might be set to 23:59:59 Dec 31 1969 (UTC). In most cases it is safe to simply assume that won't be the case, and simply treat -1 as the error indicator only, and not a possibly valid time_t but when more is needed, the check is not all that difficult (after a successful return, which is what it would be if the -1 indicates that 1969 date, the tm has been normalised, Dec 31, 1969 was a Wednesday, so set tm_wday to 0, and then check if on return it has become 3 or 4 (4, in case of a timezone east of UTC). If not, then we know the -1 means error. After that there are just two cases to check for, if tm_wday is 3, then tm_mon would be 11, tm_year 69, tm_mday 31, and tm_sec 59 (leave hour and min unchecked initially to avoid needing to worry about the zone offset). If wday is 4, then it should be tm_mon 0, tm_mday 1, tm_year 70, tm_sec 59. If those checks fail (any) then the -1 was an error return. After that if you really wanted you could look at the zone offset, and check tm_hour and tm_min as well, but I think it would be reasonable in that case, to assume that the -1 really did mean 31 Dec 1969 23:59:59. Another way, if -1 is returned, is to take the original struct tm do tm_sec++ and then mktime() again. If you now get 0 as the result than the original -1 was the 31 Dec 1969 version, otherwise it was an error (whether the 2nd mktime() returns -1 or not - anything but 0). kre

Date: Thu, 13 Aug 2020 16:18:13 -0700 From: Paul Eggert <eggert@cs.ucla.edu> Message-ID: <bd31f6a2-b365-88b6-f341-9a3324142fc7@cs.ucla.edu> | It's pretty common to design a general-purpose API I think "common" is a stretch here. Yes, I'm sure there are a few cases, but it is hardly common. Further, those that already exist either have had to already deal with this problem, or treat it as irrelevant. Either way we don't really need to be overly concerned (that is, if this issue were breaking applications it would have become more publicised long before now). Anything for the future can easily be designed to deal with it. | > The problem with this one is that all implementations need to support it, | > or it is worthless. | | That same objection applies to the EOVERFLOW errno change Not really, as in many (perhaps most) applications, the tm being passed to strftime came from a time_t (via localtime) and hence the tm is known to fit in a time_t and EOVERFLOW cannot occur. The only cases where it is possible is where the tm comes from elsewhere (eg: a parsedate variant that returned the tm instead of a time_t) and wasn't subject to an earlier mktime() or similar. I think that is quite rare. Apps can mostly ignore the EOVERFLOW possibility, as generally it cannot happen - it is there (though should be "may fail" in strftime) for completeness rather than any real necessity. That is, some implementation coding very carefully wondered "what do I do here?" It is safe to simply ignore that one. On the contrary it is also safe to check for it, ever in an implementation which never generates the error. Adding code to deal with an error that never occurs is wasteful perhaps, but harmless. This isn't true of ERANGE, if you're coding to deal with the possibility that it might not be implemented, then it is useless, if you're assuming that ERANGE will be generated, then your code breaks on implementations (standards compliant or not) which don't generate it. | > when more is needed, the check is not all that difficult | > (after a successful return, which is what it would be if the -1 indicates | > that 1969 date, the tm has been normalised, Dec 31, 1969 was a Wednesday, | > so set tm_wday to 0, and then check if on return it has become 3 or 4 | | I've used that hack too, but neither POSIX nor C11 require the hack to work. You mean because POSIX doesn't say what will be in the struct tm (*timeptr) after a failure, and you're imagining the possibility that the implementation might return -1, and simultaneously set the struct tm to be the Dec 31 23:59:59 UTC value ? Really? I suppose that might be technically allowed, as the spec doesn't indicate what will be in the struct after an error, but I don't really believe that this is something to be concerned about. We could have added to POSIX in the "ERRORS" case "the contents of the structure pointed to by timepointer shall be unspecified, except that they shall not represent any value which may have a time_t representation of (time_t)-1." (or words to that effect). That wouldn't break any actual implementations, and would allow this "hack" to work. | What I'd propose is that the standard specifies that mktime must | not not update tm_yday when it fails. | This matches all implementations that I know of, and | allows the hack to work. That's reasonable too. | > Another way, if -1 is returned, is to take the original struct tm | > do tm_sec++ and then mktime() again. | | This won't work if tm_sec == INT_MAX. Really! Can you find a reasonable value for INT_MAX where value%60 == 59 ? I can't... So all that is needed to deal with that case is if (tm_sec == INT_MAX) /* error case */; else { tm_sec++; if (mktime() != 0) /*error case*; else /* really was Dec 31 1969! */; } But if you're really concerned about that, then do it as if (tm_sec == INT_MAX) { tm_sec--; if (mktime() != (time_t)-2) /* error case */; else /* was Dec 31, 1969, UTC */; } else { tm_sec++; if (mktime() != (time_t)0) /* error case */; else /* was Dec 31, 1969 */; } which could be coded more compactly (just one mktime call) by adding a new var to hold the "not an error" value expected from the two cases. kre

Date: Fri, 14 Aug 2020 15:48:28 -0700 From: Paul Eggert <eggert@cs.ucla.edu> Message-ID: <4e86e8ac-767c-1d63-15a1-83ed3fb4b5e9@cs.ucla.edu> | If this is the sort of thing portable code needs to do in order | to determine whether mktime has succeeded, then mktime's API | needs improving. It is actually more than that, struct tm has outlived its useful lifetime, and because of the way it is used there's no really good way to extend it - we need a whole new struct, and a whole set of new functions to deal with it. Attempting to make minor improvements to what we have now isn't worth the effort. kre