commit-graph: refactor `parse_commit_graph()` to take a repository

Refactor `parse_commit_graph()` so that it takes a repository instead of
taking repository settings. On the one hand this allows us to get rid of
instances where we access `the_hash_algo` by using the repository's hash
algorithm instead. On the other hand it also allows us to move the call
of `prepare_repo_settings()` into the function itself.

Note that there's one small catch, as the commit-graph fuzzer calls this
function directly without having a fully functional repository at hand.
And while the fuzzer already initializes `the_repository` with relevant
info, the call to `prepare_repo_settings()` would fail because we don't
have a fully-initialized repository.

Work around the issue by also settings `settings.initialized` to pretend
that we've already read the settings.

While at it, remove the redundant `parse_commit_graph()` declaration in
the fuzzer. It was added together with aa658574bf (commit-graph, fuzz:
add fuzzer for commit-graph, 2019-01-15), but as we also declared the
same function in "commit-graph.h" it wasn't ever needed.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/commit-graph.c b/commit-graph.c
index 6cdaff26c2a1836c34c825db23f97c9f61bb091b..55a8ce073985315ce172e59673e98f6792aa4b28 100644 (file)
--- a/commit-graph.c
+++ b/commit-graph.c
@@ -272,9 +272,8 @@ struct commit_graph *load_commit_graph_one_fd_st(struct repository *r,
        }
        graph_map = xmmap(NULL, graph_size, PROT_READ, MAP_PRIVATE, fd, 0);
        close(fd);
-       prepare_repo_settings(r);
-       ret = parse_commit_graph(&r->settings, graph_map, graph_size);
 
+       ret = parse_commit_graph(r, graph_map, graph_size);
        if (ret)
                ret->odb_source = source;
        else
@@ -374,7 +373,7 @@ static int graph_read_bloom_data(const unsigned char *chunk_start,
        return 0;
 }
 
-struct commit_graph *parse_commit_graph(struct repo_settings *s,
+struct commit_graph *parse_commit_graph(struct repository *r,
                                        void *graph_map, size_t graph_size)
 {
        const unsigned char *data;
@@ -386,7 +385,7 @@ struct commit_graph *parse_commit_graph(struct repo_settings *s,
        if (!graph_map)
                return NULL;
 
-       if (graph_size < graph_min_size(the_hash_algo))
+       if (graph_size < graph_min_size(r->hash_algo))
                return NULL;
 
        data = (const unsigned char *)graph_map;
@@ -406,22 +405,22 @@ struct commit_graph *parse_commit_graph(struct repo_settings *s,
        }
 
        hash_version = *(unsigned char*)(data + 5);
-       if (hash_version != oid_version(the_hash_algo)) {
+       if (hash_version != oid_version(r->hash_algo)) {
                error(_("commit-graph hash version %X does not match version %X"),
-                     hash_version, oid_version(the_hash_algo));
+                     hash_version, oid_version(r->hash_algo));
                return NULL;
        }
 
        graph = alloc_commit_graph();
 
-       graph->hash_algo = the_hash_algo;
+       graph->hash_algo = r->hash_algo;
        graph->num_chunks = *(unsigned char*)(data + 6);
        graph->data = graph_map;
        graph->data_len = graph_size;
 
        if (graph_size < GRAPH_HEADER_SIZE +
                         (graph->num_chunks + 1) * CHUNK_TOC_ENTRY_SIZE +
-                        GRAPH_FANOUT_SIZE + the_hash_algo->rawsz) {
+                        GRAPH_FANOUT_SIZE + r->hash_algo->rawsz) {
                error(_("commit-graph file is too small to hold %u chunks"),
                      graph->num_chunks);
                free(graph);
@@ -452,7 +451,9 @@ struct commit_graph *parse_commit_graph(struct repo_settings *s,
        pair_chunk(cf, GRAPH_CHUNKID_BASE, &graph->chunk_base_graphs,
                   &graph->chunk_base_graphs_size);
 
-       if (s->commit_graph_generation_version >= 2) {
+       prepare_repo_settings(r);
+
+       if (r->settings.commit_graph_generation_version >= 2) {
                read_chunk(cf, GRAPH_CHUNKID_GENERATION_DATA,
                           graph_read_generation_data, graph);
                pair_chunk(cf, GRAPH_CHUNKID_GENERATION_DATA_OVERFLOW,
@@ -463,7 +464,7 @@ struct commit_graph *parse_commit_graph(struct repo_settings *s,
                        graph->read_generation_data = 1;
        }
 
-       if (s->commit_graph_changed_paths_version) {
+       if (r->settings.commit_graph_changed_paths_version) {
                read_chunk(cf, GRAPH_CHUNKID_BLOOMINDEXES,
                           graph_read_bloom_index, graph);
                read_chunk(cf, GRAPH_CHUNKID_BLOOMDATA,
@@ -480,7 +481,7 @@ struct commit_graph *parse_commit_graph(struct repo_settings *s,
        }
 
        oidread(&graph->oid, graph->data + graph->data_len - graph->hash_algo->rawsz,
-               the_repository->hash_algo);
+               r->hash_algo);
 
        free_chunkfile(cf);
        return graph;

diff --git a/commit-graph.h b/commit-graph.h
index 7dc1f2b22bd657d1ee8f3235ff6cb5c17baffb34..7bbc69989ce605c66a5a44c9279b862c7b1452cc 100644 (file)
--- a/commit-graph.h
+++ b/commit-graph.h
@@ -128,7 +128,7 @@ struct repo_settings;
  * Callers should initialize the repo_settings with prepare_repo_settings()
  * prior to calling parse_commit_graph().
  */
-struct commit_graph *parse_commit_graph(struct repo_settings *s,
+struct commit_graph *parse_commit_graph(struct repository *r,
                                        void *graph_map, size_t graph_size);
 
 /*

diff --git a/oss-fuzz/fuzz-commit-graph.c b/oss-fuzz/fuzz-commit-graph.c
index fbb77fec197721dca3cbe28c7acc4f52698a549e..fb8b8787a460f196d87455e072a498583fc1ba13 100644 (file)
--- a/oss-fuzz/fuzz-commit-graph.c
+++ b/oss-fuzz/fuzz-commit-graph.c
@@ -4,9 +4,6 @@
 #include "commit-graph.h"
 #include "repository.h"
 
-struct commit_graph *parse_commit_graph(struct repo_settings *s,
-                                       void *graph_map, size_t graph_size);
-
 int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size);
 
 int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size)
@@ -22,9 +19,10 @@ int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size)
         * possible.
         */
        repo_set_hash_algo(the_repository, GIT_HASH_SHA1);
+       the_repository->settings.initialized = 1;
        the_repository->settings.commit_graph_generation_version = 2;
        the_repository->settings.commit_graph_changed_paths_version = 1;
-       g = parse_commit_graph(&the_repository->settings, (void *)data, size);
+       g = parse_commit_graph(the_repository, (void *)data, size);
        repo_clear(the_repository);
        free_commit_graph(g);