Re: [PATCH 04/10] unpack-trees API: don't have clear_unpack_trees_porcelain() reset

[Phillip Wood <phillip.wood123@gmail.com>]

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

> 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,
>