Skip to content

Commit

Permalink
commit-graph: writing missing parents is a BUG
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
derrickstolee committed Dec 17, 2018
1 parent 0d0ac38 commit 907a24d
Showing 1 changed file with 11 additions and 6 deletions.
17 changes: 11 additions & 6 deletions commit-graph.c
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@
#define GRAPH_OID_LEN GRAPH_OID_LEN_SHA1

#define GRAPH_OCTOPUS_EDGES_NEEDED 0x80000000
#define GRAPH_PARENT_MISSING 0x7fffffff
#define GRAPH_EDGE_LAST_MASK 0x7fffffff
#define GRAPH_PARENT_NONE 0x70000000

Expand Down Expand Up @@ -496,7 +495,9 @@ static void write_graph_chunk_data(struct hashfile *f, int hash_len,
commit_to_sha1);

if (edge_value < 0)
edge_value = GRAPH_PARENT_MISSING;
BUG("missing parent %s for commit %s",
oid_to_hex(&parent->item->object.oid),
oid_to_hex(&(*list)->object.oid));
}

hashwrite_be32(f, edge_value);
Expand All @@ -514,7 +515,9 @@ static void write_graph_chunk_data(struct hashfile *f, int hash_len,
nr_commits,
commit_to_sha1);
if (edge_value < 0)
edge_value = GRAPH_PARENT_MISSING;
BUG("missing parent %s for commit %s",
oid_to_hex(&parent->item->object.oid),
oid_to_hex(&(*list)->object.oid));
}

hashwrite_be32(f, edge_value);
Expand Down Expand Up @@ -567,7 +570,9 @@ static void write_graph_chunk_large_edges(struct hashfile *f,
commit_to_sha1);

if (edge_value < 0)
edge_value = GRAPH_PARENT_MISSING;
BUG("missing parent %s for commit %s",
oid_to_hex(&parent->item->object.oid),
oid_to_hex(&(*list)->object.oid));
else if (!parent->next)
edge_value |= GRAPH_LAST_EDGE;

Expand Down Expand Up @@ -864,7 +869,7 @@ void write_commit_graph(const char *obj_dir,
count_distinct++;
}

if (count_distinct >= GRAPH_PARENT_MISSING)
if (count_distinct >= GRAPH_EDGE_LAST_MASK)
die(_("the commit graph format cannot write %d commits"), count_distinct);

commits.nr = 0;
Expand All @@ -891,7 +896,7 @@ void write_commit_graph(const char *obj_dir,
}
num_chunks = num_extra_edges ? 4 : 3;

if (commits.nr >= GRAPH_PARENT_MISSING)
if (commits.nr >= GRAPH_EDGE_LAST_MASK)
die(_("too many commits to write graph"));

compute_generation_numbers(&commits, report_progress);
Expand Down

0 comments on commit 907a24d

Please sign in to comment.