commit-graph: use chunk-format read API

Instead of parsing the table of contents directly, use the chunk-format
API methods read_table_of_contents() and pair_chunk(). While the current
implementation loses the duplicate-chunk detection, that will be added
in a future change.

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/commit-graph.c b/commit-graph.c
index a889130cc84997a14e833145d41bef2d5824aa0c..76514a879e888cc32e94866eb563c018a0c947db 100644 (file)
--- a/commit-graph.c
+++ b/commit-graph.c
@@ -59,8 +59,7 @@ void git_test_write_commit_graph_or_die(void)
 
 #define GRAPH_HEADER_SIZE 8
 #define GRAPH_FANOUT_SIZE (4 * 256)
-#define GRAPH_CHUNKLOOKUP_WIDTH 12
-#define GRAPH_MIN_SIZE (GRAPH_HEADER_SIZE + 4 * GRAPH_CHUNKLOOKUP_WIDTH \
+#define GRAPH_MIN_SIZE (GRAPH_HEADER_SIZE + 4 * CHUNK_TOC_ENTRY_SIZE \
                        + GRAPH_FANOUT_SIZE + the_hash_algo->rawsz)
 
 #define CORRECTED_COMMIT_DATE_OFFSET_OVERFLOW (1ULL << 31)
@@ -298,15 +297,43 @@ static int verify_commit_graph_lite(struct commit_graph *g)
        return 0;
 }
 
+static int graph_read_oid_lookup(const unsigned char *chunk_start,
+                                size_t chunk_size, void *data)
+{
+       struct commit_graph *g = data;
+       g->chunk_oid_lookup = chunk_start;
+       g->num_commits = chunk_size / g->hash_len;
+       return 0;
+}
+
+static int graph_read_bloom_data(const unsigned char *chunk_start,
+                                 size_t chunk_size, void *data)
+{
+       struct commit_graph *g = data;
+       uint32_t hash_version;
+       g->chunk_bloom_data = chunk_start;
+       hash_version = get_be32(chunk_start);
+
+       if (hash_version != 1)
+               return 0;
+
+       g->bloom_filter_settings = xmalloc(sizeof(struct bloom_filter_settings));
+       g->bloom_filter_settings->hash_version = hash_version;
+       g->bloom_filter_settings->num_hashes = get_be32(chunk_start + 4);
+       g->bloom_filter_settings->bits_per_entry = get_be32(chunk_start + 8);
+       g->bloom_filter_settings->max_changed_paths = DEFAULT_BLOOM_MAX_CHANGES;
+
+       return 0;
+}
+
 struct commit_graph *parse_commit_graph(struct repository *r,
                                        void *graph_map, size_t graph_size)
 {
-       const unsigned char *data, *chunk_lookup;
-       uint32_t i;
+       const unsigned char *data;
        struct commit_graph *graph;
-       uint64_t next_chunk_offset;
        uint32_t graph_signature;
        unsigned char graph_version, hash_version;
+       struct chunkfile *cf = NULL;
 
        if (!graph_map)
                return NULL;
@@ -347,7 +374,7 @@ struct commit_graph *parse_commit_graph(struct repository *r,
        graph->data_len = graph_size;
 
        if (graph_size < GRAPH_HEADER_SIZE +
-                        (graph->num_chunks + 1) * GRAPH_CHUNKLOOKUP_WIDTH +
+                        (graph->num_chunks + 1) * CHUNK_TOC_ENTRY_SIZE +
                         GRAPH_FANOUT_SIZE + the_hash_algo->rawsz) {
                error(_("commit-graph file is too small to hold %u chunks"),
                      graph->num_chunks);
@@ -355,108 +382,28 @@ struct commit_graph *parse_commit_graph(struct repository *r,
                return NULL;
        }
 
-       chunk_lookup = data + 8;
-       next_chunk_offset = get_be64(chunk_lookup + 4);
-       for (i = 0; i < graph->num_chunks; i++) {
-               uint32_t chunk_id;
-               uint64_t chunk_offset = next_chunk_offset;
-               int chunk_repeated = 0;
-
-               chunk_id = get_be32(chunk_lookup + 0);
-
-               chunk_lookup += GRAPH_CHUNKLOOKUP_WIDTH;
-               next_chunk_offset = get_be64(chunk_lookup + 4);
-
-               if (chunk_offset > graph_size - the_hash_algo->rawsz) {
-                       error(_("commit-graph improper chunk offset %08x%08x"), (uint32_t)(chunk_offset >> 32),
-                             (uint32_t)chunk_offset);
-                       goto free_and_return;
-               }
-
-               switch (chunk_id) {
-               case GRAPH_CHUNKID_OIDFANOUT:
-                       if (graph->chunk_oid_fanout)
-                               chunk_repeated = 1;
-                       else
-                               graph->chunk_oid_fanout = (uint32_t*)(data + chunk_offset);
-                       break;
-
-               case GRAPH_CHUNKID_OIDLOOKUP:
-                       if (graph->chunk_oid_lookup)
-                               chunk_repeated = 1;
-                       else {
-                               graph->chunk_oid_lookup = data + chunk_offset;
-                               graph->num_commits = (next_chunk_offset - chunk_offset)
-                                                    / graph->hash_len;
-                       }
-                       break;
+       cf = init_chunkfile(NULL);
 
-               case GRAPH_CHUNKID_DATA:
-                       if (graph->chunk_commit_data)
-                               chunk_repeated = 1;
-                       else
-                               graph->chunk_commit_data = data + chunk_offset;
-                       break;
-
-               case GRAPH_CHUNKID_GENERATION_DATA:
-                       if (graph->chunk_generation_data)
-                               chunk_repeated = 1;
-                       else
-                               graph->chunk_generation_data = data + chunk_offset;
-                       break;
-
-               case GRAPH_CHUNKID_GENERATION_DATA_OVERFLOW:
-                       if (graph->chunk_generation_data_overflow)
-                               chunk_repeated = 1;
-                       else
-                               graph->chunk_generation_data_overflow = data + chunk_offset;
-                       break;
-
-               case GRAPH_CHUNKID_EXTRAEDGES:
-                       if (graph->chunk_extra_edges)
-                               chunk_repeated = 1;
-                       else
-                               graph->chunk_extra_edges = data + chunk_offset;
-                       break;
-
-               case GRAPH_CHUNKID_BASE:
-                       if (graph->chunk_base_graphs)
-                               chunk_repeated = 1;
-                       else
-                               graph->chunk_base_graphs = data + chunk_offset;
-                       break;
-
-               case GRAPH_CHUNKID_BLOOMINDEXES:
-                       if (graph->chunk_bloom_indexes)
-                               chunk_repeated = 1;
-                       else if (r->settings.commit_graph_read_changed_paths)
-                               graph->chunk_bloom_indexes = data + chunk_offset;
-                       break;
-
-               case GRAPH_CHUNKID_BLOOMDATA:
-                       if (graph->chunk_bloom_data)
-                               chunk_repeated = 1;
-                       else if (r->settings.commit_graph_read_changed_paths) {
-                               uint32_t hash_version;
-                               graph->chunk_bloom_data = data + chunk_offset;
-                               hash_version = get_be32(data + chunk_offset);
-
-                               if (hash_version != 1)
-                                       break;
+       if (read_table_of_contents(cf, graph->data, graph_size,
+                                  GRAPH_HEADER_SIZE, graph->num_chunks))
+               goto free_and_return;
 
-                               graph->bloom_filter_settings = xmalloc(sizeof(struct bloom_filter_settings));
-                               graph->bloom_filter_settings->hash_version = hash_version;
-                               graph->bloom_filter_settings->num_hashes = get_be32(data + chunk_offset + 4);
-                               graph->bloom_filter_settings->bits_per_entry = get_be32(data + chunk_offset + 8);
-                               graph->bloom_filter_settings->max_changed_paths = DEFAULT_BLOOM_MAX_CHANGES;
-                       }
-                       break;
-               }
+       pair_chunk(cf, GRAPH_CHUNKID_OIDFANOUT,
+                  (const unsigned char **)&graph->chunk_oid_fanout);
+       read_chunk(cf, GRAPH_CHUNKID_OIDLOOKUP, graph_read_oid_lookup, graph);
+       pair_chunk(cf, GRAPH_CHUNKID_DATA, &graph->chunk_commit_data);
+       pair_chunk(cf, GRAPH_CHUNKID_EXTRAEDGES, &graph->chunk_extra_edges);
+       pair_chunk(cf, GRAPH_CHUNKID_BASE, &graph->chunk_base_graphs);
+       pair_chunk(cf, GRAPH_CHUNKID_GENERATION_DATA,
+                  &graph->chunk_generation_data);
+       pair_chunk(cf, GRAPH_CHUNKID_GENERATION_DATA_OVERFLOW,
+                  &graph->chunk_generation_data_overflow);
 
-               if (chunk_repeated) {
-                       error(_("commit-graph chunk id %08x appears multiple times"), chunk_id);
-                       goto free_and_return;
-               }
+       if (r->settings.commit_graph_read_changed_paths) {
+               pair_chunk(cf, GRAPH_CHUNKID_BLOOMINDEXES,
+                          &graph->chunk_bloom_indexes);
+               read_chunk(cf, GRAPH_CHUNKID_BLOOMDATA,
+                          graph_read_bloom_data, graph);
        }
 
        if (graph->chunk_bloom_indexes && graph->chunk_bloom_data) {
@@ -473,9 +420,11 @@ struct commit_graph *parse_commit_graph(struct repository *r,
        if (verify_commit_graph_lite(graph))
                goto free_and_return;
 
+       free_chunkfile(cf);
        return graph;
 
 free_and_return:
+       free_chunkfile(cf);
        free(graph->bloom_filter_settings);
        free(graph);
        return NULL;

diff --git a/t/t5318-commit-graph.sh b/t/t5318-commit-graph.sh
index fa27df579a579a02255a316c788051ea11980827..c7da741284e587f003195a2e249ce4028282d816 100755 (executable)
--- a/t/t5318-commit-graph.sh
+++ b/t/t5318-commit-graph.sh
@@ -564,7 +564,7 @@ test_expect_success 'detect bad hash version' '
 
 test_expect_success 'detect low chunk count' '
        corrupt_graph_and_verify $GRAPH_BYTE_CHUNK_COUNT "\01" \
-               "missing the .* chunk"
+               "final chunk has non-zero id"
 '
 
 test_expect_success 'detect missing OID fanout chunk' '