freshlybakedca.ke / git
Git fork
fork atom

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>

authored by

Patrick Steinhardt and committed by
Junio C Hamano f1141b43 e45402bb

+15 -16
unified split
+12 -11
commit-graph.c
··· 272 } 273 graph_map = xmmap(NULL, graph_size, PROT_READ, MAP_PRIVATE, fd, 0); 274 close(fd); 275 - prepare_repo_settings(r); 276 - ret = parse_commit_graph(&r->settings, graph_map, graph_size); 277 278 if (ret) 279 ret->odb_source = source; 280 else ··· 374 return 0; 375 } 376 377 - struct commit_graph *parse_commit_graph(struct repo_settings *s, 378 void *graph_map, size_t graph_size) 379 { 380 const unsigned char *data; ··· 386 if (!graph_map) 387 return NULL; 388 389 - if (graph_size < graph_min_size(the_hash_algo)) 390 return NULL; 391 392 data = (const unsigned char *)graph_map; ··· 406 } 407 408 hash_version = *(unsigned char*)(data + 5); 409 - if (hash_version != oid_version(the_hash_algo)) { 410 error(_("commit-graph hash version %X does not match version %X"), 411 - hash_version, oid_version(the_hash_algo)); 412 return NULL; 413 } 414 415 graph = alloc_commit_graph(); 416 417 - graph->hash_algo = the_hash_algo; 418 graph->num_chunks = *(unsigned char*)(data + 6); 419 graph->data = graph_map; 420 graph->data_len = graph_size; 421 422 if (graph_size < GRAPH_HEADER_SIZE + 423 (graph->num_chunks + 1) * CHUNK_TOC_ENTRY_SIZE + 424 - GRAPH_FANOUT_SIZE + the_hash_algo->rawsz) { 425 error(_("commit-graph file is too small to hold %u chunks"), 426 graph->num_chunks); 427 free(graph); ··· 452 pair_chunk(cf, GRAPH_CHUNKID_BASE, &graph->chunk_base_graphs, 453 &graph->chunk_base_graphs_size); 454 455 - if (s->commit_graph_generation_version >= 2) { 456 read_chunk(cf, GRAPH_CHUNKID_GENERATION_DATA, 457 graph_read_generation_data, graph); 458 pair_chunk(cf, GRAPH_CHUNKID_GENERATION_DATA_OVERFLOW, ··· 463 graph->read_generation_data = 1; 464 } 465 466 - if (s->commit_graph_changed_paths_version) { 467 read_chunk(cf, GRAPH_CHUNKID_BLOOMINDEXES, 468 graph_read_bloom_index, graph); 469 read_chunk(cf, GRAPH_CHUNKID_BLOOMDATA, ··· 480 } 481 482 oidread(&graph->oid, graph->data + graph->data_len - graph->hash_algo->rawsz, 483 - the_repository->hash_algo); 484 485 free_chunkfile(cf); 486 return graph;
··· 272 } 273 graph_map = xmmap(NULL, graph_size, PROT_READ, MAP_PRIVATE, fd, 0); 274 close(fd); 275 276 + ret = parse_commit_graph(r, graph_map, graph_size); 277 if (ret) 278 ret->odb_source = source; 279 else ··· 373 return 0; 374 } 375 376 + struct commit_graph *parse_commit_graph(struct repository *r, 377 void *graph_map, size_t graph_size) 378 { 379 const unsigned char *data; ··· 385 if (!graph_map) 386 return NULL; 387 388 + if (graph_size < graph_min_size(r->hash_algo)) 389 return NULL; 390 391 data = (const unsigned char *)graph_map; ··· 405 } 406 407 hash_version = *(unsigned char*)(data + 5); 408 + if (hash_version != oid_version(r->hash_algo)) { 409 error(_("commit-graph hash version %X does not match version %X"), 410 + hash_version, oid_version(r->hash_algo)); 411 return NULL; 412 } 413 414 graph = alloc_commit_graph(); 415 416 + graph->hash_algo = r->hash_algo; 417 graph->num_chunks = *(unsigned char*)(data + 6); 418 graph->data = graph_map; 419 graph->data_len = graph_size; 420 421 if (graph_size < GRAPH_HEADER_SIZE + 422 (graph->num_chunks + 1) * CHUNK_TOC_ENTRY_SIZE + 423 + GRAPH_FANOUT_SIZE + r->hash_algo->rawsz) { 424 error(_("commit-graph file is too small to hold %u chunks"), 425 graph->num_chunks); 426 free(graph); ··· 451 pair_chunk(cf, GRAPH_CHUNKID_BASE, &graph->chunk_base_graphs, 452 &graph->chunk_base_graphs_size); 453 454 + prepare_repo_settings(r); 455 + 456 + if (r->settings.commit_graph_generation_version >= 2) { 457 read_chunk(cf, GRAPH_CHUNKID_GENERATION_DATA, 458 graph_read_generation_data, graph); 459 pair_chunk(cf, GRAPH_CHUNKID_GENERATION_DATA_OVERFLOW, ··· 464 graph->read_generation_data = 1; 465 } 466 467 + if (r->settings.commit_graph_changed_paths_version) { 468 read_chunk(cf, GRAPH_CHUNKID_BLOOMINDEXES, 469 graph_read_bloom_index, graph); 470 read_chunk(cf, GRAPH_CHUNKID_BLOOMDATA, ··· 481 } 482 483 oidread(&graph->oid, graph->data + graph->data_len - graph->hash_algo->rawsz, 484 + r->hash_algo); 485 486 free_chunkfile(cf); 487 return graph;
+1 -1
commit-graph.h
··· 128 * Callers should initialize the repo_settings with prepare_repo_settings() 129 * prior to calling parse_commit_graph(). 130 */ 131 - struct commit_graph *parse_commit_graph(struct repo_settings *s, 132 void *graph_map, size_t graph_size); 133 134 /*
··· 128 * Callers should initialize the repo_settings with prepare_repo_settings() 129 * prior to calling parse_commit_graph(). 130 */ 131 + struct commit_graph *parse_commit_graph(struct repository *r, 132 void *graph_map, size_t graph_size); 133 134 /*
+2 -4
oss-fuzz/fuzz-commit-graph.c
··· 4 #include "commit-graph.h" 5 #include "repository.h" 6 7 - struct commit_graph *parse_commit_graph(struct repo_settings *s, 8 - void *graph_map, size_t graph_size); 9 - 10 int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size); 11 12 int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) ··· 22 * possible. 23 */ 24 repo_set_hash_algo(the_repository, GIT_HASH_SHA1); 25 the_repository->settings.commit_graph_generation_version = 2; 26 the_repository->settings.commit_graph_changed_paths_version = 1; 27 - g = parse_commit_graph(&the_repository->settings, (void *)data, size); 28 repo_clear(the_repository); 29 free_commit_graph(g); 30
··· 4 #include "commit-graph.h" 5 #include "repository.h" 6 7 int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size); 8 9 int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) ··· 19 * possible. 20 */ 21 repo_set_hash_algo(the_repository, GIT_HASH_SHA1); 22 + the_repository->settings.initialized = 1; 23 the_repository->settings.commit_graph_generation_version = 2; 24 the_repository->settings.commit_graph_changed_paths_version = 1; 25 + g = parse_commit_graph(the_repository, (void *)data, size); 26 repo_clear(the_repository); 27 free_commit_graph(g); 28