Re: [PATCH 04/10] unpack-trees API: don't have clear_unpack_trees_porcelain() reset
- From
- Phillip Wood <phillip.wood123@gmail.com>
- Date
- Oct 4, 2021, 09:31 UTC
- Message-ID
- <f24733e9-8742-33b6-5fae-93d89698cc00@gmail.com>
- In-Reply-To
- <patch-04.10-9d62edd35bf-20211004T002226Z-avarab@gmail.com>
Hi Ævar
On 04/10/2021 01:46, Ævar Arnfjörð Bjarmason wrote:
Show 6 quoted lines
> 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
Show 31 quoted lines
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
> 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,
>