# [PATCH] diff: report bogus input to -C/-M/-B

4 messages from 2010-10-21 to 2010-10-21. Participants: Jeff King, Jonathan Nieder, Kevin Ballard.
Thread: https://gitlist.dev/t/25499

## Jeff King, 2010-10-21 14:49

Subject: [PATCH] diff: report bogus input to -C/-M/-B
Message-ID: <20101021144947.GA18210@sigill.intra.peff.net>
URL: https://gitlist.dev/e/20101021144947.GA18210%40sigill.intra.peff.net

```
We already detect invalid input to these functions, but we
simply exit with an error code, never saying anything as
simple as "your input was wrong". Let's fix that.

Before:

  $ git diff -CM
  $ echo $?
  128

After:

  $ git diff -CM
  error: invalid argument to -C: M
  $ echo $?
  128

There should be no problems with having diff_opt_parse print
to stderr, as there is already precedent in complaining
about bogus --color and --output arguments.

Signed-off-by: Jeff King <peff@peff.net>
---
More old patch potpourri. Original thread:

  http://article.gmane.org/gmane.comp.version-control.git/146570

I think this one just got overlooked.

 diff.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/diff.c b/diff.c
index 71efa8e..8487643 100644
--- a/diff.c
+++ b/diff.c
@@ -3142,18 +3142,18 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)
 	/* renames options */
 	else if (!prefixcmp(arg, "-B")) {
 		if ((options->break_opt = diff_scoreopt_parse(arg)) == -1)
-			return -1;
+			return error("invalid argument to -B: %s", arg+2);
 	}
 	else if (!prefixcmp(arg, "-M")) {
 		if ((options->rename_score = diff_scoreopt_parse(arg)) == -1)
-			return -1;
+			return error("invalid argument to -M: %s", arg+2);
 		options->detect_rename = DIFF_DETECT_RENAME;
 	}
 	else if (!prefixcmp(arg, "-C")) {
 		if (options->detect_rename == DIFF_DETECT_COPY)
 			DIFF_OPT_SET(options, FIND_COPIES_HARDER);
 		if ((options->rename_score = diff_scoreopt_parse(arg)) == -1)
-			return -1;
+			return error("invalid argument to -C: %s", arg+2);
 		options->detect_rename = DIFF_DETECT_COPY;
 	}
 	else if (!strcmp(arg, "--no-renames"))
-- 
1.7.3.1.235.gdd6c0

```

## Jonathan Nieder, 2010-10-21 16:53

Subject: Re: [PATCH] diff: report bogus input to -C/-M/-B
Message-ID: <20101021165340.GE3732@burratino>
URL: https://gitlist.dev/e/20101021165340.GE3732%40burratino
In-Reply-To: <20101021144947.GA18210@sigill.intra.peff.net>

```
Jeff King wrote:

> Before:
> 
>   $ git diff -CM
>   $ echo $?
>   128
> 
> After:
> 
>   $ git diff -CM
>   error: invalid argument to -C: M
>   $ echo $?
>   128

Yes, please.

Who is it that exits(128) in this code path?  Are there other
functions it calls that might return an error without reporting it to
stderr?

```

## Jeff King, 2010-10-21 17:07

Subject: Re: [PATCH] diff: report bogus input to -C/-M/-B
Message-ID: <20101021170744.GB15667@sigill.intra.peff.net>
URL: https://gitlist.dev/e/20101021170744.GB15667%40sigill.intra.peff.net
In-Reply-To: <20101021165340.GE3732@burratino>

```
On Thu, Oct 21, 2010 at 11:53:40AM -0500, Jonathan Nieder wrote:

> Jeff King wrote:
> 
> > Before:
> > 
> >   $ git diff -CM
> >   $ echo $?
> >   128
> > 
> > After:
> > 
> >   $ git diff -CM
> >   error: invalid argument to -C: M
> >   $ echo $?
> >   128
> 
> Yes, please.
> 
> Who is it that exits(128) in this code path?  Are there other
> functions it calls that might return an error without reporting it to
> stderr?

It's setup_revisions(). It only happens when calling
handle_revision_opt(), which either succeeds, calls error() for its
error cases, or chains to diff_opt_parse(). The latter uses error()
appropriately except for the -B/-M/-C cases.

So I think we're good after this patch is applied.

-Peff

```

## Kevin Ballard, 2010-10-21 20:37

Subject: Re: [PATCH] diff: report bogus input to -C/-M/-B
Message-ID: <9E2CE0E8-40A4-4909-9317-B3DE9006474E@sb.org>
URL: https://gitlist.dev/e/9E2CE0E8-40A4-4909-9317-B3DE9006474E%40sb.org
In-Reply-To: <20101021144947.GA18210@sigill.intra.peff.net>

```
On Oct 21, 2010, at 7:49 AM, Jeff King wrote:

> We already detect invalid input to these functions, but we
> simply exit with an error code, never saying anything as
> simple as "your input was wrong". Let's fix that.
> 
> Before:
> 
>  $ git diff -CM
>  $ echo $?
>  128
> 
> After:
> 
>  $ git diff -CM
>  error: invalid argument to -C: M
>  $ echo $?
>  128
> 
> There should be no problems with having diff_opt_parse print
> to stderr, as there is already precedent in complaining
> about bogus --color and --output arguments.

Thank you. I've had a note to myself to fix this for the past month, and still haven't gotten around to it. Glad to see that procrastinating pays off! ;)

-Kevin Ballard
```