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>,
	"Elijah Newren" <newren@gmail.com>,
	"Nguyễn Thái Ngọc Duy" <pclouds@gmail.com>,
	"Martin Ågren" <martin.agren@gmail.com>,
	"Andrzej Hunt" <ajrhunt@google.com>, "Jeff King" <peff@peff.net>,
	"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Subject: [PATCH 02/10] merge-recursive.c: call a new unpack_trees_options_init() function
Date: Mon,  4 Oct 2021 02:46:03 +0200	[thread overview]
Message-ID: <patch-02.10-050399cbfbf-20211004T002226Z-avarab@gmail.com> (raw)
In-Reply-To: <cover-00.10-00000000000-20211004T002226Z-avarab@gmail.com>

In the preceding commit we introduced a new UNPACK_TREES_OPTIONS_INIT
macro, but "merge-recursive.c" could not be converted to it since
it (re-)initializes a "struct unpack_trees_options" on the heap.

In order to convert use the macro as the source of truth for
initialization we need to not only convert the initialization in
unpack_trees_start(), but also call the new
unpack_trees_options_init() just after the CALLOC_ARRAY() in
merge_start().

When we call merge_trees() we'll call merge_start() followed by
merge_trees_internal(), and it will call unpack_trees_start() before
it does much of anything. So it's covered by an initialization in
unpack_trees_start().

But when merge_recursive() is called it will call merge_start()
followed by merge_recursive_internal(), which won't call
unpack_trees_start() until its own call call to
merge_trees_internal(), but in the meantime it might end up using that
calloc'd "struct unpack_trees_options".

This was OK before, as setup_unpack_trees_porcelain() would call
strvec_init(), and our "struct dir_struct" in turn is OK with being
NULL'd. Let's convert the former to macro initialization, we'll deal
with the latter in a subsequent commit.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 merge-recursive.c | 3 ++-
 unpack-trees.c    | 8 ++++++--
 unpack-trees.h    | 5 ++++-
 3 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/merge-recursive.c b/merge-recursive.c
index e594d4c3fa1..d24a4903f1d 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -405,7 +405,7 @@ static int unpack_trees_start(struct merge_options *opt,
 	struct tree_desc t[3];
 	struct index_state tmp_index = { NULL };
 
-	memset(&opt->priv->unpack_opts, 0, sizeof(opt->priv->unpack_opts));
+	unpack_trees_options_init(&opt->priv->unpack_opts);
 	if (opt->priv->call_depth)
 		opt->priv->unpack_opts.index_only = 1;
 	else
@@ -3704,6 +3704,7 @@ static int merge_start(struct merge_options *opt, struct tree *head)
 
 	CALLOC_ARRAY(opt->priv, 1);
 	string_list_init_dup(&opt->priv->df_conflict_file_set);
+	unpack_trees_options_init(&opt->priv->unpack_opts);
 	return 0;
 }
 
diff --git a/unpack-trees.c b/unpack-trees.c
index 8ea0a542da8..393c1f35a5d 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -108,8 +108,6 @@ void setup_unpack_trees_porcelain(struct unpack_trees_options *opts,
 	const char **msgs = opts->msgs;
 	const char *msg;
 
-	strvec_init(&opts->msgs_to_free);
-
 	if (!strcmp(cmd, "checkout"))
 		msg = advice_enabled(ADVICE_COMMIT_BEFORE_MERGE)
 		      ? _("Your local changes to the following files would be overwritten by checkout:\n%%s"
@@ -189,6 +187,12 @@ void setup_unpack_trees_porcelain(struct unpack_trees_options *opts,
 		opts->unpack_rejects[i].strdup_strings = 1;
 }
 
+void unpack_trees_options_init(struct unpack_trees_options *o)
+{
+	struct unpack_trees_options blank = UNPACK_TREES_OPTIONS_INIT;
+	memcpy(o, &blank, sizeof(*o));
+}
+
 void clear_unpack_trees_porcelain(struct unpack_trees_options *opts)
 {
 	strvec_clear(&opts->msgs_to_free);
diff --git a/unpack-trees.h b/unpack-trees.h
index ecf256cbcea..892b65ea623 100644
--- a/unpack-trees.h
+++ b/unpack-trees.h
@@ -91,7 +91,10 @@ struct unpack_trees_options {
 	struct checkout_metadata meta;
 };
 
-#define UNPACK_TREES_OPTIONS_INIT { 0 }
+#define UNPACK_TREES_OPTIONS_INIT { \
+	.msgs_to_free = STRVEC_INIT, \
+}
+void unpack_trees_options_init(struct unpack_trees_options *o);
 
 int unpack_trees(unsigned n, struct tree_desc *t,
 		 struct unpack_trees_options *options);
-- 
2.33.0.1404.g83021034c5d


  parent reply	other threads:[~2021-10-04  0:46 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-10-04  0:46 [PATCH 00/10] unpack-trees & dir APIs: fix memory leaks Ævar Arnfjörð Bjarmason
2021-10-04  0:46 ` [PATCH 01/10] unpack-trees.[ch]: define and use a UNPACK_TREES_OPTIONS_INIT Ævar Arnfjörð Bjarmason
2021-10-04  0:46 ` Ævar Arnfjörð Bjarmason [this message]
2021-10-04 13:45   ` [PATCH 02/10] merge-recursive.c: call a new unpack_trees_options_init() function Elijah Newren
2021-10-04 14:41     ` Ævar Arnfjörð Bjarmason
2021-10-04 15:04       ` Elijah Newren
2021-10-04  0:46 ` [PATCH 03/10] unpack-trees.[ch]: embed "dir" in "struct unpack_trees_options" Ævar Arnfjörð Bjarmason
2021-10-04 13:45   ` Elijah Newren
2021-10-04  0:46 ` [PATCH 04/10] unpack-trees API: don't have clear_unpack_trees_porcelain() reset Ævar Arnfjörð Bjarmason
2021-10-04  9:31   ` Phillip Wood
2021-10-04 11:12     ` Ævar Arnfjörð Bjarmason
2021-10-04 13:45   ` Elijah Newren
2021-10-04 15:20     ` Ævar Arnfjörð Bjarmason
2021-10-04 16:28       ` Elijah Newren
2021-10-04  0:46 ` [PATCH 05/10] dir.[ch]: make DIR_INIT mandatory Ævar Arnfjörð Bjarmason
2021-10-04 13:45   ` Elijah Newren
2021-10-04  0:46 ` [PATCH 06/10] dir.c: get rid of lazy initialization Ævar Arnfjörð Bjarmason
2021-10-04 13:45   ` Elijah Newren
2021-10-04  0:46 ` [PATCH 07/10] unpack-trees API: rename clear_unpack_trees_porcelain() Ævar Arnfjörð Bjarmason
2021-10-04  9:38   ` Phillip Wood
2021-10-04 11:10     ` Ævar Arnfjörð Bjarmason
2021-10-04 13:45   ` Elijah Newren
2021-10-04  0:46 ` [PATCH 08/10] unpack-trees: don't leak memory in verify_clean_subdirectory() Ævar Arnfjörð Bjarmason
2021-10-04 13:45   ` Elijah Newren
2021-10-04  0:46 ` [PATCH 09/10] merge.c: avoid duplicate unpack_trees_options_release() code Ævar Arnfjörð Bjarmason
2021-10-04 13:45   ` Elijah Newren
2021-10-04 14:50     ` Ævar Arnfjörð Bjarmason
2021-10-04  0:46 ` [PATCH 10/10] built-ins: plug memory leaks with unpack_trees_options_release() Ævar Arnfjörð Bjarmason
2021-10-04 13:45   ` Elijah Newren
2021-10-04 14:54     ` Ævar Arnfjörð Bjarmason
2021-10-04 13:45 ` [PATCH 00/10] unpack-trees & dir APIs: fix memory leaks Elijah Newren

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-02.10-050399cbfbf-20211004T002226Z-avarab@gmail.com \
    --to=avarab@gmail.com \
    --cc=ajrhunt@google.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=martin.agren@gmail.com \
    --cc=newren@gmail.com \
    --cc=pclouds@gmail.com \
    --cc=peff@peff.net \
    /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.