[PATCHv2] git-config: Parse config files leniently

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

 



Currently, git config dies as soon as there is a parsing error. This is
especially unfortunate in case a user tries to correct config mistakes
using git config -e.

Instead, issue a warning only and treat the rest of the line as a
comment (ignore it). This benefits not only git config -e users but
also everyone else.

Signed-off-by: Michael J Gruber <git@xxxxxxxxxxxxxxxxxxxx>
Reported-by: David Reitter <david.reitter@xxxxxxxxx>
---
 config.c                |   80 ++++++++++++++++++++++++----------------------
 t/t1303-wacky-config.sh |    3 +-
 2 files changed, 44 insertions(+), 39 deletions(-)

So, after a business trip, vacation, ... I'm finally returning to this patch.
I addressed the echo/printf issue as suggested and clarified the commit message.

Regarding the global int for switching on/off lenient parsing: I reinstated my
v0 of the patch only to find out (again) that setup_git_directory_gently is the
problem here. We would have to turn on lenient parsing before we even know that
"-e" is supplied. So, the only option would be to have "git config" use lenient
parsing in all modes (edit/get/set) and have other git commands die fatally on
erroneous config.

So, here's v2 which has lenient config parsing for everyone because I don't see
a way to have it for "git config -e" only. If you prefer to have it for all of
"git config" only, that version is in another branch head now...

Cheers,
Michael

diff --git a/config.c b/config.c
index e87edea..5e0af5d 100644
--- a/config.c
+++ b/config.c
@@ -207,50 +207,54 @@ static int git_parse_file(config_fn_t fn, void *data)
 	static const unsigned char *utf8_bom = (unsigned char *) "\xef\xbb\xbf";
 	const unsigned char *bomptr = utf8_bom;
 
-	for (;;) {
-		int c = get_next_char();
-		if (bomptr && *bomptr) {
-			/* We are at the file beginning; skip UTF8-encoded BOM
-			 * if present. Sane editors won't put this in on their
-			 * own, but e.g. Windows Notepad will do it happily. */
-			if ((unsigned char) c == *bomptr) {
-				bomptr++;
+	while (!config_file_eof) {
+		for (;;) {
+			int c = get_next_char();
+			if (bomptr && *bomptr) {
+				/* We are at the file beginning; skip UTF8-encoded BOM
+				 * if present. Sane editors won't put this in on their
+				 * own, but e.g. Windows Notepad will do it happily. */
+				if ((unsigned char) c == *bomptr) {
+					bomptr++;
+					continue;
+				} else {
+					/* Do not tolerate partial BOM. */
+					if (bomptr != utf8_bom)
+						break;
+					/* No BOM at file beginning. Cool. */
+					bomptr = NULL;
+				}
+			}
+			if (c == '\n') {
+				if (config_file_eof)
+					return 0;
+				comment = 0;
 				continue;
-			} else {
-				/* Do not tolerate partial BOM. */
-				if (bomptr != utf8_bom)
+			}
+			if (comment || isspace(c))
+				continue;
+			if (c == '#' || c == ';') {
+				comment = 1;
+				continue;
+			}
+			if (c == '[') {
+				baselen = get_base_var(var);
+				if (baselen <= 0)
 					break;
-				/* No BOM at file beginning. Cool. */
-				bomptr = NULL;
+				var[baselen++] = '.';
+				var[baselen] = 0;
+				continue;
 			}
-		}
-		if (c == '\n') {
-			if (config_file_eof)
-				return 0;
-			comment = 0;
-			continue;
-		}
-		if (comment || isspace(c))
-			continue;
-		if (c == '#' || c == ';') {
-			comment = 1;
-			continue;
-		}
-		if (c == '[') {
-			baselen = get_base_var(var);
-			if (baselen <= 0)
+			if (!isalpha(c))
+				break;
+			var[baselen] = tolower(c);
+			if (get_value(fn, data, var, baselen+1) < 0)
 				break;
-			var[baselen++] = '.';
-			var[baselen] = 0;
-			continue;
 		}
-		if (!isalpha(c))
-			break;
-		var[baselen] = tolower(c);
-		if (get_value(fn, data, var, baselen+1) < 0)
-			break;
+		warning("bad config file line %d in %s", config_linenr, config_file_name);
+		comment = 1;
 	}
-	die("bad config file line %d in %s", config_linenr, config_file_name);
+	return -1;
 }
 
 static int parse_unit_factor(const char *end, unsigned long *val)
diff --git a/t/t1303-wacky-config.sh b/t/t1303-wacky-config.sh
index 080117c..0599d9f 100755
--- a/t/t1303-wacky-config.sh
+++ b/t/t1303-wacky-config.sh
@@ -44,7 +44,8 @@ LONG_VALUE=$(printf "x%01021dx a" 7)
 test_expect_success 'do not crash on special long config line' '
 	setup &&
 	git config section.key "$LONG_VALUE" &&
-	check section.key "fatal: bad config file line 2 in .git/config"
+	check section.key "warning: bad config file line 2 in .git/config
+warning: bad config file line 2 in .git/config"
 '
 
 test_done
-- 
1.6.4.2.395.ge3d52

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