On Mon, Jun 29, 2015 at 06:22:47PM -0400, Eric Sunshine wrote:
> Clients of strbuf rightly expect the buffer to grow as needed in
> order to complete the requested operation. It is, therefore, both
> weird and expectation-breaking for strbuf_addftime() to lack this
> behavior. Worse, it doesn't even signal when the format has failed
> due to insufficient buffer space.
>
> How about taking this approach (or something similar), instead, which
> grows the strbuf as needed?
Here's a patch, on top of jk/date-mode-format (I think it would also be
fine to just squash into the tip commit; the explanation in the commit
message is sufficiently mirrored in the code comment).
-- >8 --
Subject: [PATCH] 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@xxxxxxxxxxxxxx>
Signed-off-by: Jeff King <peff@xxxxxxxx>
---
strbuf.c | 38 +++++++++++++++++++++-----------------
t/t6300-for-each-ref.sh | 10 ++++++++++
2 files changed, 31 insertions(+), 17 deletions(-)
diff --git a/strbuf.c b/strbuf.c
index a7ba028..e5e7370 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -712,29 +712,33 @@ char *xstrfmt(const char *fmt, ...)
void strbuf_addftime(struct strbuf *sb, const char *fmt, const struct tm *tm)
{
+ size_t hint = 128;
size_t len;
- /*
- * strftime reports "0" if it could not fit the result in the buffer.
- * Unfortunately, it also reports "0" if the requested time string
- * takes 0 bytes. So if we were to probe and grow, we have to choose
- * some arbitrary cap beyond which we guess that the format probably
- * just results in a 0-length output. Since we have to choose some
- * reasonable cap anyway, and since it is not that big, we may
- * as well just grow to their in the first place.
- */
- strbuf_grow(sb, 128);
+ if (!*fmt)
+ return;
+
+ strbuf_grow(sb, hint);
len = strftime(sb->buf + sb->len, sb->alloc - sb->len, fmt, tm);
if (!len) {
/*
- * Either we failed, or the format actually produces a 0-length
- * output. There's not much we can do, so we leave it blank.
- * However, the output array is left in an undefined state, so
- * we must re-assert our NUL terminator.
+ * strftime reports "0" if it could not fit the result in the buffer.
+ * Unfortunately, it also reports "0" if the requested time string
+ * takes 0 bytes. So our strategy is to munge the format so that the
+ * output contains at least one character, and then drop the extra
+ * character before returning.
*/
- sb->buf[sb->len] = '\0';
- } else {
- sb->len += len;
+ struct strbuf munged_fmt = STRBUF_INIT;
+ strbuf_addf(&munged_fmt, "%s ", fmt);
+ while (!len) {
+ hint *= 2;
+ strbuf_grow(sb, hint);
+ len = strftime(sb->buf + sb->len, sb->alloc - sb->len,
+ munged_fmt.buf, tm);
+ }
+ strbuf_release(&munged_fmt);
+ len--; /* drop munged space */
}
+ strbuf_setlen(sb, sb->len + len);
}
diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
index c7f368c..7c9bec7 100755
--- a/t/t6300-for-each-ref.sh
+++ b/t/t6300-for-each-ref.sh
@@ -235,6 +235,16 @@ test_expect_success 'Check format of strftime date fields' '
test_cmp expected actual
'
+test_expect_success 'exercise strftime with odd fields' '
+ echo >expected &&
+ git for-each-ref --format="%(authordate:format:)" refs/heads >actual &&
+ test_cmp expected actual &&
+ long="long format -- $_z40$_z40$_z40$_z40$_z40$_z40$_z40" &&
+ echo $long >expected &&
+ git for-each-ref --format="%(authordate:format:$long)" refs/heads >actual &&
+ test_cmp expected actual
+'
+
cat >expected <<\EOF
refs/heads/master
refs/remotes/origin/master
--
2.5.0.rc0.336.g8460790
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html