Re: [PATCH 2/2] revision: free topo_walk_info before creating a new one in init_topo_walk

[Derrick Stolee <stolee@gmail.com>]

On 11/22/2019 3:37 AM, Mike Hommey wrote:> diff --git a/revision.c b/revision.c
> index 765a56ae33..7e23c5ed08 100644
> --- a/revision.c
> +++ b/revision.c
> @@ -3211,10 +3211,26 @@ static void compute_indegrees_to_depth(struct rev_info *revs,
>  		indegree_walk_step(revs);
>  }
>  
> +static void reset_topo_walk(struct rev_info *revs)
> +{
> +	struct topo_walk_info *info = revs->topo_walk_info;
> +
> +	clear_prio_queue(&info->explore_queue);
> +	clear_prio_queue(&info->indegree_queue);
> +	clear_prio_queue(&info->topo_queue);
> +	clear_indegree_slab(&info->indegree);

In general I like this change. I'm happy that this was split into a
method instead of crammed into the block of the "if" below.

> +	clear_author_date_slab(&info->author_date);

The only issue I have is that the author_date slab should not be
cleared. That is used by more than the topo-walk AND the values for
author dates will not change between subsequent revision walks. Just
drop that line and we should be good to go!

> +
> +	FREE_AND_NULL(revs->topo_walk_info);
> +}
> +
>  static void init_topo_walk(struct rev_info *revs)
>  {
>  	struct topo_walk_info *info;
>  	struct commit_list *list;
> +	if (revs->topo_walk_info)
> +		reset_topo_walk(revs);
> +
>  	revs->topo_walk_info = xmalloc(sizeof(struct topo_walk_info));
>  	info = revs->topo_walk_info;
>  	memset(info, 0, sizeof(struct topo_walk_info));

Thanks,
-Stolee