Re: [PATCH 04/10] unpack-trees API: don't have clear_unpack_trees_porcelain() reset
From: Phillip Wood <hidden>
Date: 2021-10-04 09:31:54
Hi Ævar On 04/10/2021 01:46, Ævar Arnfjörð Bjarmason wrote:
Change the clear_unpack_trees_porcelain() to be like a *_release() function, not a *_reset() (in strbuf.c terms). Let's move the only API user that relied on the latter to doing its own unpack_trees_options_init(). See the commit that introduced unpack_trees_options_init() for details on the control flow involved here.
Before this change if there was a call to unpack_trees() after clear_unpack_trees_porcelain() then that caller would get the default error messages. After this change we end up with a use-after-free error in that situation. I found the subject line of this patch hard to understand, the commit message explains what it is doing but I'm still not sure what the motivation for this change is. Best Wishes Phillip
quoted hunk ↗ jump to hunk
Signed-off-by: Ævar Arnfjörð Bjarmason <redacted> --- merge-recursive.c | 1 + unpack-trees.c | 1 - 2 files changed, 1 insertion(+), 1 deletion(-)diff --git a/merge-recursive.c b/merge-recursive.c index d24a4903f1d..a77f66b006c 100644 --- a/merge-recursive.c +++ b/merge-recursive.c@@ -442,6 +442,7 @@ static void unpack_trees_finish(struct merge_options *opt) { discard_index(&opt->priv->orig_index); clear_unpack_trees_porcelain(&opt->priv->unpack_opts); + unpack_trees_options_init(&opt->priv->unpack_opts); } static int save_files_dirs(const struct object_id *oid,diff --git a/unpack-trees.c b/unpack-trees.c index 94767d3f96f..e7365322e82 100644 --- a/unpack-trees.c +++ b/unpack-trees.c@@ -197,7 +197,6 @@ void clear_unpack_trees_porcelain(struct unpack_trees_options *opts) { strvec_clear(&opts->msgs_to_free); dir_clear(&opts->dir); - memset(opts->msgs, 0, sizeof(opts->msgs)); } static int do_add_entry(struct unpack_trees_options *o, struct cache_entry *ce,