replace unchecked snprintf calls with heap buffers

We'd prefer to avoid unchecked snprintf calls because
truncation can lead to unexpected results.

These are all cases where truncation shouldn't ever happen,
because the input to snprintf is fixed in size. That makes
them candidates for xsnprintf(), but it's simpler still to
just use the heap, and then nobody has to wonder if "100" is
big enough.

We'll use xstrfmt() where possible, and a strbuf when we need
the resulting size or to reuse the same buffer in a loop.

Signed-off-by: Jeff King <peff@peff.net>

diff --git a/bisect.c b/bisect.c
index 30808cadf7613d73cae2e4caaedd922ba9a48ca0..d12583eaac26edb865f22f98f54dc6495b7f994e 100644 (file)
--- a/bisect.c
+++ b/bisect.c
@@ -200,6 +200,7 @@ static struct commit_list *best_bisection_sorted(struct commit_list *list, int n
 {
        struct commit_list *p;
        struct commit_dist *array = xcalloc(nr, sizeof(*array));
+       struct strbuf buf = STRBUF_INIT;
        int cnt, i;
 
        for (p = list, cnt = 0; p; p = p->next) {
@@ -217,17 +218,18 @@ static struct commit_list *best_bisection_sorted(struct commit_list *list, int n
        }
        QSORT(array, cnt, compare_commit_dist);
        for (p = list, i = 0; i < cnt; i++) {
-               char buf[100]; /* enough for dist=%d */
                struct object *obj = &(array[i].commit->object);
 
-               snprintf(buf, sizeof(buf), "dist=%d", array[i].distance);
-               add_name_decoration(DECORATION_NONE, buf, obj);
+               strbuf_reset(&buf);
+               strbuf_addf(&buf, "dist=%d", array[i].distance);
+               add_name_decoration(DECORATION_NONE, buf.buf, obj);
 
                p->item = array[i].commit;
                p = p->next;
        }
        if (p)
                p->next = NULL;
+       strbuf_release(&buf);
        free(array);
        return list;
 }

diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index f4af2ab37ab66a118c4cdd72354b53e921a10763..197c51912d96bd0d5cbe9f34bbc8da3f2d2623c6 100644 (file)
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -1443,10 +1443,11 @@ static void final(const char *final_pack_name, const char *curr_pack_name,
        if (!from_stdin) {
                printf("%s\n", sha1_to_hex(sha1));
        } else {
-               char buf[48];
-               int len = snprintf(buf, sizeof(buf), "%s\t%s\n",
-                                  report, sha1_to_hex(sha1));
-               write_or_die(1, buf, len);
+               struct strbuf buf = STRBUF_INIT;
+
+               strbuf_addf(&buf, "%s\t%s\n", report, sha1_to_hex(sha1));
+               write_or_die(1, buf.buf, buf.len);
+               strbuf_release(&buf);
 
                /*
                 * Let's just mimic git-unpack-objects here and write

diff --git a/builtin/notes.c b/builtin/notes.c
index 0513f7455dccd15e6ec1c2c1e1d847e66af027a7..7b891471c461ddc3cd2b9d3186cc8b005f68ca2b 100644 (file)
--- a/builtin/notes.c
+++ b/builtin/notes.c
@@ -554,7 +554,7 @@ static int append_edit(int argc, const char **argv, const char *prefix)
        struct notes_tree *t;
        unsigned char object[20], new_note[20];
        const unsigned char *note;
-       char logmsg[100];
+       char *logmsg;
        const char * const *usage;
        struct note_data d = { 0, 0, NULL, STRBUF_INIT };
        struct option options[] = {
@@ -618,17 +618,16 @@ static int append_edit(int argc, const char **argv, const char *prefix)
                write_note_data(&d, new_note);
                if (add_note(t, object, new_note, combine_notes_overwrite))
                        die("BUG: combine_notes_overwrite failed");
-               snprintf(logmsg, sizeof(logmsg), "Notes added by 'git notes %s'",
-                       argv[0]);
+               logmsg = xstrfmt("Notes added by 'git notes %s'", argv[0]);
        } else {
                fprintf(stderr, _("Removing note for object %s\n"),
                        sha1_to_hex(object));
                remove_note(t, object);
-               snprintf(logmsg, sizeof(logmsg), "Notes removed by 'git notes %s'",
-                       argv[0]);
+               logmsg = xstrfmt("Notes removed by 'git notes %s'", argv[0]);
        }
        commit_notes(t, logmsg);
 
+       free(logmsg);
        free_note_data(&d);
        free_notes(t);
        return 0;

diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
index 9e53a1a7ca4a4ffa0bd810673d432c6f2449f5a5..f54d7b5028a455eae9b133e72ac24333c5035f2e 100644 (file)
--- a/builtin/rev-parse.c
+++ b/builtin/rev-parse.c
@@ -213,13 +213,14 @@ static int show_abbrev(const unsigned char *sha1, void *cb_data)
 
 static void show_datestring(const char *flag, const char *datestr)
 {
-       static char buffer[100];
+       char *buffer;
 
        /* date handling requires both flags and revs */
        if ((filter & (DO_FLAGS | DO_REVS)) != (DO_FLAGS | DO_REVS))
                return;
-       snprintf(buffer, sizeof(buffer), "%s%lu", flag, approxidate(datestr));
+       buffer = xstrfmt("%s%lu", flag, approxidate(datestr));
        show(buffer);
+       free(buffer);
 }
 
 static int show_file(const char *arg, int output_prefix)