commit-graph: fix memory leak in misused string_list API

When this code was migrated to the string_list API in
d88b14b3fd6 (commit-graph: use string-list API for input, 2018-06-27)
it was made to use used both STRING_LIST_INIT_NODUP and a
strbuf_detach() pattern.

Those should not be used together if string_list_clear() is expected
to free the memory, instead we need to either use STRING_LIST_INIT_DUP
with a string_list_append_nodup(), or a STRING_LIST_INIT_NODUP and
manually fiddle with the "strdup_strings" member before calling
string_list_clear(). Let's do the former.

Since "strdup_strings = 1" is set now other code might be broken by
relying on "pack_indexes" not to duplicate it strings, but that
doesn't happen. When we pass this down to write_commit_graph() that
code uses the "struct string_list" without modifying it. Let's add a
"const" to the variable to have the compiler enforce that assumption.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/builtin/commit-graph.c b/builtin/commit-graph.c
index 4247fbde95af2772f69f9160db07db039c44cda4..51c4040ea6c879bda3631343643f3a52063a1bdc 100644 (file)
--- a/builtin/commit-graph.c
+++ b/builtin/commit-graph.c
@@ -192,7 +192,7 @@ static int git_commit_graph_write_config(const char *var, const char *value,
 
 static int graph_write(int argc, const char **argv)
 {
-       struct string_list pack_indexes = STRING_LIST_INIT_NODUP;
+       struct string_list pack_indexes = STRING_LIST_INIT_DUP;
        struct strbuf buf = STRBUF_INIT;
        struct oidset commits = OIDSET_INIT;
        struct object_directory *odb = NULL;
@@ -273,8 +273,8 @@ static int graph_write(int argc, const char **argv)
 
        if (opts.stdin_packs) {
                while (strbuf_getline(&buf, stdin) != EOF)
-                       string_list_append(&pack_indexes,
-                                          strbuf_detach(&buf, NULL));
+                       string_list_append_nodup(&pack_indexes,
+                                                strbuf_detach(&buf, NULL));
        } else if (opts.stdin_commits) {
                oidset_init(&commits, 0);
                if (opts.progress)

diff --git a/commit-graph.c b/commit-graph.c
index 265c010122e8edefc141f1dec8f506078bbbee19..d0c94600bab8eaaeaec273da9276f0a4743c6ee7 100644 (file)
--- a/commit-graph.c
+++ b/commit-graph.c
@@ -1679,7 +1679,7 @@ int write_commit_graph_reachable(struct object_directory *odb,
 }
 
 static int fill_oids_from_packs(struct write_commit_graph_context *ctx,
-                               struct string_list *pack_indexes)
+                               const struct string_list *pack_indexes)
 {
        uint32_t i;
        struct strbuf progress_title = STRBUF_INIT;
@@ -2259,7 +2259,7 @@ out:
 }
 
 int write_commit_graph(struct object_directory *odb,
-                      struct string_list *pack_indexes,
+                      const struct string_list *const pack_indexes,
                       struct oidset *commits,
                       enum commit_graph_write_flags flags,
                       const struct commit_graph_opts *opts)

diff --git a/commit-graph.h b/commit-graph.h
index 04a94e18302d8d2e9b91933497cfb868a3cf3c12..2e3ac35237e919790555bae317bd580cf065b727 100644 (file)
--- a/commit-graph.h
+++ b/commit-graph.h
@@ -142,7 +142,7 @@ int write_commit_graph_reachable(struct object_directory *odb,
                                 enum commit_graph_write_flags flags,
                                 const struct commit_graph_opts *opts);
 int write_commit_graph(struct object_directory *odb,
-                      struct string_list *pack_indexes,
+                      const struct string_list *pack_indexes,
                       struct oidset *commits,
                       enum commit_graph_write_flags flags,
                       const struct commit_graph_opts *opts);