diff options
author | Jeff King <peff@peff.net> | 2021-11-02 07:35:34 -0400 |
---|---|---|
committer | Junio C Hamano <gitster@pobox.com> | 2021-11-04 12:38:09 -0700 |
commit | 9b591b94038ce8cab9baf66a83ad752824854163 (patch) | |
tree | 9dd3e97554fff5b146da948487f31b2f2cf6d23e /strbuf.c | |
parent | 0cddd84c9f3e9c3d793ec93034ef679335f35e49 (diff) | |
download | git-9b591b94038ce8cab9baf66a83ad752824854163.tar.gz |
strbuf_addftime(): handle "%s" manually
The strftime() function has a non-standard "%s" extension, which prints
the number of seconds since the epoch. But the "struct tm" we get has
already been adjusted for a particular time zone; going back to an epoch
time requires knowing that zone offset. Since strftime() doesn't take
such an argument, round-tripping to a "struct tm" and back to the "%s"
format may produce the wrong value (off by tz_offset seconds).
Since we're already passing in the zone offset courtesy of c3fbf81a85
(strbuf: let strbuf_addftime handle %z and %Z itself, 2017-06-15), we
can use that same value to adjust our epoch seconds accordingly.
Note that the description above makes it sound like strftime()'s "%s" is
useless (and really, the issue is shared by mktime(), which is what
strftime() would use under the hood). But it gets the two cases for
which it's designed correct:
- the result of gmtime() will have a zero offset, so no adjustment is
necessary
- the result of localtime() will be offset by the local zone offset,
and mktime() and strftime() are defined to assume this offset when
converting back (there's actually some magic here; some
implementations record this in the "struct tm", but we can't
portably access or manipulate it. But they somehow "know" whether a
"struct tm" is from gmtime() or localtime()).
This latter point means that "format-local:%s" actually works correctly
already, because in that case we rely on the system routines due to
6eced3ec5e (date: use localtime() for "-local" time formats,
2017-06-15). Our problem comes when trying to show times in the author's
zone, as the system routines provide no mechanism for converting in
non-local zones. So in those cases we have a "struct tm" that came from
gmtime(), but has been manipulated according to our offset.
The tests cover the broken round-trip by formatting "%s" for a time in a
non-system timezone. We use the made-up "+1234" here, which has two
advantages. One, we know it won't ever be the real system zone (and so
we're actually testing a case that would break). And two, since it has a
minute component, we're testing the full decoding of the +HHMM zone into
a number of seconds. Likewise, we test the "-1234" variant to make sure
there aren't any sign mistakes.
There's one final test, which covers "format-local:%s". As noted, this
already passes, but it's important to check that we didn't regress this
case. In particular, the caller in show_date() is relying on localtime()
to have done the zone adjustment, independent of any tz_offset we
compute ourselves. These should match up, since our local_tzoffset() is
likewise built around localtime(). But it would be easy for a caller to
forget to pass in a correct tz_offset to strbuf_addftime(). Fortunately
show_date() does this correctly (it has to because of the existing
handling of %z), and the test continues to pass. So this one is just
future-proofing against a change in our assumptions.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Diffstat (limited to 'strbuf.c')
-rw-r--r-- | strbuf.c | 14 |
1 files changed, 13 insertions, 1 deletions
@@ -1006,7 +1006,12 @@ void strbuf_addftime(struct strbuf *sb, const char *fmt, const struct tm *tm, /* * There is no portable way to pass timezone information to - * strftime, so we handle %z and %Z here. + * strftime, so we handle %z and %Z here. Likewise '%s', because + * going back to an epoch time requires knowing the zone. + * + * Note that tz_offset is in the "[-+]HHMM" decimal form; this is what + * we want for %z, but the computation for %s has to convert to number + * of seconds. */ for (;;) { const char *percent = strchrnul(fmt, '%'); @@ -1019,6 +1024,13 @@ void strbuf_addftime(struct strbuf *sb, const char *fmt, const struct tm *tm, strbuf_addstr(&munged_fmt, "%%"); fmt++; break; + case 's': + strbuf_addf(&munged_fmt, "%"PRItime, + (timestamp_t)tm_to_time_t(tm) - + 3600 * (tz_offset / 100) - + 60 * (tz_offset % 100)); + fmt++; + break; case 'z': strbuf_addf(&munged_fmt, "%+05d", tz_offset); fmt++; |