@matheusbsbs / matheus-bagliano / commits / cce99cd8c6

commit-graph: writing missing parents is a BUG

When writing a commit-graph, we write GRAPH_MISSING_PARENT if the parent's object id does not appear in the list of commits to be written into the commit-graph. This was done as the initial design allowed commits to have missing parents, but the final version requires the commit-graph to be closed under reachability. Thus, this GRAPH_MISSING_PARENT value should never be written. However, there are reasons why it could be written! These range from a bug in the reachable-closure code to a memory error causing the binary search into the list of object ids to fail. In either case, we should fail fast and avoid writing the commit-graph file with bad data. Remove the GRAPH_MISSING_PARENT constant in favor of the constant GRAPH_EDGE_LAST_MASK, which has the same value. Signed-off-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Derrick Stolee committed Dec 19, 2018 at 12:14 UTC cce99cd8c677446820a895e827967ca56cae9a93

1 file changed +11 -6

commit-graph.c

+11 -6

@@ -34,7 +34,6 @@

34 #define GRAPH_OID_LEN GRAPH_OID_LEN_SHA1

36 #define GRAPH_OCTOPUS_EDGES_NEEDED 0x80000000

37 -#define GRAPH_PARENT_MISSING 0x7fffffff

37 #define GRAPH_EDGE_LAST_MASK 0x7fffffff

38 #define GRAPH_PARENT_NONE 0x70000000

@@ -496,7 +495,9 @@ static void write_graph_chunk_data(struct hashfile *f, int hash_len,

495 commit_to_sha1);

496

497 if (edge_value < 0)

499 - edge_value = GRAPH_PARENT_MISSING;

498 + BUG("missing parent %s for commit %s",

499 + oid_to_hex(&parent->item->object.oid),

500 + oid_to_hex(&(*list)->object.oid));

501 }

502

503 hashwrite_be32(f, edge_value);

@@ -514,7 +515,9 @@ static void write_graph_chunk_data(struct hashfile *f, int hash_len,

515 nr_commits,

516 commit_to_sha1);

517 if (edge_value < 0)

517 - edge_value = GRAPH_PARENT_MISSING;

518 + BUG("missing parent %s for commit %s",

519 + oid_to_hex(&parent->item->object.oid),

520 + oid_to_hex(&(*list)->object.oid));

521 }

522

523 hashwrite_be32(f, edge_value);

@@ -567,7 +570,9 @@ static void write_graph_chunk_large_edges(struct hashfile *f,

570 commit_to_sha1);

571

572 if (edge_value < 0)

570 - edge_value = GRAPH_PARENT_MISSING;

573 + BUG("missing parent %s for commit %s",

574 + oid_to_hex(&parent->item->object.oid),

575 + oid_to_hex(&(*list)->object.oid));

576 else if (!parent->next)

577 edge_value |= GRAPH_LAST_EDGE;

578

@@ -864,7 +869,7 @@ void write_commit_graph(const char *obj_dir,

869 count_distinct++;

870 }

871

867 - if (count_distinct >= GRAPH_PARENT_MISSING)

872 + if (count_distinct >= GRAPH_EDGE_LAST_MASK)

873 die(_("the commit graph format cannot write %d commits"), count_distinct);

874

875 commits.nr = 0;

@@ -891,7 +896,7 @@ void write_commit_graph(const char *obj_dir,

896 }

897 num_chunks = num_extra_edges ? 4 : 3;

898

894 - if (commits.nr >= GRAPH_PARENT_MISSING)

899 + if (commits.nr >= GRAPH_EDGE_LAST_MASK)

900 die(_("too many commits to write graph"));

901

902 compute_generation_numbers(&commits, report_progress);