Re: [PATCH v5 6/7] revision.c: generation-based topo-order algorithm

From: Mike Hommey <mh@glandium.org>
Date: Nov 8, 2019, 02:50 UTC
Message-ID: <20191108025007.bphr7ynvskeoe6tb@glandium.org>
In-Reply-To: <20181101134623.84055-7-dstolee@microsoft.com>

Replying to this old thread because I have questions regarding the patch, in the context of problems I had downstream, in git-cinnabar.

On Thu, Nov 01, 2018 at 01:46:22PM +0000, Derrick Stolee wrote:

Show 5 quoted lines

>  static void init_topo_walk(struct rev_info *revs)
>  {
>  	struct topo_walk_info *info;
> +	struct commit_list *list;
>  	revs->topo_walk_info = xmalloc(sizeof(struct topo_walk_info));

Not directly from this patch, but there's nothing that frees this memory AFAICS, but then, that's also true for most of the things in struct rev_info.

Show 10 quoted lines

> diff --git a/revision.h b/revision.h
> index fd4154ff75..b0b3bb8025 100644
> --- a/revision.h
> +++ b/revision.h
> @@ -24,6 +24,8 @@
>  #define USER_GIVEN	(1u<<25) /* given directly by the user */
>  #define TRACK_LINEAR	(1u<<26)
>  #define ALL_REV_FLAGS	(((1u<<11)-1) | USER_GIVEN | TRACK_LINEAR)
> +#define TOPO_WALK_EXPLORED	(1u<<27)
> +#define TOPO_WALK_INDEGREE	(1u<<28)

Should these two flags be included in ALL_REV_FLAGS? Should they be reset by reset_revision_walk?

At least for the latter, I'd say yes, otherwise you can end up with missing revs in a subsequent topo-order revwalk.

Mike