Subject: Re: [PATCH 21/22] merge-ort: fix two leaks when handling directory rename modifications

From: Junio C Hamano <[email-masked]>
Date: Wed, 04 Sep 2024 15:56:07 -0700

Patrick Steinhardt <[email-masked]> writes:

> There are two leaks in `apply_directory_rename_modifications()`:
>
>   - We do not release the `dirs_to_insert` string list.
>
>   - We do not release some `conflict_info` we put into the
>     `opt->priv->paths` string map.
>
> The former is trivial to fix. The latter is a bit less straight forward:
> the `util` pointer of the string map may sometimes point to data that
> has been allocated via `CALLOC()`, while at other times it may point to
> data that has been allocated via a `mem_pool`.
>
> It very much seems like an oversight that we didn't also allocate the
> conflict info in this code path via the memory pool, though. So let's
> fix that, which will also plug the memory leak for us.
>
> Signed-off-by: Patrick Steinhardt <[email-masked]>
> ---

May be nice if we can hear from the original author and the area
expert.

>  merge-ort.c                         | 4 +++-
>  t/t6423-merge-rename-directories.sh | 1 +
>  2 files changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/merge-ort.c b/merge-ort.c
> index 3752c7e595d..0ed3cd06b1a 100644
> --- a/merge-ort.c
> +++ b/merge-ort.c
> @@ -2710,7 +2710,7 @@ static void apply_directory_rename_modifications(struct merge_options *opt,
>  		struct conflict_info *dir_ci;
>  		char *cur_dir = dirs_to_insert.items[i].string;
>  
> -		CALLOC_ARRAY(dir_ci, 1);
> +		dir_ci = mem_pool_calloc(&opt->priv->pool, 1, sizeof(*dir_ci));
>  
>  		dir_ci->merged.directory_name = parent_name;
>  		len = strlen(parent_name);
> @@ -2838,6 +2838,8 @@ static void apply_directory_rename_modifications(struct merge_options *opt,
>  	 * Finally, record the new location.
>  	 */
>  	pair->two->path = new_path;
> +
> +	string_list_clear(&dirs_to_insert, 0);
>  }
>  
>  /*** Function Grouping: functions related to regular rename detection ***/
> diff --git a/t/t6423-merge-rename-directories.sh b/t/t6423-merge-rename-directories.sh
> index 88d1cf2cde9..4aaaf38be68 100755
> --- a/t/t6423-merge-rename-directories.sh
> +++ b/t/t6423-merge-rename-directories.sh
> @@ -25,6 +25,7 @@ test_description="recursive merge with directory renames"
>  #                     underscore notation is to differentiate different
>  #                     files that might be renamed into each other's paths.)
>  
> +TEST_PASSES_SANITIZE_LEAK=true
>  . ./test-lib.sh
>  . "$TEST_DIRECTORY"/lib-merge.sh