@matheusbsbs / matheus-bagliano / commits / 8ab5aa4bd8

parseopt: handle malformed --expire arguments more nicely

A few commands that parse --expire=<time> command line option behave sillily when given nonsense input. For example $ git prune --no-expire Segmentation falut $ git prune --expire=npw; echo $? 129 Both come from parse_opt_expiry_date_cb(). The former is because the function is not prepared to see arg==NULL (for "--no-expire", it is a norm; "--expire" at the end of the command line could be made to pass NULL, if it is told that the argument is optional, but we don't so we do not have to worry about that case). The latter is because it does not check the value returned from the underlying parse_expiry_date(). This seems to be a recent regression introduced while we attempted to avoid spewing the entire usage message when given a correct option but with an invalid value at 3bb0923f ("parse-options: do not show usage upon invalid option value", 2018-03-22). Before that, we didn't fail silently but showed a full usage help (which arguably is not all that better). Also catch this error early when "git gc --prune=<expiration>" is misspelled by doing a dummy parsing before the main body of "gc" that is time consuming even begins. Otherwise, we'd spend time to pack objects and then later have "git prune" first notice the error. Aborting "gc" in the middle that way is not harmful but is ugly and can be avoided. Helped-by: Linus Torvalds <torvalds@linux-foundation.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Junio C Hamano committed Apr 21, 2018 at 12:13 UTC 8ab5aa4bd8e218b9658dbdd7f5bfcaa512f607dc

3 files changed +19 -1

builtin/gc.c

@@ -353,6 +353,7 @@ int cmd_gc(int argc, const char **argv, const char *prefix)

353 const char *name;

354 pid_t pid;

355 int daemonized = 0;

356 + timestamp_t dummy;

357

358 struct option builtin_gc_options[] = {

359 OPT__QUIET(&quiet, N_("suppress progress reporting")),

@@ -388,6 +389,9 @@ int cmd_gc(int argc, const char **argv, const char *prefix)

389 if (argc > 0)

390 usage_with_options(builtin_gc_usage, builtin_gc_options);

391

392 + if (prune_expire && parse_expiry_date(prune_expire, &dummy))

393 + die(_("failed to parse prune expiry value %s"), prune_expire);

394 +

395 if (aggressive) {

396 argv_array_push(&repack, "-f");

397 if (aggressive_depth > 0)

parse-options-cb.c

+5 -1

@@ -38,7 +38,11 @@ int parse_opt_approxidate_cb(const struct option *opt, const char *arg,

38 int parse_opt_expiry_date_cb(const struct option *opt, const char *arg,

39 int unset)

40 {

41 - return parse_expiry_date(arg, (timestamp_t *)opt->value);

41 + if (unset)

42 + arg = "never";

43 + if (parse_expiry_date(arg, (timestamp_t *)opt->value))

44 + die(_("malformed expiration date '%s'"), arg);

45 + return 0;

46 }

48 int parse_opt_color_flag_cb(const struct option *opt, const char *arg,

t/t5304-prune.sh

+10

@@ -320,4 +320,14 @@ test_expect_success 'prune: handle HEAD reflog in multiple worktrees' '

320 test_cmp expected actual

321 '

322

323 +test_expect_success 'prune: handle expire option correctly' '

324 + test_must_fail git prune --expire 2>error &&

325 + test_i18ngrep "requires a value" error &&

326 +

327 + test_must_fail git prune --expire=nyah 2>error &&

328 + test_i18ngrep "malformed expiration" error &&

329 +

330 + git prune --no-expire

331 +'

332 +

333 test_done