From: Derrick Stolee
The can_all_from_reach_with_flag() algorithm was refactored in 4fbcca4e
"commit-reach: make can_all_from_reach... linear" but incorrectly
assumed that all objects provided were commits. During a fetch
negotiation, ok_to_give_up() in upload-pack.c may provide unpeeled tags
to the 'from' array. The current code creates a segfault.
Add a direct call to can_all_from_reach_with_flag() in 'test-tool reach'
and add a test in t6600-test-reach.sh that demonstrates this segfault.
Correct the issue by peeling tags when investigating the initial list
of objects in the 'from' array.
The can_all_from_reach_with_flag() method uses 'assign_flag' as a
value we can use to mark objects temporarily during our commit walk.
The intent is that these flags are removed from all objects before
returning. However, this is not the case.
The 'from' array could also contain objects that are not commits, and
we mark those objects with 'assign_flag'. Add a loop to the 'cleanup'
section that removes these markers.
Also, we forgot to free() the memory for 'list', so add that to the
'cleanup' section. Also, use a cleaner mechanism for clearing those
flags.
Signed-off-by: Jeff King
Signed-off-by: Derrick Stolee
---
commit-reach.c| 44 +--
t/helper/test-reach.c | 22 +-
t/t6600-test-reach.sh | 30 +++--
3 files changed, 79 insertions(+), 17 deletions(-)
diff --git a/commit-reach.c b/commit-reach.c
index 86715c103c..db84f85986 100644
--- a/commit-reach.c
+++ b/commit-reach.c
@@ -544,20 +544,42 @@ 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;
+ if (!from_one || 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;
+ }
+
+ list[nr_commits] = (struct commit *)from_one;
+ if (parse_commit(list[nr_commits]) ||
+ list[nr_commits]->generation < min_generation) {
+ result = 0;
+ goto cleanup;
+ }
+
+ 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;
@@ -600,10 +622,12 @@ int can_all_from_reach_with_flag(struct object_array
*from,
}
cleanup:
- for (i = 0; i < from->nr; i++) {
- clear_commit_marks(list[i], RESULT);
- clear_commit_marks(list[i], assign_flag);
- }
+ clear_commit_marks_many(nr_commits, list, RESULT | assign_flag);
+ free(list);
+
+ for (i = 0; i < from->nr; i++)
+ from->objects[i].item->flags &= ~assign_flag;
+
return result;
}
diff --git a/t/helper/test-reach.c b/t/helper/test-reach.c
index eb21103998..08d2ea68e8 100644
--- a/t/helper/test-reach.c
+++ b/t/helper/test-reach.c
@@ -31,6 +31,7 @@ int cmd__reach(int ac, const char **av)
struct object_id oid_A, oid_B;
struct commit *A, *B;
struct commit_list *X, *Y;
+ struct object_array X_obj = OBJECT_ARRAY_INIT;
struct commit **X_array;
int X_nr, X_alloc;
struct strbuf buf = STRBUF_INIT;
@@ -49,7 +50,8 @@ int cmd__reach(int ac, const char **av)
while (strbuf_getline(, stdin) != EOF) {
struct object_id oid;
- struct object *o;
+ struct object *orig;
+ struct object *peeled;
struct commit *c;
if (buf.len < 3)
continue;
@@ -57,14 +59,14 @@ int cmd__reach(int ac, const char **av)
if (get_oid_committish(buf.buf + 2, ))
die("failed to resolve %s", buf.buf + 2);
- o = parse_object(r, );
-
