[PATCH] diff-no-index: correctly diagnose error return from diff_opt_parse()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



diff_opt_parse() returns the number of options parsed, or often
returns error() which is defined to return -1.  Yes, return value of
0 is "I did not process that option at all", which should cause the
caller to say that, but negative return should not be forgotten.

This bug the caused it to infinitely show the same error message
because the returned value was used to decrement the loop control
variable, e.g.

        $ git diff --no-index --color=words a b
        error: option `color' expects "always", "auto", or "never"
        error: option `color' expects "always", "auto", or "never"
        ...

Instead, make it act like so:

        $ git diff --no-index --color=words a b
        error: option `color' expects "always", "auto", or "never"
        fatal: invalid diff option/value: --color=words

Reported-by: Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx>
Signed-off-by: Junio C Hamano <gitster@xxxxxxxxx>
---

 * The other callchain to this function comes from rev-list.c, and
   the caller does handle it correctly.

 diff-no-index.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/diff-no-index.c b/diff-no-index.c
index 00a8eef..ecf9293 100644
--- a/diff-no-index.c
+++ b/diff-no-index.c
@@ -244,7 +244,7 @@ void diff_no_index(struct rev_info *revs,
 			i++;
 		else {
 			j = diff_opt_parse(&revs->diffopt, argv + i, argc - i);
-			if (!j)
+			if (j <= 0)
 				die("invalid diff option/value: %s", argv[i]);
 			i += j;
 		}
-- 
1.9.1-513-gce53123

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]