gc: use argv-array for sub-commands

git-gc executes many sub-commands. The argument list for some of these is constant, but for others we add more arguments at runtime. The latter is implemented by allocating a constant extra number of NULLs, and either using a custom append function, or just referencing unused slots by number. As of commit 7e52f56, which added two new arguments, it is possible to exceed the constant number of slots for "repack" by running "git gc --aggressive", causing "git gc" to die. This patch converts all of the static argv lists to use argv-array. In addition to fixing the overflow caused by 7e52f56, it has a few advantages: 1. We can drop the custom append function (which, incidentally, had an off-by-one error exacerbating the static limit). 2. We can drop the ugly magic numbers used when adding arguments to "prune". 3. Adding further arguments will be easier; you can just add new "push" calls without worrying about increasing any static limits. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jeff King committed Apr 18, 2012 at 14:10 UTC 234587fc87b156dc20461fc61353beeb904b43bc
1 file changed +33 -45
builtin/gc.c
+33 -45
@@ -14,6 +14,7 @@
14 #include "cache.h"
15 #include "parse-options.h"
16 #include "run-command.h"
17 +#include "argv-array.h"
18
19 #define FAILED_RUN "failed to run %s"
20
@@ -28,12 +29,11 @@ static int gc_auto_threshold = 6700;
29 static int gc_auto_pack_limit = 50;
30 static const char *prune_expire = "2.weeks.ago";
31
31 -#define MAX_ADD 10
32 -static const char *argv_pack_refs[] = {"pack-refs", "--all", "--prune", NULL};
33 -static const char *argv_reflog[] = {"reflog", "expire", "--all", NULL};
34 -static const char *argv_repack[MAX_ADD] = {"repack", "-d", "-l", NULL};
35 -static const char *argv_prune[] = {"prune", "--expire", NULL, NULL, NULL};
36 -static const char *argv_rerere[] = {"rerere", "gc", NULL};
32 +static struct argv_array pack_refs_cmd = ARGV_ARRAY_INIT;
33 +static struct argv_array reflog = ARGV_ARRAY_INIT;
34 +static struct argv_array repack = ARGV_ARRAY_INIT;
35 +static struct argv_array prune = ARGV_ARRAY_INIT;
36 +static struct argv_array rerere = ARGV_ARRAY_INIT;
37
38 static int gc_config(const char *var, const char *value, void *cb)
39 {
@@ -67,19 +67,6 @@ static int gc_config(const char *var, const char *value, void *cb)
67 return git_default_config(var, value, cb);
68 }
69
70 -static void append_option(const char **cmd, const char *opt, int max_length)
71 -{
72 - int i;
73 -
74 - for (i = 0; cmd[i]; i++)
75 - ;
76 -
77 - if (i + 2 >= max_length)
78 - die(_("Too many options specified"));
79 - cmd[i++] = opt;
80 - cmd[i] = NULL;
81 -}
82 -
70 static int too_many_loose_objects(void)
71 {
72 /*
@@ -147,13 +134,11 @@ static int too_many_packs(void)
134 static void add_repack_all_option(void)
135 {
136 if (prune_expire && !strcmp(prune_expire, "now"))
150 - append_option(argv_repack, "-a", MAX_ADD);
137 + argv_array_push(&repack, "-a");
138 else {
152 - append_option(argv_repack, "-A", MAX_ADD);
153 - if (prune_expire) {
154 - append_option(argv_repack, "--unpack-unreachable", MAX_ADD);
155 - append_option(argv_repack, prune_expire, MAX_ADD);
156 - }
139 + argv_array_push(&repack, "-A");
140 + if (prune_expire)
141 + argv_array_pushf(&repack, "--unpack-unreachable=%s", prune_expire);
142 }
143 }
144
@@ -187,7 +172,6 @@ int cmd_gc(int argc, const char **argv, const char *prefix)
172 int aggressive = 0;
173 int auto_gc = 0;
174 int quiet = 0;
190 - char buf[80];
175
176 struct option builtin_gc_options[] = {
177 OPT__QUIET(&quiet, "suppress progress reporting"),
@@ -202,6 +186,12 @@ int cmd_gc(int argc, const char **argv, const char *prefix)
186 if (argc == 2 && !strcmp(argv[1], "-h"))
187 usage_with_options(builtin_gc_usage, builtin_gc_options);
188
189 + argv_array_pushl(&pack_refs_cmd, "pack-refs", "--all", "--prune", NULL);
190 + argv_array_pushl(&reflog, "reflog", "expire", "--all", NULL);
191 + argv_array_pushl(&repack, "repack", "-d", "-l", NULL);
192 + argv_array_pushl(&prune, "prune", "--expire", NULL );
193 + argv_array_pushl(&rerere, "rerere", "gc", NULL);
194 +
195 git_config(gc_config, NULL);
196
197 if (pack_refs < 0)
@@ -213,15 +203,13 @@ int cmd_gc(int argc, const char **argv, const char *prefix)
203 usage_with_options(builtin_gc_usage, builtin_gc_options);
204
205 if (aggressive) {
216 - append_option(argv_repack, "-f", MAX_ADD);
217 - append_option(argv_repack, "--depth=250", MAX_ADD);
218 - if (aggressive_window > 0) {
219 - sprintf(buf, "--window=%d", aggressive_window);
220 - append_option(argv_repack, buf, MAX_ADD);
221 - }
206 + argv_array_push(&repack, "-f");
207 + argv_array_push(&repack, "--depth=250");
208 + if (aggressive_window > 0)
209 + argv_array_pushf(&repack, "--window=%d", aggressive_window);
210 }
211 if (quiet)
224 - append_option(argv_repack, "-q", MAX_ADD);
212 + argv_array_push(&repack, "-q");
213
214 if (auto_gc) {
215 /*
@@ -239,25 +227,25 @@ int cmd_gc(int argc, const char **argv, const char *prefix)
227 } else
228 add_repack_all_option();
229
242 - if (pack_refs && run_command_v_opt(argv_pack_refs, RUN_GIT_CMD))
243 - return error(FAILED_RUN, argv_pack_refs[0]);
230 + if (pack_refs && run_command_v_opt(pack_refs_cmd.argv, RUN_GIT_CMD))
231 + return error(FAILED_RUN, pack_refs_cmd.argv[0]);
232
245 - if (run_command_v_opt(argv_reflog, RUN_GIT_CMD))
246 - return error(FAILED_RUN, argv_reflog[0]);
233 + if (run_command_v_opt(reflog.argv, RUN_GIT_CMD))
234 + return error(FAILED_RUN, reflog.argv[0]);
235
248 - if (run_command_v_opt(argv_repack, RUN_GIT_CMD))
249 - return error(FAILED_RUN, argv_repack[0]);
236 + if (run_command_v_opt(repack.argv, RUN_GIT_CMD))
237 + return error(FAILED_RUN, repack.argv[0]);
238
239 if (prune_expire) {
252 - argv_prune[2] = prune_expire;
240 + argv_array_push(&prune, prune_expire);
241 if (quiet)
254 - argv_prune[3] = "--no-progress";
255 - if (run_command_v_opt(argv_prune, RUN_GIT_CMD))
256 - return error(FAILED_RUN, argv_prune[0]);
242 + argv_array_push(&prune, "--no-progress");
243 + if (run_command_v_opt(prune.argv, RUN_GIT_CMD))
244 + return error(FAILED_RUN, prune.argv[0]);
245 }
246
259 - if (run_command_v_opt(argv_rerere, RUN_GIT_CMD))
260 - return error(FAILED_RUN, argv_rerere[0]);
247 + if (run_command_v_opt(rerere.argv, RUN_GIT_CMD))
248 + return error(FAILED_RUN, rerere.argv[0]);
249
250 if (auto_gc && too_many_loose_objects())
251 warning(_("There are too many unreachable loose objects; "