[1/3] commit-graph: pass a 'struct repository *' in more places

Commit Message

Taylor Blau June 30, 2020, 5:17 p.m. UTC

In a future commit, some commit-graph internals will want access to
'r->settings', but we only have the 'struct object_directory *'
corresponding to that repository.

Add an additional parameter to pass the repository around in more
places. In the next patch, we will remove the object directory (and
instead reference it with 'r->odb').

Signed-off-by: Taylor Blau <[e-mail]>
---
 builtin/commit-graph.c |  2 +-
 commit-graph.c         | 13 ++++++++-----
 commit-graph.h         |  4 +++-
 fuzz-commit-graph.c    |  5 +++--
 4 files changed, 15 insertions(+), 9 deletions(-)

Comments

Derrick Stolee June 30, 2020, 8:52 p.m. UTC | #1

On 6/30/2020 1:17 PM, Taylor Blau wrote:
> In a future commit, some commit-graph internals will want access to
> 'r->settings', but we only have the 'struct object_directory *'
> corresponding to that repository.

It is good to use "struct repository *" more.

> Add an additional parameter to pass the repository around in more
> places. In the next patch, we will remove the object directory (and
> instead reference it with 'r->odb').

And this is a good reason why we need the repository here: we will
need the settings AND odb.

> diff --git a/commit-graph.h b/commit-graph.h
> index 3ba0da1e5f..03d848e168 100644
> --- a/commit-graph.h
> +++ b/commit-graph.h
> @@ -76,10 +76,12 @@ struct commit_graph {
>  };
>  
>  struct commit_graph *load_commit_graph_one_fd_st(int fd, struct stat *st,
> +						 struct repository *r,
>  						 struct object_directory *odb);

I suppose my only nit is that the struct repository pointer is
not always the first parameter. I understand that you have grouped
it where the 'odb' parameter is here, so perhaps that is fine.

Feel free to ignore this nit.

Thanks,
-Stolee

Patch

diff --git a/builtin/commit-graph.c b/builtin/commit-graph.c
index 75455da138..7c0f98531d 100644
--- a/builtin/commit-graph.c
+++ b/builtin/commit-graph.c
@@ -106,7 +106,7 @@  static int graph_verify(int argc, const char **argv)
 	FREE_AND_NULL(graph_name);
 
 	if (open_ok)
-		graph = load_commit_graph_one_fd_st(fd, &st, odb);
+		graph = load_commit_graph_one_fd_st(fd, &st, the_repository, odb);
 	else
 		graph = read_commit_graph_one(the_repository, odb);
 
diff --git a/commit-graph.c b/commit-graph.c
index 2ff042fbf4..fdfb0888f0 100644
--- a/commit-graph.c
+++ b/commit-graph.c
@@ -170,6 +170,7 @@  int open_commit_graph(const char *graph_file, int *fd, struct stat *st)
 }
 
 struct commit_graph *load_commit_graph_one_fd_st(int fd, struct stat *st,
+						 struct repository *r,
 						 struct object_directory *odb)
 {
 	void *graph_map;
@@ -185,7 +186,7 @@  struct commit_graph *load_commit_graph_one_fd_st(int fd, struct stat *st,
 	}
 	graph_map = xmmap(NULL, graph_size, PROT_READ, MAP_PRIVATE, fd, 0);
 	close(fd);
-	ret = parse_commit_graph(graph_map, graph_size);
+	ret = parse_commit_graph(r, graph_map, graph_size);
 
 	if (ret)
 		ret->odb = odb;
@@ -225,7 +226,8 @@  static int verify_commit_graph_lite(struct commit_graph *g)
 	return 0;
 }
 
-struct commit_graph *parse_commit_graph(void *graph_map, size_t graph_size)
+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;
@@ -396,6 +398,7 @@  struct commit_graph *parse_commit_graph(void *graph_map, size_t graph_size)
 }
 
 static struct commit_graph *load_commit_graph_one(const char *graph_file,
+						  struct repository *r,
 						  struct object_directory *odb)
 {
 
@@ -407,7 +410,7 @@  static struct commit_graph *load_commit_graph_one(const char *graph_file,
 	if (!open_ok)
 		return NULL;
 
-	g = load_commit_graph_one_fd_st(fd, &st, odb);
+	g = load_commit_graph_one_fd_st(fd, &st, r, odb);
 
 	if (g)
 		g->filename = xstrdup(graph_file);
@@ -419,7 +422,7 @@  static struct commit_graph *load_commit_graph_v1(struct repository *r,
 						 struct object_directory *odb)
 {
 	char *graph_name = get_commit_graph_filename(odb);
-	struct commit_graph *g = load_commit_graph_one(graph_name, odb);
+	struct commit_graph *g = load_commit_graph_one(graph_name, r, odb);
 	free(graph_name);
 
 	return g;
@@ -500,7 +503,7 @@  static struct commit_graph *load_commit_graph_chain(struct repository *r,
 		valid = 0;
 		for (odb = r->objects->odb; odb; odb = odb->next) {
 			char *graph_name = get_split_graph_filename(odb, line.buf);
-			struct commit_graph *g = load_commit_graph_one(graph_name, odb);
+			struct commit_graph *g = load_commit_graph_one(graph_name, r, odb);
 
 			free(graph_name);
 
diff --git a/commit-graph.h b/commit-graph.h
index 3ba0da1e5f..03d848e168 100644
--- a/commit-graph.h
+++ b/commit-graph.h
@@ -76,10 +76,12 @@  struct commit_graph {
 };
 
 struct commit_graph *load_commit_graph_one_fd_st(int fd, struct stat *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(void *graph_map, size_t graph_size);
+struct commit_graph *parse_commit_graph(struct repository *r,
+					void *graph_map, size_t graph_size);
 
 /*
  * Return 1 if and only if the repository has a commit-graph
diff --git a/fuzz-commit-graph.c b/fuzz-commit-graph.c
index 430817214d..e7cf6d5b0f 100644
--- a/fuzz-commit-graph.c
+++ b/fuzz-commit-graph.c
@@ -1,7 +1,8 @@ 
 #include "commit-graph.h"
 #include "repository.h"
 
-struct commit_graph *parse_commit_graph(void *graph_map, size_t graph_size);
+struct commit_graph *parse_commit_graph(struct repository *r,
+					void *graph_map, size_t graph_size);
 
 int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size);
 
@@ -10,7 +11,7 @@  int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size)
 	struct commit_graph *g;
 
 	initialize_the_repository();
-	g = parse_commit_graph((void *)data, size);
+	g = parse_commit_graph(the_repository, (void *)data, size);
 	repo_clear(the_repository);
 	free_commit_graph(g);