[PATCH 1/4] revision: make handle_dotdot() interface less confusing

From

Jeff King <peff@peff.net>

Date

Mar 26, 2026, 19:04 UTC

Message-ID

<20260326190444.GA415796@coredump.intra.peff.net>

In-Reply-To

<20260326190243.GA412983@coredump.intra.peff.net>

 1. In handle_dotdot_1(), we assume that the incoming arguments "dotdot"
    and "arg" are part of the same string, with the first digit of the
    range-operator blanked to a NUL. Then when we want the full name
    (e.g., to report an error), we replace the NUL with a dot to restore
    the original string.

 2. In handle_dotdot(), we take in a const string, but then we modify it
    by overwriting the range operator with a NUL. This has worked OK in
    practice since we tend to pass in buffers that are actually
    writeable (including argv), but segfaults with something like:

      handle_revision_arg("..HEAD", &revs, 0, 0);

    On top of that, building with recent versions of glibc causes the
    compiler to complain, because it notices when we use strchr() or
    strstr() to launder away constness (basically detecting the
    possibility of the segfault above via the type system).

Signed-off-by: Jeff King <peff@peff.net>
---
 revision.c | 42 +++++++++++++++++++-----------------------
 1 file changed, 19 insertions(+), 23 deletions(-)

diff --git a/revision.c b/revision.c
index 31808e3df0..f61262436f 100644
--- a/revision.c
+++ b/revision.c
@@ -2038,41 +2038,32 @@ static void prepare_show_merge(struct rev_info *revs)
 	free(prune);
 }
 
-static int dotdot_missing(const char *arg, char *dotdot,
+static int dotdot_missing(const char *full_name,
 			  struct rev_info *revs, int symmetric)
 {
 	if (revs->ignore_missing)
 		return 0;
-	/* de-munge so we report the full argument */
-	*dotdot = '.';
 	die(symmetric
 	    ? "Invalid symmetric difference expression %s"
-	    : "Invalid revision range %s", arg);
+	    : "Invalid revision range %s", full_name);
 }
 
-static int handle_dotdot_1(const char *arg, char *dotdot,
+static int handle_dotdot_1(const char *a_name, const char *b_name,
+			   const char *full_name, int symmetric,
 			   struct rev_info *revs, int flags,
 			   int cant_be_filename,
 			   struct object_context *a_oc,
 			   struct object_context *b_oc)
 {
-	const char *a_name, *b_name;
 	struct object_id a_oid, b_oid;
 	struct object *a_obj, *b_obj;
 	unsigned int a_flags, b_flags;
-	int symmetric = 0;
 	unsigned int flags_exclude = flags ^ (UNINTERESTING | BOTTOM);
 	unsigned int oc_flags = GET_OID_COMMITTISH | GET_OID_RECORD_PATH;
 
-	a_name = arg;
 	if (!*a_name)
 		a_name = "HEAD";
 
-	b_name = dotdot + 2;
-	if (*b_name == '.') {
-		symmetric = 1;
-		b_name++;
-	}
 	if (!*b_name)
 		b_name = "HEAD";
 
@@ -2081,15 +2072,13 @@ static int handle_dotdot_1(const char *arg, char *dotdot,
 		return -1;
 
 	if (!cant_be_filename) {
-		*dotdot = '.';
-		verify_non_filename(revs->prefix, arg);
-		*dotdot = '\0';
+		verify_non_filename(revs->prefix, full_name);
 	}
 
 	a_obj = parse_object(revs->repo, &a_oid);
 	b_obj = parse_object(revs->repo, &b_oid);
 	if (!a_obj || !b_obj)
-		return dotdot_missing(arg, dotdot, revs, symmetric);
+		return dotdot_missing(full_name, revs, symmetric);
 
 	if (!symmetric) {
 		/* just A..B */
@@ -2103,7 +2092,7 @@ static int handle_dotdot_1(const char *arg, char *dotdot,
 		a = lookup_commit_reference(revs->repo, &a_obj->oid);
 		b = lookup_commit_reference(revs->repo, &b_obj->oid);
 		if (!a || !b)
-			return dotdot_missing(arg, dotdot, revs, symmetric);
+			return dotdot_missing(full_name, revs, symmetric);
 
 		if (repo_get_merge_bases(the_repository, a, b, &exclude) < 0) {
 			commit_list_free(exclude);
@@ -2132,16 +2121,23 @@ static int handle_dotdot(const char *arg,
 			 int cant_be_filename)
 {
 	struct object_context a_oc = {0}, b_oc = {0};
-	char *dotdot = strstr(arg, "..");
+	const char *dotdot = strstr(arg, "..");
+	char *tmp;
+	int symmetric = 0;
 	int ret;
 
 	if (!dotdot)
 		return -1;
 
-	*dotdot = '\0';
-	ret = handle_dotdot_1(arg, dotdot, revs, flags, cant_be_filename,
-			      &a_oc, &b_oc);
-	*dotdot = '.';
+	tmp = xmemdupz(arg, dotdot - arg);
+	dotdot += 2;
+	if (*dotdot == '.') {
+		symmetric = 1;
+		dotdot++;
+	}
+	ret = handle_dotdot_1(tmp, dotdot, arg, symmetric, revs, flags,
+			      cant_be_filename, &a_oc, &b_oc);
+	free(tmp);
 
 	object_context_release(&a_oc);
 	object_context_release(&b_oc);

-- 
2.53.0.1081.gf77a8b8145