Re: [PATCH 21/24] revisions API: release "reflog_info" in release revisions()

From: Derrick Stolee <derrickstolee@github.com>
Date: Mar 9, 2022, 19:24 UTC
Message-ID: <99c429f7-af5d-78ac-9888-9f4f58d01b62@github.com>
In-Reply-To: <patch-21.24-ccf276641d6-20220309T123321Z-avarab@gmail.com>

On 3/9/2022 8:16 AM, Ævar Arnfjörð Bjarmason wrote:

Show 26 quoted lines

> Add a missing reflog_walk_info_release() to "reflog-walk.c" and use it
> in release_revisions().
> 
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
>  reflog-walk.c            | 26 ++++++++++++++++++++++++--
>  reflog-walk.h            |  1 +
>  revision.c               |  1 +
>  t/t0100-previous.sh      |  1 +
>  t/t1401-symbolic-ref.sh  |  2 ++
>  t/t1411-reflog-show.sh   |  1 +
>  t/t1412-reflog-loop.sh   |  2 ++
>  t/t1415-worktree-refs.sh |  1 +
>  8 files changed, 33 insertions(+), 2 deletions(-)
> 
> diff --git a/reflog-walk.c b/reflog-walk.c
> index 8ac4b284b6b..4322228d122 100644
> --- a/reflog-walk.c
> +++ b/reflog-walk.c
> @@ -7,7 +7,7 @@
>  #include "reflog-walk.h"
>  
>  struct complete_reflogs {
> -	char *ref;
> +	const char *ref;
>  	const char *short_ref;

This seems like the opposite change from what I would expect, because the 'const' implies non-ownership.

> -	free(array->ref);
> +	free((char *)array->ref);
> +	free((char *)array->short_ref);

This further makes the point that we should be keeping non-const versions so we can clearly document ownership.

Show 5 quoted lines

> +static void complete_reflogs_clear(void *util, const char *str)
> +{
> +	struct complete_reflogs *array = util;
> +	free_complete_reflog(array);
> +}

Is there a reason we don't do the cast inside?

	free_complete_reflog((struct complete_reflogs *)util);

Thanks, -Stolee