Re: [PATCH v2 2/2] rebase: validate -C<n> and --whitespace=<mode> parameters early

Hi Phillip,

On Wed, 14 Nov 2018, Phillip Wood wrote:

Thanks for doing this, I think this patch is good.

Thanks!

I've not checked the first patch as I think it is the same as before
judging from the covering letter.

Indeed, that's what the range-diff said. Sorry for not stating this
explicitly.

Thank you for your review,
Dscho

Best Wishes

Phillip

On 14/11/2018 16:25, Johannes Schindelin via GitGitGadget wrote:

quoted

From: Johannes Schindelin <redacted>

It is a good idea to error out early upon seeing, say, `-Cbad`, rather
than starting the rebase only to have the `--am` backend complain later.

Let's do this.

The only options accepting parameters which we pass through to `git am`
(which may, or may not, forward them to `git apply`) are `-C` and
`--whitespace`. The other options we pass through do not accept
parameters, so we do not have to validate them here.

Signed-off-by: Johannes Schindelin <redacted>
---
  builtin/rebase.c          | 12 +++++++++++-
  t/t3406-rebase-message.sh |  7 +++++++
  2 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/builtin/rebase.c b/builtin/rebase.c
index 96ffa80b71..571cf899d5 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c

@@ -1064,12 +1064,22 @@ int cmd_rebase(int argc, const char **argv,

const char *prefix)
   }
  
  	for (i = 0; i < options.git_am_opts.argc; i++) {
-		const char *option = options.git_am_opts.argv[i];
+		const char *option = options.git_am_opts.argv[i], *p;
    if (!strcmp(option, "--committer-date-is-author-date") ||
!strcmp(option, "--ignore-date") ||
!strcmp(option, "--whitespace=fix") ||
!strcmp(option, "--whitespace=strip"))
  			options.flags |= REBASE_FORCE;
+		else if (skip_prefix(option, "-C", &p)) {
+			while (*p)
+				if (!isdigit(*(p++)))
+					die(_("switch `C' expects a "
+					      "numerical value"));
+		} else if (skip_prefix(option, "--whitespace=", &p)) {
+			if (*p && strcmp(p, "warn") && strcmp(p, "nowarn")
&&
+			    strcmp(p, "error") && strcmp(p, "error-all"))
+				die("Invalid whitespace option: '%s'", p);
+		}
   }
  
  	if (!(options.flags & REBASE_NO_QUIET))

diff --git a/t/t3406-rebase-message.sh b/t/t3406-rebase-message.sh
index 0392e36d23..2c79eed4fe 100755
--- a/t/t3406-rebase-message.sh
+++ b/t/t3406-rebase-message.sh

@@ -84,4 +84,11 @@ test_expect_success 'rebase --onto outputs the

invalid ref' '
  	test_i18ngrep "invalid-ref" err
  '
  
+test_expect_success 'error out early upon -C<n> or --whitespace=<bad>'
'
+	test_must_fail git rebase -Cnot-a-number HEAD 2>err &&
+	test_i18ngrep "numerical value" err &&
+	test_must_fail git rebase --whitespace=bad HEAD 2>err &&
+	test_i18ngrep "Invalid whitespace option" err
+'
+
  test_done