Skip to content

Commit 10e8a52

Browse files
chiyutianyigitster
authored andcommitted
negotiator/skipping: fix some problems in mark_common()
The mark_common() method in negotiator/skipping.c was converted from recursive to iterative in 4654134 (negotiator/skipping: avoid stack overflow, 2022-10-25), but there is some more work to do: 1. prio_queue() should be used with clear_prio_queue(), otherwise there will be a memory leak. 2. It does not do duplicate protection before prio_queue_put(). (The COMMON bit would work here, too.) 3. When it translated from recursive to iterative it kept "return" statements that should probably be "continue" statements. 4. It does not attempt to parse commits, and instead returns immediately when finding an unparsed commit. This is something that it did in its original version, so maybe it is by design, but it doesn't match the doc comment for the method. Helped-by: Derrick Stolee <[email protected]> Signed-off-by: Han Xin <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent 8e21ff5 commit 10e8a52

File tree

1 file changed

+15
-7
lines changed

1 file changed

+15
-7
lines changed

negotiator/skipping.c

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -85,29 +85,37 @@ static int clear_marks(const char *refname, const struct object_id *oid,
8585
}
8686

8787
/*
88-
* Mark this SEEN commit and all its SEEN ancestors as COMMON.
88+
* Mark this SEEN commit and all its parsed SEEN ancestors as COMMON.
8989
*/
9090
static void mark_common(struct data *data, struct commit *seen_commit)
9191
{
9292
struct prio_queue queue = { NULL };
9393
struct commit *c;
9494

95+
if (seen_commit->object.flags & COMMON)
96+
return;
97+
9598
prio_queue_put(&queue, seen_commit);
99+
seen_commit->object.flags |= COMMON;
96100
while ((c = prio_queue_get(&queue))) {
97101
struct commit_list *p;
98-
if (c->object.flags & COMMON)
99-
return;
100-
c->object.flags |= COMMON;
102+
101103
if (!(c->object.flags & POPPED))
102104
data->non_common_revs--;
103105

104106
if (!c->object.parsed)
105-
return;
107+
continue;
106108
for (p = c->parents; p; p = p->next) {
107-
if (p->item->object.flags & SEEN)
108-
prio_queue_put(&queue, p->item);
109+
if (!(p->item->object.flags & SEEN) ||
110+
(p->item->object.flags & COMMON))
111+
continue;
112+
113+
p->item->object.flags |= COMMON;
114+
prio_queue_put(&queue, p->item);
109115
}
110116
}
117+
118+
clear_prio_queue(&queue);
111119
}
112120

113121
/*

0 commit comments

Comments
 (0)