git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Patrick Steinhardt <ps@pks.im>
To: git@vger.kernel.org
Cc: Taylor Blau <me@ttaylorr.com>, Derrick Stolee <stolee@gmail.com>,
	 Oswald Buddenhagen <oswald.buddenhagen@gmx.de>,
	 Junio C Hamano <gitster@pobox.com>
Subject: [PATCH v2 07/10] commit-graph: refactor `parse_commit_graph()` to take a repository
Date: Wed, 06 Aug 2025 14:00:12 +0200	[thread overview]
Message-ID: <20250806-b4-pks-commit-graph-wo-the-repository-v2-7-911bae638e61@pks.im> (raw)
In-Reply-To: <20250806-b4-pks-commit-graph-wo-the-repository-v2-0-911bae638e61@pks.im>

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>
---
 commit-graph.c               | 23 ++++++++++++-----------
 commit-graph.h               |  2 +-
 oss-fuzz/fuzz-commit-graph.c |  6 ++----
 3 files changed, 15 insertions(+), 16 deletions(-)

diff --git a/commit-graph.c b/commit-graph.c
index 590a6972d2a..50391cc0f32 100644
--- a/commit-graph.c
+++ b/commit-graph.c
@@ -270,9 +270,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
@@ -372,7 +371,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;
@@ -384,7 +383,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;
@@ -404,22 +403,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);
@@ -450,7 +449,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,
@@ -461,7 +462,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,
@@ -478,7 +479,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 f20d28ff3a0..5a5c876af0b 100644
--- 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 fbb77fec197..fb8b8787a46 100644
--- 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);
 

-- 
2.51.0.rc0.215.g125493bb4a.dirty


  parent reply	other threads:[~2025-08-06 12:00 UTC|newest]

Thread overview: 69+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-08-04  8:17 [PATCH 0/9] commit-graph: remove reliance on global state Patrick Steinhardt
2025-08-04  8:17 ` [PATCH 1/9] trace2: introduce function to trace unsigned integers Patrick Steinhardt
2025-08-04 21:33   ` Taylor Blau
2025-08-04  8:17 ` [PATCH 2/9] commit-graph: stop using signed integers to count bloom filters Patrick Steinhardt
2025-08-04  9:13   ` Oswald Buddenhagen
2025-08-04 11:18     ` Patrick Steinhardt
2025-08-04 18:34       ` Junio C Hamano
2025-08-04 21:44         ` Taylor Blau
2025-08-06  6:23           ` Patrick Steinhardt
2025-08-06 12:54             ` Oswald Buddenhagen
2025-08-06 19:04               ` Junio C Hamano
2025-08-06 15:41             ` Junio C Hamano
2025-08-07  7:04               ` Patrick Steinhardt
2025-08-07 22:41                 ` Junio C Hamano
2025-08-11  8:05                   ` Patrick Steinhardt
2025-08-05 15:13         ` Junio C Hamano
2025-08-04 21:42   ` Taylor Blau
2025-08-04  8:17 ` [PATCH 3/9] commit-graph: fix type for some write options Patrick Steinhardt
2025-08-04 21:52   ` Taylor Blau
2025-08-04  8:17 ` [PATCH 4/9] commit-graph: fix sign comparison warnings Patrick Steinhardt
2025-08-04 22:04   ` Taylor Blau
2025-08-06  6:52     ` Patrick Steinhardt
2025-08-04  8:17 ` [PATCH 5/9] commit-graph: stop using `the_hash_algo` via macros Patrick Steinhardt
2025-08-04 22:05   ` Taylor Blau
2025-08-04  8:17 ` [PATCH 6/9] commit-graph: store the hash algorithm instead of its length Patrick Steinhardt
2025-08-04 22:07   ` Taylor Blau
2025-08-04  8:17 ` [PATCH 7/9] commit-graph: stop using `the_hash_algo` Patrick Steinhardt
2025-08-04 22:10   ` Taylor Blau
2025-08-06  6:53     ` Patrick Steinhardt
2025-08-04  8:17 ` [PATCH 8/9] commit-graph: stop using `the_repository` Patrick Steinhardt
2025-08-04 22:11   ` Taylor Blau
2025-08-04  8:17 ` [PATCH 9/9] commit-graph: stop passing in redundant repository Patrick Steinhardt
2025-08-05  4:27 ` [PATCH 0/9] commit-graph: remove reliance on global state Derrick Stolee
2025-08-06  6:53   ` Patrick Steinhardt
2025-08-06 12:00 ` [PATCH v2 00/10] " Patrick Steinhardt
2025-08-06 12:00   ` [PATCH v2 01/10] trace2: introduce function to trace unsigned integers Patrick Steinhardt
2025-08-06 12:00   ` [PATCH v2 02/10] commit-graph: stop using signed integers to count Bloom filters Patrick Steinhardt
2025-08-06 12:00   ` [PATCH v2 03/10] commit-graph: fix type for some write options Patrick Steinhardt
2025-08-06 12:34     ` Oswald Buddenhagen
2025-08-06 15:40       ` Junio C Hamano
2025-08-07  7:07         ` Patrick Steinhardt
2025-08-06 12:00   ` [PATCH v2 04/10] commit-graph: fix sign comparison warnings Patrick Steinhardt
2025-08-06 12:00   ` [PATCH v2 05/10] commit-graph: stop using `the_hash_algo` via macros Patrick Steinhardt
2025-08-06 12:00   ` [PATCH v2 06/10] commit-graph: store the hash algorithm instead of its length Patrick Steinhardt
2025-08-06 12:00   ` Patrick Steinhardt [this message]
2025-08-06 12:00   ` [PATCH v2 08/10] commit-graph: stop using `the_hash_algo` Patrick Steinhardt
2025-08-06 12:00   ` [PATCH v2 09/10] commit-graph: stop using `the_repository` Patrick Steinhardt
2025-08-06 12:00   ` [PATCH v2 10/10] commit-graph: stop passing in redundant repository Patrick Steinhardt
2025-08-07  8:04 ` [PATCH v3 00/10] commit-graph: remove reliance on global state Patrick Steinhardt
2025-08-07  8:04   ` [PATCH v3 01/10] trace2: introduce function to trace unsigned integers Patrick Steinhardt
2025-08-07  8:04   ` [PATCH v3 02/10] commit-graph: stop using signed integers to count Bloom filters Patrick Steinhardt
2025-08-07  8:04   ` [PATCH v3 03/10] commit-graph: fix type for some write options Patrick Steinhardt
2025-08-07 22:40     ` Junio C Hamano
2025-08-11  8:24       ` Patrick Steinhardt
2025-08-07  8:04   ` [PATCH v3 04/10] commit-graph: fix sign comparison warnings Patrick Steinhardt
2025-08-07  8:04   ` [PATCH v3 05/10] commit-graph: stop using `the_hash_algo` via macros Patrick Steinhardt
2025-08-07  8:04   ` [PATCH v3 06/10] commit-graph: store the hash algorithm instead of its length Patrick Steinhardt
2025-08-07  8:04   ` [PATCH v3 07/10] commit-graph: refactor `parse_commit_graph()` to take a repository Patrick Steinhardt
2025-08-07  8:04   ` [PATCH v3 08/10] commit-graph: stop using `the_hash_algo` Patrick Steinhardt
2025-08-07  8:04   ` [PATCH v3 09/10] commit-graph: stop using `the_repository` Patrick Steinhardt
2025-08-07  8:04   ` [PATCH v3 10/10] commit-graph: stop passing in redundant repository Patrick Steinhardt
2025-08-15  5:49 ` [PATCH v4 0/6] commit-graph: remove reliance on global state Patrick Steinhardt
2025-08-15  5:49   ` [PATCH v4 1/6] commit-graph: stop using `the_hash_algo` via macros Patrick Steinhardt
2025-08-15  5:49   ` [PATCH v4 2/6] commit-graph: store the hash algorithm instead of its length Patrick Steinhardt
2025-08-15  5:49   ` [PATCH v4 3/6] commit-graph: refactor `parse_commit_graph()` to take a repository Patrick Steinhardt
2025-08-15  5:49   ` [PATCH v4 4/6] commit-graph: stop using `the_hash_algo` Patrick Steinhardt
2025-08-15  5:49   ` [PATCH v4 5/6] commit-graph: stop using `the_repository` Patrick Steinhardt
2025-08-15  5:49   ` [PATCH v4 6/6] commit-graph: stop passing in redundant repository Patrick Steinhardt
2025-08-15 15:17   ` [PATCH v4 0/6] commit-graph: remove reliance on global state Derrick Stolee

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20250806-b4-pks-commit-graph-wo-the-repository-v2-7-911bae638e61@pks.im \
    --to=ps@pks.im \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=me@ttaylorr.com \
    --cc=oswald.buddenhagen@gmx.de \
    --cc=stolee@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).