On Wed, Sep 12, 2018 at 12:14:25AM -0400, Jeff King wrote:
> > + ALLOC_ARRAY(list, from->nr);
> > for (i = 0; i < from->nr; i++) {
> > - struct object *from_one = from->objects[i].item;
> > + list[i] = (struct commit *)from->objects[i].item;
>
> Some of the objects in my array are not commits, but rather tags, so
> this is a bogus cast.
>
> You can see that the original code peeled them and threw away
> non-commits:
>
> >
> > - if (from_one->flags & assign_flag)
> > - continue;
> > - from_one = deref_tag(the_repository, from_one, "a from object", 0);
> > - if (!from_one || from_one->type != OBJ_COMMIT) {
> > - /* no way to tell if this is reachable by
> > - * looking at the ancestry chain alone, so
> > - * leave a note to ourselves not to worry about
> > - * this object anymore.
> > - */
> > - from->objects[i].item->flags |= assign_flag;
> > - continue;
> > - }
>
> So presumably we'd need to do something similar.
This patch seems to fix it for me. It's more or less a reversion of the
hunk above, though I didn't dig into whether I'm violating some other
assumption in your new code.
I think this function leaks "list" both from the location I noted here,
as well as from normal exit
---
commit-reach.c | 25 ++++++++++++++++++-------
1 file changed, 18 insertions(+), 7 deletions(-)
diff --git a/commit-reach.c b/commit-reach.c
index 622eeb313d..abe90a2f55 100644
--- a/commit-reach.c
+++ b/commit-reach.c
@@ -547,20 +547,31 @@ int can_all_from_reach_with_flag(struct object_array *from,
{
struct commit **list = NULL;
int i;
+ int nr_commits;
int result = 1;
ALLOC_ARRAY(list, from->nr);
+ nr_commits = 0;
for (i = 0; i < from->nr; i++) {
- list[i] = (struct commit *)from->objects[i].item;
+ struct object *from_one = from->objects[i].item;
- if (parse_commit(list[i]) ||
- list[i]->generation < min_generation)
- return 0;
+ from_one = deref_tag(the_repository, from_one,
+ "a from object", 0);
+ if (!from_one || from_one->type != OBJ_COMMIT) {
+ from->objects[i].item->flags |= assign_flag;
+ continue;
+ }
+
+ list[nr_commits] = (struct commit *)from_one;
+ if (parse_commit(list[nr_commits]) ||
+ list[nr_commits]->generation < min_generation)
+ return 0; /* is this a leak? */
+ nr_commits++;
}
- QSORT(list, from->nr, compare_commits_by_gen);
+ QSORT(list, nr_commits, compare_commits_by_gen);
- for (i = 0; i < from->nr; i++) {
+ for (i = 0; i < nr_commits; i++) {
/* DFS from list[i] */
struct commit_list *stack = NULL;
@@ -603,7 +614,7 @@ int can_all_from_reach_with_flag(struct object_array *from,
}
cleanup:
- for (i = 0; i < from->nr; i++) {
+ for (i = 0; i < nr_commits; i++) {
clear_commit_marks(list[i], RESULT);
clear_commit_marks(list[i], assign_flag);
}
--
2.19.0.600.ga229f7d059
