From ad2dd5bb63b6c2da0091d63463e29ae27e47a893 Mon Sep 17 00:00:00 2001 From: Taylor Blau Date: Mon, 3 Feb 2020 13:18:02 -0800 Subject: commit-graph.c: remove path normalization, comparison As of the previous patch, all calls to 'commit-graph.c' functions which perform path normalization (for e.g., 'get_commit_graph_filename()') are of the form 'ctx->odb->path', which is always in normalized form. Now that there are no callers passing non-normalized paths to these functions, ensure that future callers are bound by the same restrictions by making these functions take a 'struct object_directory *' instead of a 'const char *'. To match, replace all calls with arguments of the form 'ctx->odb->path' with 'ctx->odb' To recover the path, functions that perform path manipulation simply use 'odb->path'. Further, avoid string comparisons with arguments of the form 'odb->path', and instead prefer raw pointer comparisons, which accomplish the same effect, but are far less brittle. This has a pleasant side-effect of making these functions much more robust to paths that cannot be normalized by 'normalize_path_copy()', i.e., because they are outside of the current working directory. For example, prior to this patch, Valgrind reports that the following uninitialized memory read [1]: $ ( cd t && GIT_DIR=../.git valgrind git rev-parse HEAD^ ) because 'normalize_path_copy()' can't normalize '../.git' (since it's relative to but above of the current working directory) [2]. By using a 'struct object_directory *' directly, 'get_commit_graph_filename()' does not need to normalize, because all paths are relative to the current working directory since they are always read from the '->path' of an object directory. [1]: https://lore.kernel.org/git/20191027042116.GA5801@sigill.intra.peff.net. [2]: The bug here is that 'get_commit_graph_filename()' returns the result of 'normalize_path_copy()' without checking the return value. Signed-off-by: Taylor Blau Signed-off-by: Junio C Hamano --- t/helper/test-read-graph.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 't/helper') diff --git a/t/helper/test-read-graph.c b/t/helper/test-read-graph.c index d2884efe0a..2c2f65f06c 100644 --- a/t/helper/test-read-graph.c +++ b/t/helper/test-read-graph.c @@ -11,12 +11,12 @@ int cmd__read_graph(int argc, const char **argv) int open_ok; int fd; struct stat st; - const char *object_dir; + struct object_directory *odb; setup_git_directory(); - object_dir = get_object_directory(); + odb = the_repository->objects->odb; - graph_name = get_commit_graph_filename(object_dir); + graph_name = get_commit_graph_filename(odb); open_ok = open_commit_graph(graph_name, &fd, &st); if (!open_ok) -- cgit v1.2.3 From a7df60cac834cb7f91c3463205c2b6ba5e694200 Mon Sep 17 00:00:00 2001 From: Taylor Blau Date: Mon, 3 Feb 2020 13:18:04 -0800 Subject: commit-graph.h: use odb in 'load_commit_graph_one_fd_st' Apply a similar treatment as in the previous patch to pass a 'struct object_directory *' through the 'load_commit_graph_one_fd_st' initializer, too. This prevents a potential bug where a pointer comparison is made to a NULL 'g->odb', which would cause the commit-graph machinery to think that a pair of commit-graphs belonged to different alternates when in fact they do not (i.e., in the case of no '--object-dir'). Signed-off-by: Taylor Blau Signed-off-by: Junio C Hamano --- t/helper/test-read-graph.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 't/helper') diff --git a/t/helper/test-read-graph.c b/t/helper/test-read-graph.c index 2c2f65f06c..f8a461767c 100644 --- a/t/helper/test-read-graph.c +++ b/t/helper/test-read-graph.c @@ -22,7 +22,7 @@ int cmd__read_graph(int argc, const char **argv) if (!open_ok) die_errno(_("Could not open commit-graph '%s'"), graph_name); - graph = load_commit_graph_one_fd_st(fd, &st); + graph = load_commit_graph_one_fd_st(fd, &st, odb); if (!graph) return 1; -- cgit v1.2.3