Re: [PATCH] fix incorrect line number report for bad gitconfig

[Martin Stenberg <martin@gnutiken.se>]

[-- Attachment #1: Type: text/plain, Size: 3933 bytes --]

On Sun, Mar 11, 2012 at 10:55:34PM -0700, Junio C Hamano wrote:
> Junio C Hamano <gitster@pobox.com> writes:
> 
> > Martin Stenberg <martin@gnutiken.se> writes:
> >
> >> From c88f00e06cb877f9d944fdec480f53a7a42dd990 Mon Sep 17 00:00:00 2001
> >> From: Martin Stenberg <martin@gnutiken.se>
> >> Date: Fri, 9 Mar 2012 22:57:54 +0100
> >> Subject: [PATCH] fix incorrect line number report for bad gitconfig
> >
> > Please drop these four lines. The commit object name is no use to
> > me, and the other lines duplicate what you have in the mail header.
> > ...
> > Please have a paragraph break before your S-o-b: line.
> >
> > Thanks. Review of the patch text follows.
> > ...
> 
> Also could you add a couple of tests (you identified two cases in
> your log message) to make sure this fix will not be broken in the
> future, perhaps to t1300?
> 
> Thanks.

Thanks for the feedback. Fixed the code and added two test cases.

New commit description and patch follows.

fix incorrect line number report for bad config

A section in a config file with a missing "]" reports the next line as being
bad, same goes to a value with a missing end quote.

This happens because the error is not detected until the end of the line, when
line number is already increased. Fix this issue by decreasing line number by
one for these cases.

Signed-off-by: Martin Stenberg <martin@gnutiken.se>
---
 config.c               |   14 ++++++++++----
 t/t1300-repo-config.sh |   36 ++++++++++++++++++++++++++++++++++++
 2 files changed, 46 insertions(+), 4 deletions(-)

diff --git a/config.c b/config.c
index ad03908..0019b1c 100644
--- a/config.c
+++ b/config.c
@@ -197,7 +197,7 @@ static char *parse_value(void)
 		int c = get_next_char();
 		if (c == '\n') {
 			if (quote)
-				return NULL;
+				goto error_incomplete_line;
 			return cf->value.buf;
 		}
 		if (comment)
@@ -245,6 +245,9 @@ static char *parse_value(void)
 		}
 		strbuf_addch(&cf->value, c);
 	}
+error_incomplete_line:
+	cf->linenr--;
+	return NULL;
 }
 
 static inline int iskeychar(int c)
@@ -287,7 +290,7 @@ static int get_extended_base_var(char *name, int baselen, int c)
 {
 	do {
 		if (c == '\n')
-			return -1;
+			goto error_incomplete_line;
 		c = get_next_char();
 	} while (isspace(c));
 
@@ -299,13 +302,13 @@ static int get_extended_base_var(char *name, int baselen, int c)
 	for (;;) {
 		int c = get_next_char();
 		if (c == '\n')
-			return -1;
+			goto error_incomplete_line;
 		if (c == '"')
 			break;
 		if (c == '\\') {
 			c = get_next_char();
 			if (c == '\n')
-				return -1;
+				goto error_incomplete_line;
 		}
 		name[baselen++] = c;
 		if (baselen > MAXNAME / 2)
@@ -316,6 +319,9 @@ static int get_extended_base_var(char *name, int baselen, int c)
 	if (get_next_char() != ']')
 		return -1;
 	return baselen;
+error_incomplete_line:
+	cf->linenr--;
+	return -1;
 }
 
 static int get_base_var(char *name)
diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh
index 5f249f6..dac3008 100755
--- a/t/t1300-repo-config.sh
+++ b/t/t1300-repo-config.sh
@@ -985,4 +985,40 @@ test_expect_success 'git config --edit respects core.editor' '
 	test_cmp expect actual
 '
 
+cat > expect <<EOF
+fatal: bad config file line 1 in .git/config
+EOF
+
+cat > .git/config <<\EOF
+[section
+key = value
+EOF
+
+test_expect_success 'incomplete section line' '
+	if git config --get section.key 2>actual
+	then
+		echo config should have failed
+		false
+	fi &&
+	cmp actual expect
+'
+
+cat > expect <<EOF
+fatal: bad config file line 2 in .git/config
+EOF
+
+cat > .git/config <<\EOF
+[section]
+    key = "value
+EOF
+
+test_expect_success 'incomplete quoted line' '
+	if git config --get section.key 2>actual
+	then
+		echo config should have failed
+		false
+	fi &&
+	cmp actual expect
+'
+
 test_done
-- 
1.7.9.1


[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]