From 234587fc87b156dc20461fc61353beeb904b43bc Mon Sep 17 00:00:00 2001 From: Jeff King Date: Wed, 18 Apr 2012 14:10:19 -0700 Subject: [PATCH] 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 Signed-off-by: Junio C Hamano --- builtin/gc.c | 78 ++++++++++++++++++++++------------------------------ 1 file changed, 33 insertions(+), 45 deletions(-) diff --git a/builtin/gc.c b/builtin/gc.c index 1bc2fe336e..9b4232c8f3 100644 --- a/builtin/gc.c +++ b/builtin/gc.c @@ -14,6 +14,7 @@ #include "cache.h" #include "parse-options.h" #include "run-command.h" +#include "argv-array.h" #define FAILED_RUN "failed to run %s" @@ -28,12 +29,11 @@ static int gc_auto_threshold = 6700; static int gc_auto_pack_limit = 50; static const char *prune_expire = "2.weeks.ago"; -#define MAX_ADD 10 -static const char *argv_pack_refs[] = {"pack-refs", "--all", "--prune", NULL}; -static const char *argv_reflog[] = {"reflog", "expire", "--all", NULL}; -static const char *argv_repack[MAX_ADD] = {"repack", "-d", "-l", NULL}; -static const char *argv_prune[] = {"prune", "--expire", NULL, NULL, NULL}; -static const char *argv_rerere[] = {"rerere", "gc", NULL}; +static struct argv_array pack_refs_cmd = ARGV_ARRAY_INIT; +static struct argv_array reflog = ARGV_ARRAY_INIT; +static struct argv_array repack = ARGV_ARRAY_INIT; +static struct argv_array prune = ARGV_ARRAY_INIT; +static struct argv_array rerere = ARGV_ARRAY_INIT; static int gc_config(const char *var, const char *value, void *cb) { @@ -67,19 +67,6 @@ static int gc_config(const char *var, const char *value, void *cb) return git_default_config(var, value, cb); } -static void append_option(const char **cmd, const char *opt, int max_length) -{ - int i; - - for (i = 0; cmd[i]; i++) - ; - - if (i + 2 >= max_length) - die(_("Too many options specified")); - cmd[i++] = opt; - cmd[i] = NULL; -} - static int too_many_loose_objects(void) { /* @@ -147,13 +134,11 @@ static int too_many_packs(void) static void add_repack_all_option(void) { if (prune_expire && !strcmp(prune_expire, "now")) - append_option(argv_repack, "-a", MAX_ADD); + argv_array_push(&repack, "-a"); else { - append_option(argv_repack, "-A", MAX_ADD); - if (prune_expire) { - append_option(argv_repack, "--unpack-unreachable", MAX_ADD); - append_option(argv_repack, prune_expire, MAX_ADD); - } + argv_array_push(&repack, "-A"); + if (prune_expire) + argv_array_pushf(&repack, "--unpack-unreachable=%s", prune_expire); } } @@ -187,7 +172,6 @@ int cmd_gc(int argc, const char **argv, const char *prefix) int aggressive = 0; int auto_gc = 0; int quiet = 0; - char buf[80]; struct option builtin_gc_options[] = { OPT__QUIET(&quiet, "suppress progress reporting"), @@ -202,6 +186,12 @@ int cmd_gc(int argc, const char **argv, const char *prefix) if (argc == 2 && !strcmp(argv[1], "-h")) usage_with_options(builtin_gc_usage, builtin_gc_options); + argv_array_pushl(&pack_refs_cmd, "pack-refs", "--all", "--prune", NULL); + argv_array_pushl(&reflog, "reflog", "expire", "--all", NULL); + argv_array_pushl(&repack, "repack", "-d", "-l", NULL); + argv_array_pushl(&prune, "prune", "--expire", NULL ); + argv_array_pushl(&rerere, "rerere", "gc", NULL); + git_config(gc_config, NULL); if (pack_refs < 0) @@ -213,15 +203,13 @@ int cmd_gc(int argc, const char **argv, const char *prefix) usage_with_options(builtin_gc_usage, builtin_gc_options); if (aggressive) { - append_option(argv_repack, "-f", MAX_ADD); - append_option(argv_repack, "--depth=250", MAX_ADD); - if (aggressive_window > 0) { - sprintf(buf, "--window=%d", aggressive_window); - append_option(argv_repack, buf, MAX_ADD); - } + argv_array_push(&repack, "-f"); + argv_array_push(&repack, "--depth=250"); + if (aggressive_window > 0) + argv_array_pushf(&repack, "--window=%d", aggressive_window); } if (quiet) - append_option(argv_repack, "-q", MAX_ADD); + argv_array_push(&repack, "-q"); if (auto_gc) { /* @@ -239,25 +227,25 @@ int cmd_gc(int argc, const char **argv, const char *prefix) } else add_repack_all_option(); - if (pack_refs && run_command_v_opt(argv_pack_refs, RUN_GIT_CMD)) - return error(FAILED_RUN, argv_pack_refs[0]); + if (pack_refs && run_command_v_opt(pack_refs_cmd.argv, RUN_GIT_CMD)) + return error(FAILED_RUN, pack_refs_cmd.argv[0]); - if (run_command_v_opt(argv_reflog, RUN_GIT_CMD)) - return error(FAILED_RUN, argv_reflog[0]); + if (run_command_v_opt(reflog.argv, RUN_GIT_CMD)) + return error(FAILED_RUN, reflog.argv[0]); - if (run_command_v_opt(argv_repack, RUN_GIT_CMD)) - return error(FAILED_RUN, argv_repack[0]); + if (run_command_v_opt(repack.argv, RUN_GIT_CMD)) + return error(FAILED_RUN, repack.argv[0]); if (prune_expire) { - argv_prune[2] = prune_expire; + argv_array_push(&prune, prune_expire); if (quiet) - argv_prune[3] = "--no-progress"; - if (run_command_v_opt(argv_prune, RUN_GIT_CMD)) - return error(FAILED_RUN, argv_prune[0]); + argv_array_push(&prune, "--no-progress"); + if (run_command_v_opt(prune.argv, RUN_GIT_CMD)) + return error(FAILED_RUN, prune.argv[0]); } - if (run_command_v_opt(argv_rerere, RUN_GIT_CMD)) - return error(FAILED_RUN, argv_rerere[0]); + if (run_command_v_opt(rerere.argv, RUN_GIT_CMD)) + return error(FAILED_RUN, rerere.argv[0]); if (auto_gc && too_many_loose_objects()) warning(_("There are too many unreachable loose objects; " -- 2.43.2