All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: git@vger.kernel.org
Cc: "Junio C Hamano" <gitster@pobox.com>,
	"Derrick Stolee" <derrickstolee@github.com>,
	"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Subject: [PATCH v2 09/14] commit-graph: fix memory leak in misused string_list API
Date: Fri,  4 Mar 2022 19:32:12 +0100	[thread overview]
Message-ID: <patch-v2-09.14-3fadb265d13-20220304T182902Z-avarab@gmail.com> (raw)
In-Reply-To: <cover-v2-00.14-00000000000-20220304T182902Z-avarab@gmail.com>

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>
---
 builtin/commit-graph.c | 6 +++---
 commit-graph.c         | 4 ++--
 commit-graph.h         | 2 +-
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/builtin/commit-graph.c b/builtin/commit-graph.c
index 4247fbde95a..51c4040ea6c 100644
--- 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 265c010122e..d0c94600bab 100644
--- 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 @@ static void expire_commit_graphs(struct write_commit_graph_context *ctx)
 }
 
 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 04a94e18302..2e3ac35237e 100644
--- 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);
-- 
2.35.1.1248.gb68c9165ad8


  parent reply	other threads:[~2022-03-04 18:32 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-03-02 17:10 [PATCH 00/14] tree-wide: small fixes for memory leaks Ævar Arnfjörð Bjarmason
2022-03-02 17:10 ` [PATCH 01/14] index-pack: fix " Ævar Arnfjörð Bjarmason
2022-03-02 20:36   ` Derrick Stolee
2022-03-03 16:19     ` Ævar Arnfjörð Bjarmason
2022-03-02 17:10 ` [PATCH 02/14] merge-base: free() allocated "struct commit **" list Ævar Arnfjörð Bjarmason
2022-03-02 17:10 ` [PATCH 03/14] diff.c: free "buf" in diff_words_flush() Ævar Arnfjörð Bjarmason
2022-03-02 17:10 ` [PATCH 04/14] urlmatch.c: add and use a *_release() function Ævar Arnfjörð Bjarmason
2022-03-02 17:10 ` [PATCH 05/14] remote-curl.c: free memory in cmd_main() Ævar Arnfjörð Bjarmason
2022-03-02 17:10 ` [PATCH 06/14] bundle: call strvec_clear() on allocated strvec Ævar Arnfjörð Bjarmason
2022-03-02 17:10 ` [PATCH 07/14] transport: stop needlessly copying bundle header references Ævar Arnfjörð Bjarmason
2022-03-02 20:47   ` Derrick Stolee
2022-03-03 16:20     ` Ævar Arnfjörð Bjarmason
2022-03-02 17:10 ` [PATCH 08/14] submodule--helper: fix trivial leak in module_add() Ævar Arnfjörð Bjarmason
2022-03-02 17:10 ` [PATCH 09/14] commit-graph: fix memory leak in misused string_list API Ævar Arnfjörð Bjarmason
2022-03-02 17:10 ` [PATCH 10/14] commit-graph: stop fill_oids_from_packs() progress on error and free() Ævar Arnfjörð Bjarmason
2022-03-02 17:10 ` [PATCH 11/14] lockfile API users: simplify and don't leak "path" Ævar Arnfjörð Bjarmason
2022-03-02 17:10 ` [PATCH 12/14] range-diff: plug memory leak in common invocation Ævar Arnfjörð Bjarmason
2022-03-02 17:10 ` [PATCH 13/14] range-diff: plug memory leak in read_patches() Ævar Arnfjörð Bjarmason
2022-03-02 17:10 ` [PATCH 14/14] repository.c: free the "path cache" in repo_clear() Ævar Arnfjörð Bjarmason
2022-03-02 20:58 ` [PATCH 00/14] tree-wide: small fixes for memory leaks Derrick Stolee
2022-03-04 18:32 ` [PATCH v2 " Ævar Arnfjörð Bjarmason
2022-03-04 18:32   ` [PATCH v2 01/14] index-pack: fix " Ævar Arnfjörð Bjarmason
2022-03-04 18:32   ` [PATCH v2 02/14] merge-base: free() allocated "struct commit **" list Ævar Arnfjörð Bjarmason
2022-03-04 18:32   ` [PATCH v2 03/14] diff.c: free "buf" in diff_words_flush() Ævar Arnfjörð Bjarmason
2022-03-04 18:32   ` [PATCH v2 04/14] urlmatch.c: add and use a *_release() function Ævar Arnfjörð Bjarmason
2022-03-04 18:32   ` [PATCH v2 05/14] remote-curl.c: free memory in cmd_main() Ævar Arnfjörð Bjarmason
2022-03-04 18:32   ` [PATCH v2 06/14] bundle: call strvec_clear() on allocated strvec Ævar Arnfjörð Bjarmason
2022-03-04 18:32   ` [PATCH v2 07/14] transport: stop needlessly copying bundle header references Ævar Arnfjörð Bjarmason
2022-03-04 19:10     ` Derrick Stolee
2022-03-04 18:32   ` [PATCH v2 08/14] submodule--helper: fix trivial leak in module_add() Ævar Arnfjörð Bjarmason
2022-03-04 18:32   ` Ævar Arnfjörð Bjarmason [this message]
2022-03-04 18:32   ` [PATCH v2 10/14] commit-graph: stop fill_oids_from_packs() progress on error and free() Ævar Arnfjörð Bjarmason
2022-03-04 18:32   ` [PATCH v2 11/14] lockfile API users: simplify and don't leak "path" Ævar Arnfjörð Bjarmason
2022-03-04 18:32   ` [PATCH v2 12/14] range-diff: plug memory leak in common invocation Ævar Arnfjörð Bjarmason
2022-03-04 18:32   ` [PATCH v2 13/14] range-diff: plug memory leak in read_patches() Ævar Arnfjörð Bjarmason
2022-03-04 18:32   ` [PATCH v2 14/14] repository.c: free the "path cache" in repo_clear() Ævar Arnfjörð Bjarmason
2022-03-04 19:11   ` [PATCH v2 00/14] tree-wide: small fixes for memory leaks Derrick Stolee

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=patch-v2-09.14-3fadb265d13-20220304T182902Z-avarab@gmail.com \
    --to=avarab@gmail.com \
    --cc=derrickstolee@github.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.