@matheusbsbs / matheus-bagliano / commits / 4d5fb9377b

revision: make handle_dotdot() interface less confusing

There are two very subtle bits to the way we parse ".." (and "...") range operators: 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). Instead of munging the buffer, let's instead make a temporary copy of the left-hand side of the range operator. That avoids any const violations, and lets us pass around the parsed elements independently: the left-hand side, the right-hand side, the number of dots (via the "symmetric" flag), and the original full string for error messages. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jeff King committed Mar 26, 2026 at 15:04 UTC 4d5fb9377bba1f45a940e10b0b7354fe7db2b301

1 file changed +19 -23

revision.c

+19 -23

@@ -2038,41 +2038,32 @@ static void prepare_show_merge(struct rev_info *revs)

2038 free(prune);

2039 }

2040

2041 -static int dotdot_missing(const char *arg, char *dotdot,

2041 +static int dotdot_missing(const char *full_name,

2042 struct rev_info *revs, int symmetric)

2043 {

2044 if (revs->ignore_missing)

2045 return 0;

2046 - /* de-munge so we report the full argument */

2047 - *dotdot = '.';

2046 die(symmetric

2047 ? "Invalid symmetric difference expression %s"

2050 - : "Invalid revision range %s", arg);

2048 + : "Invalid revision range %s", full_name);

2049 }

2050

2053 -static int handle_dotdot_1(const char *arg, char *dotdot,

2051 +static int handle_dotdot_1(const char *a_name, const char *b_name,

2052 + const char *full_name, int symmetric,

2053 struct rev_info *revs, int flags,

2054 int cant_be_filename,

2055 struct object_context *a_oc,

2056 struct object_context *b_oc)

2057 {

2059 - const char *a_name, *b_name;

2058 struct object_id a_oid, b_oid;

2059 struct object *a_obj, *b_obj;

2060 unsigned int a_flags, b_flags;

2063 - int symmetric = 0;

2061 unsigned int flags_exclude = flags ^ (UNINTERESTING | BOTTOM);

2062 unsigned int oc_flags = GET_OID_COMMITTISH | GET_OID_RECORD_PATH;

2063

2067 - a_name = arg;

2064 if (!*a_name)

2065 a_name = "HEAD";

2066

2071 - b_name = dotdot + 2;

2072 - if (*b_name == '.') {

2073 - symmetric = 1;

2074 - b_name++;

2075 - }

2067 if (!*b_name)

2068 b_name = "HEAD";

2069

@@ -2081,15 +2072,13 @@ static int handle_dotdot_1(const char *arg, char *dotdot,

2072 return -1;

2073

2074 if (!cant_be_filename) {

2084 - *dotdot = '.';

2085 - verify_non_filename(revs->prefix, arg);

2086 - *dotdot = '\0';

2075 + verify_non_filename(revs->prefix, full_name);

2076 }

2077

2078 a_obj = parse_object(revs->repo, &a_oid);

2079 b_obj = parse_object(revs->repo, &b_oid);

2080 if (!a_obj || !b_obj)

2092 - return dotdot_missing(arg, dotdot, revs, symmetric);

2081 + return dotdot_missing(full_name, revs, symmetric);

2082

2083 if (!symmetric) {

2084 /* just A..B */

@@ -2103,7 +2092,7 @@ static int handle_dotdot_1(const char *arg, char *dotdot,

2092 a = lookup_commit_reference(revs->repo, &a_obj->oid);

2093 b = lookup_commit_reference(revs->repo, &b_obj->oid);

2094 if (!a || !b)

2106 - return dotdot_missing(arg, dotdot, revs, symmetric);

2095 + return dotdot_missing(full_name, revs, symmetric);

2096

2097 if (repo_get_merge_bases(the_repository, a, b, &exclude) < 0) {

2098 commit_list_free(exclude);

@@ -2132,16 +2121,23 @@ static int handle_dotdot(const char *arg,

2121 int cant_be_filename)

2122 {

2123 struct object_context a_oc = {0}, b_oc = {0};

2135 - char *dotdot = strstr(arg, "..");

2124 + const char *dotdot = strstr(arg, "..");

2125 + char *tmp;

2126 + int symmetric = 0;

2127 int ret;

2128

2129 if (!dotdot)

2130 return -1;

2131

2141 - *dotdot = '\0';

2142 - ret = handle_dotdot_1(arg, dotdot, revs, flags, cant_be_filename,

2143 - &a_oc, &b_oc);

2144 - *dotdot = '.';

2132 + tmp = xmemdupz(arg, dotdot - arg);

2133 + dotdot += 2;

2134 + if (*dotdot == '.') {

2135 + symmetric = 1;

2136 + dotdot++;

2137 + }

2138 + ret = handle_dotdot_1(tmp, dotdot, arg, symmetric, revs, flags,

2139 + cant_be_filename, &a_oc, &b_oc);

2140 + free(tmp);

2141

2142 object_context_release(&a_oc);

2143 object_context_release(&b_oc);