strbuf: make strbuf_addftime more robust
The return value of strftime is poorly designed; when it returns 0, the caller cannot tell if the buffer was not large enough, or if the output was actually 0 bytes. In the original implementation of strbuf_addftime, we simply punted and guessed that our 128-byte hint would be large enough. We can do better, though, if we're willing to treat strftime like less of a black box. We can munge the incoming format to make sure that it never produces 0-length output, and then "fix" the resulting output. That lets us reliably grow the buffer based on strftime's return value. Clever-idea-by: Eric Sunshine <sunshine@sunshineco.com> Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King committed
Jun 30, 2015 at 09:26 UTC
e4f031e34b08e3217c10942e682920a6939308a0
2 files changed
+31
-17
strbuf.c
+21
-17
@@ -712,29 +712,33 @@ char *xstrfmt(const char *fmt, ...)
712
713
void strbuf_addftime(struct strbuf *sb, const char *fmt, const struct tm *tm)
714
{
715
+ size_t hint = 128;
716
size_t len;
717
717
- /*
718
- * strftime reports "0" if it could not fit the result in the buffer.
719
- * Unfortunately, it also reports "0" if the requested time string
720
- * takes 0 bytes. So if we were to probe and grow, we have to choose
721
- * some arbitrary cap beyond which we guess that the format probably
722
- * just results in a 0-length output. Since we have to choose some
723
- * reasonable cap anyway, and since it is not that big, we may
724
- * as well just grow to their in the first place.
725
- */
726
- strbuf_grow(sb, 128);
718
+ if (!*fmt)
719
+ return;
720
+
721
+ strbuf_grow(sb, hint);
722
len = strftime(sb->buf + sb->len, sb->alloc - sb->len, fmt, tm);
723
724
if (!len) {
725
/*
731
- * Either we failed, or the format actually produces a 0-length
732
- * output. There's not much we can do, so we leave it blank.
733
- * However, the output array is left in an undefined state, so
734
- * we must re-assert our NUL terminator.
726
+ * strftime reports "0" if it could not fit the result in the buffer.
727
+ * Unfortunately, it also reports "0" if the requested time string
728
+ * takes 0 bytes. So our strategy is to munge the format so that the
729
+ * output contains at least one character, and then drop the extra
730
+ * character before returning.
731
*/
736
- sb->buf[sb->len] = '\0';
737
- } else {
738
- sb->len += len;
732
+ struct strbuf munged_fmt = STRBUF_INIT;
733
+ strbuf_addf(&munged_fmt, "%s ", fmt);
734
+ while (!len) {
735
+ hint *= 2;
736
+ strbuf_grow(sb, hint);
737
+ len = strftime(sb->buf + sb->len, sb->alloc - sb->len,
738
+ munged_fmt.buf, tm);
739
+ }
740
+ strbuf_release(&munged_fmt);
741
+ len--; /* drop munged space */
742
}
743
+ strbuf_setlen(sb, sb->len + len);
744
}
t/t6300-for-each-ref.sh
+10
@@ -235,6 +235,16 @@ test_expect_success 'Check format of strftime date fields' '
235
test_cmp expected actual
236
'
237
238
+test_expect_success 'exercise strftime with odd fields' '
239
+ echo >expected &&
240
+ git for-each-ref --format="%(authordate:format:)" refs/heads >actual &&
241
+ test_cmp expected actual &&
242
+ long="long format -- $_z40$_z40$_z40$_z40$_z40$_z40$_z40" &&
243
+ echo $long >expected &&
244
+ git for-each-ref --format="%(authordate:format:$long)" refs/heads >actual &&
245
+ test_cmp expected actual
246
+'
247
+
248
cat >expected <<\EOF
249
refs/heads/master
250
refs/remotes/origin/master