From: Phillip Wood <phillip.wood123@gmail.com>
Date: Fri, 01 Apr 2022 15:15:06 GMT
Subject: Re: [PATCH v4 05/27] revision.[ch]: split freeing of revs->commit into a function
Message-ID: <e1bdb860-070b-8596-2a32-e9bc3ad354b0@gmail.com>
In-Reply-To: <patch-v4-05.27-2d0ed57ec2e-20220331T005325Z-avarab@gmail.com>

Hi Ævar

On 31/03/2022 02:11, Ævar Arnfjörð Bjarmason wrote:
> Move the existing code for invoking free_commit_list() and setting
> revs->commits to NULL into a new release_revisions_commit_list()
> function. This will be used as part of a general free()-ing mechanism
> for "struct rev_info".
> 
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
>   revision.c | 13 +++++++++----
>   1 file changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/revision.c b/revision.c
> index 7d435f80480..29f102cb83d 100644
> --- a/revision.c
> +++ b/revision.c
> @@ -2923,6 +2923,14 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s
>   	return left;
>   }
>   
> +static void release_revisions_commit_list(struct rev_info *revs)
> +{
> +	if (revs->commits) {

This guard copied for the original is not needed, free_commit_list() 
will handle NULL just fine.

Best Wishes

Phillip
> +		free_commit_list(revs->commits);
> +		revs->commits = NULL;
> +	}
> +}
> +
>   static void add_child(struct rev_info *revs, struct commit *parent, struct commit *child)
>   {
>   	struct commit_list *l = xcalloc(1, sizeof(*l));
> @@ -4080,10 +4088,7 @@ static void create_boundary_commit_list(struct rev_info *revs)
>   	 * boundary commits anyway.  (This is what the code has always
>   	 * done.)
>   	 */
> -	if (revs->commits) {
> -		free_commit_list(revs->commits);
> -		revs->commits = NULL;
> -	}
> +	release_revisions_commit_list(revs);
>   
>   	/*
>   	 * Put all of the actual boundary commits from revs->boundary_commits