On 11/7/2019 9:50 PM, Mike Hommey wrote:
> 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:
>> 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.
This is true, the 'struct rev_info' doesn't get cleaned up at the end.
It is probably a lot of work to find all the consumers and get them to
clean everything up, and the value is rather low. I believe the expectation
is that each process will only run a revision walk at most once.
>> 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.
This is probably true. Sounds like a quick contribution could
be in order?
Thanks,
-Stolee
