1 file changed, 23 insertions, 71 deletions
diff --git a/source/ui-patch.c b/source/ui-patch.c
index 04083fe..0fa9a2e 100644
--- a/source/ui-patch.c
+++ b/source/ui-patch.c
@@ -10,100 +10,52 @@
 
 #include "cgit.h"
 #include "html.h"
-#include "ui-diff.h"
 #include "ui-patch.h"
 #include "ui-shared.h"
 
 // Room for two hex object ids, the two dots between them, and the null byte.
 #define REV_RANGE_LEN (2 * GIT_MAX_HEXSZ + 3)
 
-// Where the format argument sits in the walk arguments below.
-#define FORMAT_ARG 2
-
-/*
- * A root commit has no parent, so the old end is left null and the caller asks
- * for that one commit instead.
- */
-static int resolve_range(const char *new_rev, const char *old_rev,
-	struct object_id *new_oid, struct object_id *old_oid)
-{
-	struct commit *commit;
-
-	if (repo_get_oid(the_repository, new_rev, new_oid)) {
-		cgit_print_error_page(404, "Not Found", "Bad object id: %s", new_rev);
-		return -1;
-	}
-	commit = lookup_commit_reference(the_repository, new_oid);
-	if (!commit) {
-		cgit_print_error_page(404, "Not Found", "Not a commit: %s", new_rev);
-		return -1;
-	}
-
-	if (old_rev) {
-		if (repo_get_oid(the_repository, old_rev, old_oid)) {
-			cgit_print_error_page(404, "Not Found", "Bad object id: %s", old_rev);
-			return -1;
-		}
-		if (!lookup_commit_reference(the_repository, old_oid)) {
-			cgit_print_error_page(404, "Not Found", "Not a commit: %s", old_rev);
-			return -1;
-		}
-	} else if (commit->parents && commit->parents->item) {
-		oidcpy(old_oid, &commit->parents->item->object.oid);
-	} else {
-		oidclr(old_oid, the_repository->hash_algo);
-	}
-	return 0;
-}
-
 void cgit_print_patch(const char *new_rev, const char *old_rev, const char *prefix)
 {
 	struct rev_info rev;
 	struct commit *commit;
 	struct object_id new_oid, old_oid;
+	struct strvec rev_argv = STRVEC_INIT;
 	char rev_range[REV_RANGE_LEN];
 	const char *version;
-	// setup_revisions reads these the way git reads a command line, so the
-	// first entry stands in for the program name and is skipped, and the
-	// array has to stay null terminated because the path after the double
-	// dash is picked up past the count.
-	const char *rev_argv[] = { NULL, "--reverse", "--format=email", rev_range, "--", prefix, NULL };
-	int rev_argc = ARRAY_SIZE(rev_argv) - 1;
-
-	if (!prefix)
-		rev_argc--;
 
 	if (!new_rev)
 		new_rev = ctx.qry.head;
 
-	// A leading colon makes git read the path as a pathspec with magic,
-	// which can end the request inside git after the headers are out.
-	if (prefix && prefix[0] == ':') {
-		cgit_print_error_page(400, "Bad Request", "Invalid path");
+	if (cgit_refuse_pathspec_magic(prefix))
 		return;
-	}
 
-	if (resolve_range(new_rev, old_rev, &new_oid, &old_oid))
+	commit = cgit_get_commit(new_rev, &new_oid);
+	if (!commit || cgit_get_old_oid(old_rev, commit, &old_oid))
 		return;
 
 	if (is_null_oid(&old_oid)) {
-		memcpy(rev_range, oid_to_hex(&new_oid), the_hash_algo->hexsz + 1);
+		oid_to_hex_r(rev_range, &new_oid);
 	} else {
-		xsnprintf(rev_range, REV_RANGE_LEN, "%s..%s", oid_to_hex(&old_oid), oid_to_hex(&new_oid));
+		xsnprintf(rev_range, sizeof(rev_range), "%s..%s", oid_to_hex(&old_oid), oid_to_hex(&new_oid));
 	}
 
-	if (!ctx.cfg.enable_plain_email) {
-		rev_argv[FORMAT_ARG] =
-			"--format=format:From %H Mon Sep 17 00:00:00 2001%n"
-			"From: %an%nDate: %aD%n%w(78,0,1)Subject: %s%n%n%w(0)%b";
-	}
+	strvec_pushl(&rev_argv, "patch", "--reverse", NULL);
+	if (ctx.cfg.enable_plain_email)
+		strvec_push(&rev_argv, "--format=email");
+	else
+		strvec_push(&rev_argv, "--format=format:From %H Mon Sep 17 00:00:00 2001%n"
+			"From: %an%nDate: %aD%n%w(78,0,1)Subject: %s%n%n%w(0)%b");
+	strvec_pushl(&rev_argv, rev_range, "--", NULL);
+	if (prefix)
+		strvec_push(&rev_argv, prefix);
 
 	// Git inlines a blob up to its own big-file threshold, far past what
 	// any other view inflates, so max-blob-size is applied through it.
 	if (ctx.cfg.max_blob_size)
 		repo_settings_set_big_file_threshold(the_repository, (unsigned long)ctx.cfg.max_blob_size * 1024);
 	repo_init_revisions(the_repository, &rev, NULL);
-	rev.abbrev = DEFAULT_ABBREV;
 	rev.verbose_header = 1;
 	rev.diff = 1;
 	rev.show_root_diff = 1;
@@ -113,17 +65,16 @@ void cgit_print_patch(const char *new_rev, const char *old_rev, const char *pref
 	// the path comes from the request and a long one would abort the
 	// process.
 	if (prefix)
-		rev.diffopt.stat_sep = cgit_fmtalloc("(limited to '%s')\n\n", prefix);
-	setup_revisions(rev_argc, rev_argv, &rev, NULL);
+		rev.diffopt.stat_sep = xstrfmt("(limited to '%s')\n\n", prefix);
+	setup_revisions_from_strvec(&rev_argv, &rev, NULL);
 	// A single commit resolves to a range starting at its parent, so this
 	// only ever cuts an explicit range short and keeps one request from
 	// emitting a patch for the whole history.
 	if (ctx.cfg.max_patch_count > 0)
 		rev.max_count = ctx.cfg.max_patch_count;
-	// A failed setup leaves the walk holding freed commits, so it must
-	// not be read from, and the headers wait until it has succeeded.
-	if (prepare_revision_walk(&rev)) {
-		cgit_print_error_page(500, "Internal Server Error", "Unable to read the history");
+	// The headers wait until the walk has started.
+	if (cgit_prepare_walk(&rev)) {
+		strvec_clear(&rev_argv);
 		return;
 	}
 
@@ -132,8 +83,7 @@ void cgit_print_patch(const char *new_rev, const char *old_rev, const char *pref
 	cgit_print_http_headers();
 
 	// git format-patch signs with its bare version number, so the v that
-	// cgit's tags carry is left off to read the same way. Without
-	// enable-version-info the signature names cgit alone.
+	// cgit's tags carry is left off to read the same way.
 	if (!skip_prefix(cgit_version, "v", &version))
 		version = cgit_version;
 
@@ -147,4 +97,6 @@ void cgit_print_patch(const char *new_rev, const char *old_rev, const char *pref
 		else
 			printf("-- \ncgit\n\n");
 	}
+	release_revisions(&rev);
+	strvec_clear(&rev_argv);
 }