cherry-pick: detect bogus arguments to --mainlinejk/cherry-pick-0-mainline

The cherry-pick and revert commands use OPT_INTEGER() to
parse --mainline. The stock parser is smart enough to reject
non-numeric nonsense, but it doesn't know that parent
counting starts at 1.

Worse, the value "0" is indistinguishable from the unset
case, so a user who assumes the counting is 0-based will get
a confusing message:

  $ git cherry-pick -m 0 $merge
  error: commit ... is a merge but no -m option was given.

Let's use a custom callback that enforces our range.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

2 files changed, 29 insertions, 1 deletions

diff --git a/builtin/revert.c b/builtin/revert.c
index 4e693808b1..37df613ac7 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -54,6 +54,24 @@ static int option_parse_x(const struct option *opt,
 	return 0;
 }
 
+static int option_parse_m(const struct option *opt,
+			  const char *arg, int unset)
+{
+	struct replay_opts *replay = opt->value;
+	char *end;
+
+	if (unset) {
+		replay->mainline = 0;
+		return 0;
+	}
+
+	replay->mainline = strtol(arg, &end, 10);
+	if (*end || replay->mainline <= 0)
+		return opterror(opt, "expects a number greater than zero", 0);
+
+	return 0;
+}
+
 LAST_ARG_MUST_BE_NULL
 static void verify_opt_compatible(const char *me, const char *base_opt, ...)
 {
@@ -84,7 +102,8 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)
 		OPT_BOOL('e', "edit", &opts->edit, N_("edit the commit message")),
 		OPT_NOOP_NOARG('r', NULL),
 		OPT_BOOL('s', "signoff", &opts->signoff, N_("add Signed-off-by:")),
-		OPT_INTEGER('m', "mainline", &opts->mainline, N_("parent number")),
+		OPT_CALLBACK('m', "mainline", opts, N_("parent-number"),
+			     N_("select mainline parent"), option_parse_m),
 		OPT_RERERE_AUTOUPDATE(&opts->allow_rerere_auto),
 		OPT_STRING(0, "strategy", &opts->strategy, N_("strategy"), N_("merge strategy")),
 		OPT_CALLBACK('X', "strategy-option", &opts, N_("option"),
diff --git a/t/t3502-cherry-pick-merge.sh b/t/t3502-cherry-pick-merge.sh
index e37547f41a..b1602718f8 100755
--- a/t/t3502-cherry-pick-merge.sh
+++ b/t/t3502-cherry-pick-merge.sh
@@ -31,6 +31,15 @@ test_expect_success setup '
 
 '
 
+test_expect_success 'cherry-pick -m complains of bogus numbers' '
+	# expect 129 here to distinguish between cases where
+	# there was nothing to cherry-pick
+	test_expect_code 129 git cherry-pick -m &&
+	test_expect_code 129 git cherry-pick -m foo b &&
+	test_expect_code 129 git cherry-pick -m -1 b &&
+	test_expect_code 129 git cherry-pick -m 0 b
+'
+
 test_expect_success 'cherry-pick a non-merge with -m should fail' '
 
 	git reset --hard &&