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>

diff --git a/revision.c b/revision.c
index 31808e3df055c76df98bcb5303dc0e05fe6042c2..f61262436f5678cdae9afc8ac7b05a97bea7bab1 100644 (file)
--- 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);