The append_option function appends a string to a
NULL-terminated array, taking care not to overflow the
array. However, it's comparison was over-cautious, and
checked that there was enough room for two options, not one
(plus the NULL-terminator, of course). This could cause
very long command-lines (e.g., "git gc --aggressive" when
"--unpack-unreachable" is in use) to hit the limit.
Signed-off-by: Jeff King <peff@xxxxxxxx>
---
I noticed this when trying a "git gc --aggressive" with the current
"next". The culprit is my 7e52f56 (gc: do not explode objects which will
be immediately pruned) which adds two extra options to the repack
invocation.
This patch is enough to fix it, as the current limit is big enough when
the off-by-one error is removed. The currentl limit is 10, which
appears to have been pulled out of thin air by 0d7566a (Add --aggressive
option to 'git gc', 2007-05-09) as "big enough".
Obviously this would be much nicer with argv_array. Unfortunately, I
don't think there is a way to have a nice initializer with argv_array.
The current code looks like this:
static const char *argv_repack[MAX_ADD] = {"repack", "-d", "-l", NULL };
and in an ideal world you would have something like:
static struct argv_array argv_repack = ARGV_ARRAY_INIT("repack", "-d", "-l");
Of course that would involve variadic macros, which we can't count on.
But even beyond that it's tough, because you are mixing static
initialization with a dynamic structure. You can hack around that by
copying the defaults during first push, but that's extra complexity, and
the syntax is still ugly.
What do you think of just:
static struct argv_array argv_repack;
[...]
argv_init_from_string(&argv_repack, "repack -d -l");
I prefer static initialization when we can do it, but it just seems like
too much trouble in this case.
builtin/gc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/builtin/gc.c b/builtin/gc.c
index 1bc2fe3..d80a961 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -74,7 +74,7 @@ static void append_option(const char **cmd, const char *opt, int max_length)
for (i = 0; cmd[i]; i++)
;
- if (i + 2 >= max_length)
+ if (i + 1 >= max_length)
die(_("Too many options specified"));
cmd[i++] = opt;
cmd[i] = NULL;
--
1.7.9.6.8.g992e5
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html