Jeff King wrote:
> As an aside, this is one of those places where C's string functions do
> gross things with const.
Yes, yuck.
The fundamental grossness is that argv is semantically char **
(assuming this doesn't segfault) but passed around as const char **.
I wonder why we don't use the same trick in git_extract_argv0_path and
when handling STRIP_EXTENSION in git.c.
Alternatively, try_difference could create a new buffer for its own
use. Originally (v0.99~273, 2005-06-13) this dotdot handling code was
pretty simple:
for (i = 1; i < argc; i++) {
char *arg = argv[i];
...
dotdot = strstr(arg, "..");
if (dotdot) {
unsigned char end[20];
char *n = dotdot+2;
*dotdot = 0;
if (!get_sha1(arg, sha1)) {
if (!*n)
n = "HEAD";
if (!get_sha1(n, end)) {
printf("%s\n", sha1_to_hex(end));
printf("^%s\n", sha1_to_hex(sha1));
continue;
}
}
*dotdot = '.';
}
printf("%s\n", arg);
}
The 'continue' without making the argument right again was justified
because we were done with the argument.
Now it is still not much more complicated --- the function is
logically just trying to parse the argument into a "this" and "next"
part and print them.
Something like the following (untested). Or if the allocation churn
is too much, it could use a static strbuf.
diff --git i/builtin/rev-parse.c w/builtin/rev-parse.c
index c76b89d..98262b3 100644
--- i/builtin/rev-parse.c
+++ w/builtin/rev-parse.c
@@ -234,37 +234,35 @@ static int try_difference(const char *arg)
unsigned char sha1[20];
unsigned char end[20];
const char *next;
- const char *this;
+ struct strbuf this = STRBUF_INIT;
int symmetric;
- static const char head_by_default[] = "HEAD";
if (!(dotdot = strstr(arg, "..")))
return 0;
next = dotdot + 2;
- this = arg;
symmetric = (*next == '.');
-
- *dotdot = 0;
next += symmetric;
- if (!*next)
- next = head_by_default;
- if (dotdot == arg)
- this = head_by_default;
-
- if (this == head_by_default && next == head_by_default &&
- !symmetric) {
+ if (!*next && dotdot == arg && !symmetric)
/*
* Just ".."? That is not a range but the
* pathspec for the parent directory.
*/
- *dotdot = '.';
return 0;
+
+ strbuf_add(&this, arg, dotdot - arg);
+
+ if (!*next)
+ next = "HEAD";
+ if (dotdot == arg) {
+ strbuf_reset(&this);
+ strbuf_addstr(&this, "HEAD");
}
- if (!get_sha1_committish(this, sha1) && !get_sha1_committish(next,
end)) {
+ if (!get_sha1_committish(this.buf, sha1)
+ && !get_sha1_committish(next, end)) {
show_rev(NORMAL, end, next);
- show_rev(symmetric ? NORMAL : REVERSED, sha1, this);
+ show_rev(symmetric ? NORMAL : REVERSED, sha1, this.buf);
if (symmetric) {
struct commit_list *exclude;
struct commit *a, *b;
@@ -279,9 +277,10 @@ static int try_difference(const char *arg)
exclude = n;
}
}
+ strbuf_release(&this);
return 1;
}
- *dotdot = '.';
+ strbuf_release(&this);
return 0;
}
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
