[PATCH] fix incorrect line number report for bad gitconfig

[PATCH] fix incorrect line number report for bad gitconfig

[Martin Stenberg]

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

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

A .gitconfig section with a missing "]" reports the next line as beeing 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 |   16 ++++++++++++----
 1 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/config.c b/config.c
index ad03908..8d96ba1 100644
--- a/config.c
+++ b/config.c
@@ -196,8 +196,10 @@ static char *parse_value(void)
 	for (;;) {
 		int c = get_next_char();
 		if (c == '\n') {
-			if (quote)
+			if (quote) {
+				cf->linenr--;
 				return NULL;
+			}
 			return cf->value.buf;
 		}
 		if (comment)
@@ -286,8 +288,10 @@ static int get_value(config_fn_t fn, void *data, char *name, unsigned int len)
 static int get_extended_base_var(char *name, int baselen, int c)
 {
 	do {
-		if (c == '\n')
+		if (c == '\n') {
+			cf->linenr--;
 			return -1;
+		}
 		c = get_next_char();
 	} while (isspace(c));
 
@@ -298,14 +302,18 @@ static int get_extended_base_var(char *name, int baselen, int c)
 
 	for (;;) {
 		int c = get_next_char();
-		if (c == '\n')
+		if (c == '\n') {
+			cf->linenr--;
 			return -1;
+		}
 		if (c == '"')
 			break;
 		if (c == '\\') {
 			c = get_next_char();
-			if (c == '\n')
+			if (c == '\n') {
+				cf->linenr--;
 				return -1;
+			}
 		}
 		name[baselen++] = c;
 		if (baselen > MAXNAME / 2)
-- 
1.7.9.1


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

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

[Junio C Hamano]

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.

> A .gitconfig section with a missing "]" reports the next line as beeing bad,
> same goes to a value with a missing end quote.

If you say ".gitconfig", it gives a false impression to the readers
that this is only limited to parsing of $HOME/.gitconfig, which is
not the problem you are addressing.  "A section in a config file"
would be a better phrasing.

s/beeing/being/;

> 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>

Please have a paragraph break before your S-o-b: line.

Thanks. Review of the patch text follows.

> ---
>  config.c |   16 ++++++++++++----
>  1 files changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/config.c b/config.c
> index ad03908..8d96ba1 100644
> --- a/config.c
> +++ b/config.c
> @@ -196,8 +196,10 @@ static char *parse_value(void)
>  	for (;;) {
>  		int c = get_next_char();
>  		if (c == '\n') {
> -			if (quote)
> +			if (quote) {
> +				cf->linenr--;
>  				return NULL;
> +			}
>  			return cf->value.buf;
>  		}
>  		if (comment)
> @@ -286,8 +288,10 @@ static int get_value(config_fn_t fn, void *data, char *name, unsigned int len)
>  static int get_extended_base_var(char *name, int baselen, int c)
>  {
>  	do {
> -		if (c == '\n')
> +		if (c == '\n') {
> +			cf->linenr--;
> ...
>  		}

We might want to consolidate the error-return codepath in this
function to a single place by judicious use of "goto", perhaps like
this:

 config.c |    9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/config.c b/config.c
index ad03908..e222e85 100644
--- a/config.c
+++ b/config.c
@@ -287,7 +287,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 +299,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 +316,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)

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

[Junio C Hamano]

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.

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

[Martin Stenberg]

[-- 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 --]