This v3 fixes issues raised by Ramsay in
https://public-inbox.org/git/1908832c-8dd0-377e-917b-acb33b002...@ramsayjones.plus.com/
While I was at it I changed a die("Whatevs") to
die("whatevs"). I.e. start with lower-case as discussed in other
places on-list recently.
The range-diff looks scarier than this v2..v3 really is. There's no
code changes aside from the s/0/NULL/ in one place, but marking a
couple of functions as "static" required moving them around, and
removing their entry from the corresponding *.h file.
Ævar Arnfjörð Bjarmason (8):
commit-graph tests: split up corrupt_graph_and_verify()
commit-graph tests: test a graph that's too small
commit-graph: fix segfault on e.g. "git status"
commit-graph: don't early exit(1) on e.g. "git status"
commit-graph: don't pass filename to load_commit_graph_one_fd_st()
commit-graph verify: detect inability to read the graph
commit-graph write: don't die if the existing graph is corrupt
commit-graph: improve & i18n error messages
builtin/commit-graph.c | 23 +++++--
commit-graph.c | 132 +++++++++++++++++++++++++++-------------
commit-graph.h | 4 +-
commit.h | 6 ++
t/t5318-commit-graph.sh | 42 +++++++++++--
5 files changed, 153 insertions(+), 54 deletions(-)
Range-diff:
1: 2f8ba0adf8 = 1: 83ff92a39d commit-graph tests: split up
corrupt_graph_and_verify()
2: 800b17edde = 2: b9170c35e6 commit-graph tests: test a graph that's too
small
3: 7083ab81c7 ! 3: daf38a9af7 commit-graph: fix segfault on e.g. "git status"
@@ -58,20 +58,10 @@
--- a/commit-graph.c
+++ b/commit-graph.c
@@
- last_chunk_offset = chunk_offset;
- }
-
-+ if (verify_commit_graph_lite(graph))
-+ return NULL;
-+
- return graph;
+ return ret;
}
-@@
- #define GENERATION_ZERO_EXISTS 1
- #define GENERATION_NUMBER_EXISTS 2
-
-+int verify_commit_graph_lite(struct commit_graph *g)
++static int verify_commit_graph_lite(struct commit_graph *g)
+{
+ /*
+ * Basic validation shared between parse_commit_graph()
@@ -101,9 +91,19 @@
+ return 0;
+}
+
- int verify_commit_graph(struct repository *r, struct commit_graph *g)
+ struct commit_graph *parse_commit_graph(void *graph_map, int fd,
+ size_t graph_size)
{
- uint32_t i, cur_fanout_pos = 0;
+@@
+ last_chunk_offset = chunk_offset;
+ }
+
++ if (verify_commit_graph_lite(graph))
++ return NULL;
++
+ return graph;
+ }
+
@@
return 1;
}
@@ -122,18 +122,6 @@
return verify_commit_graph_error;
- diff --git a/commit-graph.h b/commit-graph.h
- --- a/commit-graph.h
- +++ b/commit-graph.h
-@@
- struct string_list *commit_hex,
- int append, int report_progress);
-
-+int verify_commit_graph_lite(struct commit_graph *g);
- int verify_commit_graph(struct repository *r, struct commit_graph *g);
-
- void close_commit_graph(struct repository *);
-
diff --git a/t/t5318-commit-graph.sh b/t/t5318-commit-graph.sh
--- a/t/t5318-commit-graph.sh
+++ b/t/t5318-commit-graph.sh
4: d00564ae89 ! 4: 3d7d8c4deb commit-graph: don't early exit(1) on e.g. "git
status"
@@ -29,7 +29,15 @@
passed to that function for the the "graph file %s is too small" error
message.
+ This leaves load_commit_graph_one() unused by everything except the
+ internal prepare_commit_graph_one() function, so let's mark it as
+ "static". If someone needs it in the future we can remove the "static"
+ attribute. I could also rewrite its sole remaining
+ user ("prepare_commit_graph_one()") to use
+ load_commit_graph_one_fd_st() instead, but let's leave it at this.
+
Signed-off-by: Ævar Arnfjörð Bjarmason <ava...@gmail.com>
+ Signed-off-by: Ramsay Jones <ram...@ramsayjones.plus.com>
diff --git a/builtin/commit-graph.c b/builtin/commit-graph.c
--- a/builtin/commit-graph.c
@@ -131,7 +139,7 @@
close(fd);
- die(_("graph file %s is too small"), graph_file);
+ error(_("graph file %s is too small"), graph_file);
-+ return 0;
++ return NULL;
}
graph_map = xmmap(NULL, graph_size, PROT_READ, MAP_PRIVATE, fd, 0);
ret = parse_commit_graph(graph_map, fd, graph_size);
@@ -143,9 +151,11 @@
}
return ret;
+@@
+ return graph;
}
-+struct commit_graph *load_commit_graph_one(const char *graph_file)
++static struct commit_graph *load_commit_graph_one(const char *graph_file)
+{
+
+ struct stat st;
@@ -158,9 +168,9 @@
+ return load_commit_graph_one_fd_st(graph_file, fd, &st);
+}
+
- struct commit_graph *parse_commit_graph(void *graph_map, int fd,
- size_t graph_size)
+ static void prepare_commit_graph_one(struct repository *r, const char
*obj_dir)
{
+ char *graph_name;
diff --git a/commit-graph.h b/commit-graph.h
--- a/commit-graph.h
@@ -174,9 +184,10 @@
/*
* Given a commit struct, try to fill the commit struct info, including:
@@
+ const unsigned char *chunk_extra_edges;
};
- struct commit_graph *load_commit_graph_one(const char *graph_file);
+-struct commit_graph *load_commit_graph_one(const char *graph_file);
+struct commit_graph *load_commit_graph_one_fd_st(const char *graph_file,
+ int fd, struct stat *st);
5: 25ee185bf7 ! 5: 15687fb21f commit-graph: don't pass filename to
load_commit_graph_one_fd_st()
@@ -59,7 +59,7 @@
close(fd);
- error(_("graph file %s is too small"), graph_file);
+ error(_("commit-graph file is too small"));
- return 0;
+ return NULL;
}
graph_map = xmmap(NULL, graph_size, PROT_READ, MAP_PRIVATE, fd, 0);
@@
@@ -70,15 +70,15 @@
+ return load_commit_graph_one_fd_st(fd, &st);
}
- struct commit_graph *parse_commit_graph(void *graph_map, int fd,
+ static void prepare_commit_graph_one(struct repository *r, const char
*obj_dir)
diff --git a/commit-graph.h b/commit-graph.h
--- a/commit-graph.h
+++ b/commit-graph.h
@@
+ const unsigned char *chunk_extra_edges;
};
- struct commit_graph *load_commit_graph_one(const char *graph_file);
-struct commit_graph *load_commit_graph_one_fd_st(const char *graph_file,
- int fd, struct stat *st);
+struct commit_graph *load_commit_graph_one_fd_st(int fd, struct stat *st);
6: 7619b46987 = 6: 30145c2dca commit-graph verify: detect inability to read
the graph
7: 17ee4fc050 ! 7: 51e3aa651b commit-graph write: don't die if the existing
graph is corrupt
@@ -57,7 +57,7 @@
int config_value;
+ if (git_env_bool(GIT_TEST_COMMIT_GRAPH_DIE_ON_LOAD, 0))
-+ die("Dying as requested by the '%s' variable on commit-graph
load!",
++ die("dying as requested by the '%s' variable on commit-graph
load!",
+ GIT_TEST_COMMIT_GRAPH_DIE_ON_LOAD);
+
if (r->objects->commit_graph_attempted)
8: 29ab2895b7 = 8: e7c801f73b commit-graph: improve & i18n error messages
--
2.21.0.360.g471c308f928
