diff options
| author | Taylor Blau <me@ttaylorr.com> | 2022-07-14 14:43:06 -0700 |
|---|---|---|
| committer | Junio C Hamano <gitster@pobox.com> | 2022-07-14 15:42:17 -0700 |
| commit | a92d8523cef66d46e24cd5ef2f01ef97dc4ab239 (patch) | |
| tree | 22e9f374000184d9eb40af63d1bbab896c5d59e0 | |
| parent | 8168d5e9c23ed44ae3d604f392320d66556453c9 (diff) | |
| download | git-a92d8523cef66d46e24cd5ef2f01ef97dc4ab239.tar.gz | |
commit-graph: pass repo_settings instead of repository
The parse_commit_graph() function takes a 'struct repository *' pointer,
but it only ever accesses config settings (either directly or through
the .settings field of the repo struct). Move all relevant config
settings into the repo_settings struct, and update parse_commit_graph()
and its existing callers so that it takes 'struct repo_settings *'
instead.
Callers of parse_commit_graph() will now need to call
prepare_repo_settings() themselves, or initialize a 'struct
repo_settings' directly.
Prior to ab14d0676c (commit-graph: pass a 'struct repository *' in more
places, 2020-09-09), parsing a commit-graph was a pure function
depending only on the contents of the commit-graph itself. Commit
ab14d0676c introduced a dependency on a `struct repository` pointer, and
later commits such as b66d84756f (commit-graph: respect
'commitGraph.readChangedPaths', 2020-09-09) added dependencies on config
settings, which were accessed through the `settings` field of the
repository pointer. This field was initialized via a call to
`prepare_repo_settings()`.
Additionally, this fixes an issue in fuzz-commit-graph: In 44c7e62
(2021-12-06, repo-settings:prepare_repo_settings only in git repos),
prepare_repo_settings was changed to issue a BUG() if it is called by a
process whose CWD is not a Git repository.
The combination of commits mentioned above broke fuzz-commit-graph,
which attempts to parse arbitrary fuzzing-engine-provided bytes as a
commit graph file. Prior to this change, parse_commit_graph() called
prepare_repo_settings(), but since we run the fuzz tests without a valid
repository, we are hitting the BUG() from 44c7e62 for every test case.
Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Josh Steadmon <steadmon@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
| -rw-r--r-- | commit-graph.c | 11 | ||||
| -rw-r--r-- | commit-graph.h | 7 | ||||
| -rw-r--r-- | fuzz-commit-graph.c | 12 | ||||
| -rw-r--r-- | repo-settings.c | 12 | ||||
| -rw-r--r-- | repository.h | 1 |
5 files changed, 33 insertions, 10 deletions
diff --git a/commit-graph.c b/commit-graph.c index 92d4503336..5a6280290f 100644 --- a/commit-graph.c +++ b/commit-graph.c @@ -252,7 +252,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); - ret = parse_commit_graph(r, graph_map, graph_size); + prepare_repo_settings(r); + ret = parse_commit_graph(&r->settings, graph_map, graph_size); if (ret) ret->odb = odb; @@ -321,7 +322,7 @@ static int graph_read_bloom_data(const unsigned char *chunk_start, return 0; } -struct commit_graph *parse_commit_graph(struct repository *r, +struct commit_graph *parse_commit_graph(struct repo_settings *s, void *graph_map, size_t graph_size) { const unsigned char *data; @@ -359,8 +360,6 @@ struct commit_graph *parse_commit_graph(struct repository *r, return NULL; } - prepare_repo_settings(r); - graph = alloc_commit_graph(); graph->hash_len = the_hash_algo->rawsz; @@ -390,7 +389,7 @@ struct commit_graph *parse_commit_graph(struct repository *r, pair_chunk(cf, GRAPH_CHUNKID_EXTRAEDGES, &graph->chunk_extra_edges); pair_chunk(cf, GRAPH_CHUNKID_BASE, &graph->chunk_base_graphs); - if (get_configured_generation_version(r) >= 2) { + if (s->commit_graph_generation_version >= 2) { pair_chunk(cf, GRAPH_CHUNKID_GENERATION_DATA, &graph->chunk_generation_data); pair_chunk(cf, GRAPH_CHUNKID_GENERATION_DATA_OVERFLOW, @@ -400,7 +399,7 @@ struct commit_graph *parse_commit_graph(struct repository *r, graph->read_generation_data = 1; } - if (r->settings.commit_graph_read_changed_paths) { + if (s->commit_graph_read_changed_paths) { pair_chunk(cf, GRAPH_CHUNKID_BLOOMINDEXES, &graph->chunk_bloom_indexes); read_chunk(cf, GRAPH_CHUNKID_BLOOMDATA, diff --git a/commit-graph.h b/commit-graph.h index 2e3ac35237..25cdec127a 100644 --- a/commit-graph.h +++ b/commit-graph.h @@ -93,7 +93,12 @@ struct commit_graph *load_commit_graph_one_fd_st(struct repository *r, struct object_directory *odb); struct commit_graph *read_commit_graph_one(struct repository *r, struct object_directory *odb); -struct commit_graph *parse_commit_graph(struct repository *r, + +/* + * 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, void *graph_map, size_t graph_size); /* diff --git a/fuzz-commit-graph.c b/fuzz-commit-graph.c index e7cf6d5b0f..914026f5d8 100644 --- a/fuzz-commit-graph.c +++ b/fuzz-commit-graph.c @@ -1,7 +1,7 @@ #include "commit-graph.h" #include "repository.h" -struct commit_graph *parse_commit_graph(struct repository *r, +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); @@ -11,7 +11,15 @@ int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) struct commit_graph *g; initialize_the_repository(); - g = parse_commit_graph(the_repository, (void *)data, size); + /* + * Initialize the_repository with commit-graph settings that would + * normally be read from the repository's gitdir. We want to avoid + * touching the disk to keep the individual fuzz-test cases as fast as + * possible. + */ + the_repository->settings.commit_graph_generation_version = 2; + the_repository->settings.commit_graph_read_changed_paths = 1; + g = parse_commit_graph(&the_repository->settings, (void *)data, size); repo_clear(the_repository); free_commit_graph(g); diff --git a/repo-settings.c b/repo-settings.c index 2dfcb2b654..43bc881bfc 100644 --- a/repo-settings.c +++ b/repo-settings.c @@ -11,6 +11,13 @@ static void repo_cfg_bool(struct repository *r, const char *key, int *dest, *dest = def; } +static void repo_cfg_int(struct repository *r, const char *key, int *dest, + int def) +{ + if (repo_config_get_int(r, key, dest)) + *dest = def; +} + void prepare_repo_settings(struct repository *r) { int experimental; @@ -42,11 +49,14 @@ void prepare_repo_settings(struct repository *r) r->settings.core_untracked_cache = UNTRACKED_CACHE_WRITE; } - /* Boolean config or default, does not cascade (simple) */ + /* Commit graph config or default, does not cascade (simple) */ repo_cfg_bool(r, "core.commitgraph", &r->settings.core_commit_graph, 1); + repo_cfg_int(r, "commitgraph.generationversion", &r->settings.commit_graph_generation_version, 2); repo_cfg_bool(r, "commitgraph.readchangedpaths", &r->settings.commit_graph_read_changed_paths, 1); repo_cfg_bool(r, "gc.writecommitgraph", &r->settings.gc_write_commit_graph, 1); repo_cfg_bool(r, "fetch.writecommitgraph", &r->settings.fetch_write_commit_graph, 0); + + /* Boolean config or default, does not cascade (simple) */ repo_cfg_bool(r, "pack.usesparse", &r->settings.pack_use_sparse, 1); repo_cfg_bool(r, "core.multipackindex", &r->settings.core_multi_pack_index, 1); repo_cfg_bool(r, "index.sparse", &r->settings.sparse_index, 0); diff --git a/repository.h b/repository.h index 6cc661e5a4..797f471cce 100644 --- a/repository.h +++ b/repository.h @@ -30,6 +30,7 @@ struct repo_settings { int initialized; int core_commit_graph; + int commit_graph_generation_version; int commit_graph_read_changed_paths; int gc_write_commit_graph; int fetch_write_commit_graph; |