List for cgit developers and users
 help / color / mirror / Atom feed
From: Jason at zx2c4.com (Jason A. Donenfeld)
Subject: [PATCH] global: use proper accessors for maybe_tree
Date: Thu, 12 Mar 2020 21:01:18 -0600	[thread overview]
Message-ID: <20200313030118.833911-1-Jason@zx2c4.com> (raw)

A previous commit changed ->tree to ->maybe_tree throughout, which may
have worked at the time, but wasn't safe, because maybe_tree is loaded
lazily. This manifested itself in crashes when using the "follow" log
feature. The proper fix is to use the correct contextual accessors
everytime we want access to maybe_tree. Thankfully, the commit.cocci
script takes care of creating mostly-correct patches that we could then
fix up, resulting in this commit here.

Fixes: 255b78f ("git: update to v2.18.0")
Cc: Christian Hesse <mail at eworm.de>
Signed-off-by: Jason A. Donenfeld <Jason at zx2c4.com>
---
 ui-blame.c  |  6 ++++--
 ui-blob.c   | 17 +++++++++++------
 ui-commit.c |  2 +-
 ui-diff.c   |  4 ++--
 ui-log.c    |  4 ++--
 ui-plain.c  |  7 ++++---
 ui-tree.c   |  8 +++++---
 7 files changed, 29 insertions(+), 19 deletions(-)

diff --git a/ui-blame.c b/ui-blame.c
index 644c30a..f28eea0 100644
--- a/ui-blame.c
+++ b/ui-blame.c
@@ -290,8 +290,10 @@ void cgit_print_blame(void)
 	walk_tree_ctx.match_baselen = (path_items.match) ?
 				       basedir_len(path_items.match) : -1;
 
-	read_tree_recursive(the_repository, commit->maybe_tree, "", 0, 0,
-		&paths, walk_tree, &walk_tree_ctx);
+	read_tree_recursive(the_repository,
+			    repo_get_commit_tree(the_repository, commit),
+			    "", 0, 0,
+			    &paths, walk_tree, &walk_tree_ctx);
 	if (!walk_tree_ctx.state)
 		cgit_print_error_page(404, "Not found", "Not found");
 	else if (walk_tree_ctx.state == 2)
diff --git a/ui-blob.c b/ui-blob.c
index 30e2d4b..f76c641 100644
--- a/ui-blob.c
+++ b/ui-blob.c
@@ -56,8 +56,9 @@ int cgit_ref_path_exists(const char *path, const char *ref, int file_only)
 		goto done;
 	if (oid_object_info(the_repository, &oid, &size) != OBJ_COMMIT)
 		goto done;
-	read_tree_recursive(the_repository, lookup_commit_reference(the_repository, &oid)->maybe_tree,
-		"", 0, 0, &paths, walk_tree, &walk_tree_ctx);
+	read_tree_recursive(the_repository,
+			    repo_get_commit_tree(the_repository, lookup_commit_reference(the_repository, &oid)),
+			    "", 0, 0, &paths, walk_tree, &walk_tree_ctx);
 
 done:
 	free(path_items.match);
@@ -91,8 +92,10 @@ int cgit_print_file(char *path, const char *head, int file_only)
 	type = oid_object_info(the_repository, &oid, &size);
 	if (type == OBJ_COMMIT) {
 		commit = lookup_commit_reference(the_repository, &oid);
-		read_tree_recursive(the_repository, commit->maybe_tree,
-			"", 0, 0, &paths, walk_tree, &walk_tree_ctx);
+		read_tree_recursive(the_repository,
+				    repo_get_commit_tree(the_repository, commit),
+				    "", 0, 0, &paths, walk_tree,
+				    &walk_tree_ctx);
 		if (!walk_tree_ctx.found_path)
 			return -1;
 		type = oid_object_info(the_repository, &oid, &size);
@@ -148,8 +151,10 @@ void cgit_print_blob(const char *hex, char *path, const char *head, int file_onl
 
 	if ((!hex) && type == OBJ_COMMIT && path) {
 		commit = lookup_commit_reference(the_repository, &oid);
-		read_tree_recursive(the_repository, commit->maybe_tree,
-			"", 0, 0, &paths, walk_tree, &walk_tree_ctx);
+		read_tree_recursive(the_repository,
+				    repo_get_commit_tree(the_repository, commit),
+				    "", 0, 0, &paths, walk_tree,
+				    &walk_tree_ctx);
 		type = oid_object_info(the_repository, &oid, &size);
 	}
 
diff --git a/ui-commit.c b/ui-commit.c
index 9a47b54..783211f 100644
--- a/ui-commit.c
+++ b/ui-commit.c
@@ -78,7 +78,7 @@ void cgit_print_commit(char *hex, const char *prefix)
 	html(")</td></tr>\n");
 	html("<tr><th>tree</th><td colspan='2' class='sha1'>");
 	tmp = xstrdup(hex);
-	cgit_tree_link(oid_to_hex(&commit->maybe_tree->object.oid), NULL, NULL,
+	cgit_tree_link(oid_to_hex(get_commit_tree_oid(commit)), NULL, NULL,
 		       ctx.qry.head, tmp, NULL);
 	if (prefix) {
 		html(" /");
diff --git a/ui-diff.c b/ui-diff.c
index c60aefd..329c350 100644
--- a/ui-diff.c
+++ b/ui-diff.c
@@ -413,7 +413,7 @@ void cgit_print_diff(const char *new_rev, const char *old_rev,
 			"Bad commit: %s", oid_to_hex(new_rev_oid));
 		return;
 	}
-	new_tree_oid = &commit->maybe_tree->object.oid;
+	new_tree_oid = get_commit_tree_oid(commit);
 
 	if (old_rev) {
 		if (get_oid(old_rev, old_rev_oid)) {
@@ -434,7 +434,7 @@ void cgit_print_diff(const char *new_rev, const char *old_rev,
 				"Bad commit: %s", oid_to_hex(old_rev_oid));
 			return;
 		}
-		old_tree_oid = &commit2->maybe_tree->object.oid;
+		old_tree_oid = get_commit_tree_oid(commit2);
 	} else {
 		old_tree_oid = NULL;
 	}
diff --git a/ui-log.c b/ui-log.c
index dc5cb1e..2939c01 100644
--- a/ui-log.c
+++ b/ui-log.c
@@ -153,8 +153,8 @@ static int show_commit(struct commit *commit, struct rev_info *revs)
 	rem_lines = 0;
 
 	revs->diffopt.flags.recursive = 1;
-	diff_tree_oid(&parent->maybe_tree->object.oid,
-		      &commit->maybe_tree->object.oid,
+	diff_tree_oid(get_commit_tree_oid(parent),
+		      get_commit_tree_oid(commit),
 		      "", &revs->diffopt);
 	diffcore_std(&revs->diffopt);
 
diff --git a/ui-plain.c b/ui-plain.c
index b73c1cf..2a7b18c 100644
--- a/ui-plain.c
+++ b/ui-plain.c
@@ -193,13 +193,14 @@ void cgit_print_plain(void)
 	if (!path_items.match) {
 		path_items.match = "";
 		walk_tree_ctx.match_baselen = -1;
-		print_dir(&commit->maybe_tree->object.oid, "", 0, "");
+		print_dir(get_commit_tree_oid(commit), "", 0, "");
 		walk_tree_ctx.match = 2;
 	}
 	else
 		walk_tree_ctx.match_baselen = basedir_len(path_items.match);
-	read_tree_recursive(the_repository, commit->maybe_tree,
-		"", 0, 0, &paths, walk_tree, &walk_tree_ctx);
+	read_tree_recursive(the_repository,
+		            repo_get_commit_tree(the_repository, commit),
+		            "", 0, 0, &paths, walk_tree, &walk_tree_ctx);
 	if (!walk_tree_ctx.match)
 		cgit_print_error_page(404, "Not found", "Not found");
 	else if (walk_tree_ctx.match == 2)
diff --git a/ui-tree.c b/ui-tree.c
index 84eb17d..1e4efb2 100644
--- a/ui-tree.c
+++ b/ui-tree.c
@@ -370,12 +370,14 @@ void cgit_print_tree(const char *rev, char *path)
 	walk_tree_ctx.curr_rev = xstrdup(rev);
 
 	if (path == NULL) {
-		ls_tree(&commit->maybe_tree->object.oid, NULL, &walk_tree_ctx);
+		ls_tree(get_commit_tree_oid(commit), NULL, &walk_tree_ctx);
 		goto cleanup;
 	}
 
-	read_tree_recursive(the_repository, commit->maybe_tree, "", 0, 0,
-		&paths, walk_tree, &walk_tree_ctx);
+	read_tree_recursive(the_repository,
+			    repo_get_commit_tree(the_repository, commit),
+			    "", 0, 0,
+			    &paths, walk_tree, &walk_tree_ctx);
 	if (walk_tree_ctx.state == 1)
 		ls_tail();
 	else if (walk_tree_ctx.state == 2)
-- 
2.25.1



                 reply	other threads:[~2020-03-13  3:01 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

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=20200313030118.833911-1-Jason@zx2c4.com \
    --to=cgit@lists.zx2c4.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).