{"thread":{"id":"29901","subject":"[PATCH] fix incorrect line number report for bad gitconfig","startedAt":"2012-03-09T22:30:14Z","lastAt":"2012-03-12T12:01:05Z","messageCount":4,"participants":["Martin Stenberg","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"186594","messageId":"20120309223014.GA6380@gnutiken.se","threadId":"29901","inReplyTo":null,"subject":"[PATCH] fix incorrect line number report for bad gitconfig","fromName":"Martin Stenberg","fromEmail":"martin@gnutiken.se","sentAt":"2012-03-09T22:30:14Z","receivedAt":"2012-03-09T22:30:14Z","isPatch":true,"sender":{"key":"martin@gnutiken.se","avatar":null},"body":"From c88f00e06cb877f9d944fdec480f53a7a42dd990 Mon Sep 17 00:00:00 2001\nFrom: Martin Stenberg \nDate: Fri, 9 Mar 2012 22:57:54 +0100\nSubject: [PATCH] fix incorrect line number report for bad gitconfig\n\nA .gitconfig section with a missing \"]\" reports the next line as beeing bad,\nsame goes to a value with a missing end quote.\n\nThis happens because the error is not detected until the end of the line, when\nline number is already increased. Fix this issue by decreasing line number by\none for these cases.\nSigned-off-by: Martin Stenberg \n---\n config.c | 16 ++++++++++++----\n 1 files changed, 12 insertions(+), 4 deletions(-)\n\ndiff --git a/config.c b/config.c\nindex ad03908..8d96ba1 100644\n--- a/config.c\n+++ b/config.c\n@@ -196,8 +196,10 @@ static char *parse_value(void)\n \tfor (;;) {\n \t\tint c = get_next_char();\n \t\tif (c == '\\n') {\n-\t\t\tif (quote)\n+\t\t\tif (quote) {\n+\t\t\t\tcf->linenr--;\n \t\t\t\treturn NULL;\n+\t\t\t}\n \t\t\treturn cf->value.buf;\n \t\t}\n \t\tif (comment)\n@@ -286,8 +288,10 @@ static int get_value(config_fn_t fn, void *data, char *name, unsigned int len)\n static int get_extended_base_var(char *name, int baselen, int c)\n {\n \tdo {\n-\t\tif (c == '\\n')\n+\t\tif (c == '\\n') {\n+\t\t\tcf->linenr--;\n \t\t\treturn -1;\n+\t\t}\n \t\tc = get_next_char();\n \t} while (isspace(c));\n \n@@ -298,14 +302,18 @@ static int get_extended_base_var(char *name, int baselen, int c)\n \n \tfor (;;) {\n \t\tint c = get_next_char();\n-\t\tif (c == '\\n')\n+\t\tif (c == '\\n') {\n+\t\t\tcf->linenr--;\n \t\t\treturn -1;\n+\t\t}\n \t\tif (c == '\"')\n \t\t\tbreak;\n \t\tif (c == '\\\\') {\n \t\t\tc = get_next_char();\n-\t\t\tif (c == '\\n')\n+\t\t\tif (c == '\\n') {\n+\t\t\t\tcf->linenr--;\n \t\t\t\treturn -1;\n+\t\t\t}\n \t\t}\n \t\tname[baselen++] = c;\n \t\tif (baselen > MAXNAME / 2)\n-- \n1.7.9.1\n\n"},{"id":"186598","messageId":"7v8vj94c1a.fsf@alter.siamese.dyndns.org","threadId":"29901","inReplyTo":"20120309223014.GA6380@gnutiken.se","subject":"Re: [PATCH] fix incorrect line number report for bad gitconfig","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-03-09T22:58:25Z","receivedAt":"2012-03-09T22:58:25Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Martin Stenberg writes:\n\n> From c88f00e06cb877f9d944fdec480f53a7a42dd990 Mon Sep 17 00:00:00 2001\n> From: Martin Stenberg \n> Date: Fri, 9 Mar 2012 22:57:54 +0100\n> Subject: [PATCH] fix incorrect line number report for bad gitconfig\n\nPlease drop these four lines. The commit object name is no use to\nme, and the other lines duplicate what you have in the mail header.\n\n> A .gitconfig section with a missing \"]\" reports the next line as beeing bad,\n> same goes to a value with a missing end quote.\n\nIf you say \".gitconfig\", it gives a false impression to the readers\nthat this is only limited to parsing of $HOME/.gitconfig, which is\nnot the problem you are addressing. \"A section in a config file\"\nwould be a better phrasing.\n\ns/beeing/being/;\n\n> This happens because the error is not detected until the end of the line, when\n> line number is already increased. Fix this issue by decreasing line number by\n> one for these cases.\n> Signed-off-by: Martin Stenberg \n\nPlease have a paragraph break before your S-o-b: line.\n\nThanks. Review of the patch text follows.\n\n> ---\n> config.c | 16 ++++++++++++----\n> 1 files changed, 12 insertions(+), 4 deletions(-)\n>\n> diff --git a/config.c b/config.c\n> index ad03908..8d96ba1 100644\n> --- a/config.c\n> +++ b/config.c\n> @@ -196,8 +196,10 @@ static char *parse_value(void)\n> \tfor (;;) {\n> \t\tint c = get_next_char();\n> \t\tif (c == '\\n') {\n> -\t\t\tif (quote)\n> +\t\t\tif (quote) {\n> +\t\t\t\tcf->linenr--;\n> \t\t\t\treturn NULL;\n> +\t\t\t}\n> \t\t\treturn cf->value.buf;\n> \t\t}\n> \t\tif (comment)\n> @@ -286,8 +288,10 @@ static int get_value(config_fn_t fn, void *data, char *name, unsigned int len)\n> static int get_extended_base_var(char *name, int baselen, int c)\n> {\n> \tdo {\n> -\t\tif (c == '\\n')\n> +\t\tif (c == '\\n') {\n> +\t\t\tcf->linenr--;\n> ...\n> \t\t}\n\nWe might want to consolidate the error-return codepath in this\nfunction to a single place by judicious use of \"goto\", perhaps like\nthis:\n\n config.c | 9 ++++++---\n 1 file changed, 6 insertions(+), 3 deletions(-)\n\ndiff --git a/config.c b/config.c\nindex ad03908..e222e85 100644\n--- a/config.c\n+++ b/config.c\n@@ -287,7 +287,7 @@ static int get_extended_base_var(char *name, int baselen, int c)\n {\n \tdo {\n \t\tif (c == '\\n')\n-\t\t\treturn -1;\n+\t\t\tgoto error_incomplete_line;\n \t\tc = get_next_char();\n \t} while (isspace(c));\n \n@@ -299,13 +299,13 @@ static int get_extended_base_var(char *name, int baselen, int c)\n \tfor (;;) {\n \t\tint c = get_next_char();\n \t\tif (c == '\\n')\n-\t\t\treturn -1;\n+\t\t\tgoto error_incomplete_line;\n \t\tif (c == '\"')\n \t\t\tbreak;\n \t\tif (c == '\\\\') {\n \t\t\tc = get_next_char();\n \t\t\tif (c == '\\n')\n-\t\t\t\treturn -1;\n+\t\t\t\tgoto error_incomplete_line;\n \t\t}\n \t\tname[baselen++] = c;\n \t\tif (baselen > MAXNAME / 2)\n@@ -316,6 +316,9 @@ static int get_extended_base_var(char *name, int baselen, int c)\n \tif (get_next_char() != ']')\n \t\treturn -1;\n \treturn baselen;\n+error_incomplete_line:\n+\tcf->linenr--;\n+\treturn -1;\n }\n \n static int get_base_var(char *name)\n"},{"id":"186679","messageId":"7vhaxuuzvt.fsf@alter.siamese.dyndns.org","threadId":"29901","inReplyTo":"7v8vj94c1a.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] fix incorrect line number report for bad gitconfig","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-03-12T05:55:34Z","receivedAt":"2012-03-12T05:55:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Martin Stenberg writes:\n>\n>> From c88f00e06cb877f9d944fdec480f53a7a42dd990 Mon Sep 17 00:00:00 2001\n>> From: Martin Stenberg \n>> Date: Fri, 9 Mar 2012 22:57:54 +0100\n>> Subject: [PATCH] fix incorrect line number report for bad gitconfig\n>\n> Please drop these four lines. The commit object name is no use to\n> me, and the other lines duplicate what you have in the mail header.\n> ...\n> Please have a paragraph break before your S-o-b: line.\n>\n> Thanks. Review of the patch text follows.\n> ...\n\nAlso could you add a couple of tests (you identified two cases in\nyour log message) to make sure this fix will not be broken in the\nfuture, perhaps to t1300?\n\nThanks.\n"},{"id":"186697","messageId":"20120312120105.GA19176@gnutiken.se","threadId":"29901","inReplyTo":"7vhaxuuzvt.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] fix incorrect line number report for bad gitconfig","fromName":"Martin Stenberg","fromEmail":"martin@gnutiken.se","sentAt":"2012-03-12T12:01:05Z","receivedAt":"2012-03-12T12:01:05Z","isPatch":true,"sender":{"key":"martin@gnutiken.se","avatar":null},"body":"On Sun, Mar 11, 2012 at 10:55:34PM -0700, Junio C Hamano wrote:\n> Junio C Hamano writes:\n> \n> > Martin Stenberg writes:\n> >\n> >> From c88f00e06cb877f9d944fdec480f53a7a42dd990 Mon Sep 17 00:00:00 2001\n> >> From: Martin Stenberg \n> >> Date: Fri, 9 Mar 2012 22:57:54 +0100\n> >> Subject: [PATCH] fix incorrect line number report for bad gitconfig\n> >\n> > Please drop these four lines. The commit object name is no use to\n> > me, and the other lines duplicate what you have in the mail header.\n> > ...\n> > Please have a paragraph break before your S-o-b: line.\n> >\n> > Thanks. Review of the patch text follows.\n> > ...\n> \n> Also could you add a couple of tests (you identified two cases in\n> your log message) to make sure this fix will not be broken in the\n> future, perhaps to t1300?\n> \n> Thanks.\n\nThanks for the feedback. Fixed the code and added two test cases.\n\nNew commit description and patch follows.\n\nfix incorrect line number report for bad config\n\nA section in a config file with a missing \"]\" reports the next line as being\nbad, same goes to a value with a missing end quote.\n\nThis happens because the error is not detected until the end of the line, when\nline number is already increased. Fix this issue by decreasing line number by\none for these cases.\n\nSigned-off-by: Martin Stenberg \n---\n config.c | 14 ++++++++++----\n t/t1300-repo-config.sh | 36 ++++++++++++++++++++++++++++++++++++\n 2 files changed, 46 insertions(+), 4 deletions(-)\n\ndiff --git a/config.c b/config.c\nindex ad03908..0019b1c 100644\n--- a/config.c\n+++ b/config.c\n@@ -197,7 +197,7 @@ static char *parse_value(void)\n \t\tint c = get_next_char();\n \t\tif (c == '\\n') {\n \t\t\tif (quote)\n-\t\t\t\treturn NULL;\n+\t\t\t\tgoto error_incomplete_line;\n \t\t\treturn cf->value.buf;\n \t\t}\n \t\tif (comment)\n@@ -245,6 +245,9 @@ static char *parse_value(void)\n \t\t}\n \t\tstrbuf_addch(&cf->value, c);\n \t}\n+error_incomplete_line:\n+\tcf->linenr--;\n+\treturn NULL;\n }\n \n static inline int iskeychar(int c)\n@@ -287,7 +290,7 @@ static int get_extended_base_var(char *name, int baselen, int c)\n {\n \tdo {\n \t\tif (c == '\\n')\n-\t\t\treturn -1;\n+\t\t\tgoto error_incomplete_line;\n \t\tc = get_next_char();\n \t} while (isspace(c));\n \n@@ -299,13 +302,13 @@ static int get_extended_base_var(char *name, int baselen, int c)\n \tfor (;;) {\n \t\tint c = get_next_char();\n \t\tif (c == '\\n')\n-\t\t\treturn -1;\n+\t\t\tgoto error_incomplete_line;\n \t\tif (c == '\"')\n \t\t\tbreak;\n \t\tif (c == '\\\\') {\n \t\t\tc = get_next_char();\n \t\t\tif (c == '\\n')\n-\t\t\t\treturn -1;\n+\t\t\t\tgoto error_incomplete_line;\n \t\t}\n \t\tname[baselen++] = c;\n \t\tif (baselen > MAXNAME / 2)\n@@ -316,6 +319,9 @@ static int get_extended_base_var(char *name, int baselen, int c)\n \tif (get_next_char() != ']')\n \t\treturn -1;\n \treturn baselen;\n+error_incomplete_line:\n+\tcf->linenr--;\n+\treturn -1;\n }\n \n static int get_base_var(char *name)\ndiff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh\nindex 5f249f6..dac3008 100755\n--- a/t/t1300-repo-config.sh\n+++ b/t/t1300-repo-config.sh\n@@ -985,4 +985,40 @@ test_expect_success 'git config --edit respects core.editor' '\n \ttest_cmp expect actual\n '\n \n+cat > expect < .git/config <<\\EOF\n+[section\n+key = value\n+EOF\n+\n+test_expect_success 'incomplete section line' '\n+\tif git config --get section.key 2>actual\n+\tthen\n+\t\techo config should have failed\n+\t\tfalse\n+\tfi &&\n+\tcmp actual expect\n+'\n+\n+cat > expect < .git/config <<\\EOF\n+[section]\n+ key = \"value\n+EOF\n+\n+test_expect_success 'incomplete quoted line' '\n+\tif git config --get section.key 2>actual\n+\tthen\n+\t\techo config should have failed\n+\t\tfalse\n+\tfi &&\n+\tcmp actual expect\n+'\n+\n test_done\n-- \n1.7.9.1\n\n"}]}