git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH 2/4] config: really keep value-internal whitespace verbatim

From
DSDragan Simic <dsimic@manjaro.org>
Date
Mar 15, 2024, 13:22 UTC
Message-ID
<cd890a7015733e311e0e0a4939c539d2894e31cf.1710508691.git.dsimic@manjaro.org>
In-Reply-To
<cover.1710508691.git.dsimic@manjaro.org>

Fix a bug in function parse_value() that prevented whitespace characters (i.e. spaces and horizontal tabs) found inside configuration option values from being parsed and returned in their original form. The bug caused any number of consecutive whitespace characters to be wrongly "squashed" into the same number of space characters.

This bug was introduced back in July 2009, in commit ebdaae372b46 ("config: Keep inner whitespace verbatim").

Further investigation showed that setting a configuration value, by invoking git-config(1), converts value-internal horizontal tabs into "\t" escape sequences, which the buggy value-parsing logic in function parse_value() didn't "squash" into spaces. That's why the test included in the ebdaae37 commit passed, which presumably made the bug remain undetected for this long. On the other hand, value-internal literal horizontal tab characters, found in a configuration file edited by hand, do get "squashed" by the value-parsing logic, so the right choice was to fix this bug by making the value-internal whitespace characters preserved verbatim.

Fixes: ebdaae372b46 ("config: Keep inner whitespace verbatim")
Signed-off-by: Dragan Simic <dsimic@manjaro.org>
---
 config.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/config.c b/config.c
index a86a20cdf5cb..5072f12e62e4 100644
--- a/config.c
+++ b/config.c
@@ -817,33 +817,38 @@ static int get_next_char(struct config_source *cs)
 
 static char *parse_value(struct config_source *cs)
 {
-	int quote = 0, comment = 0, space = 0;
+	int quote = 0, comment = 0;
+	size_t trim_len = 0;
 
 	strbuf_reset(&cs->value);
 	for (;;) {
 		int c = get_next_char(cs);
 		if (c == '\n') {
 			if (quote) {
 				cs->linenr--;
 				return NULL;
 			}
+			if (trim_len)
+				strbuf_setlen(&cs->value, trim_len);
 			return cs->value.buf;
 		}
 		if (comment)
 			continue;
 		if (isspace(c) && !quote) {
+			if (!trim_len)
+				trim_len = cs->value.len;
 			if (cs->value.len)
-				space++;
+				strbuf_addch(&cs->value, c);
 			continue;
 		}
 		if (!quote) {
 			if (c == ';' || c == '#') {
 				comment = 1;
 				continue;
 			}
 		}
-		for (; space; space--)
-			strbuf_addch(&cs->value, ' ');
+		if (trim_len)
+			trim_len = 0;
 		if (c == '\\') {
 			c = get_next_char(cs);
 			switch (c) {
Previous: Dragan SimicNext: Junio C Hamano
Message 8 of 11 in “Fix a bug in configuration parsing, and improve tests and documentation”
  1. 0/4 Fix a bug in configuration parsing, and improve tests and documentationDragan Simic, Mar 15, 2024
  2. 1/4 config: minor addition of whitespaceDragan Simic, Mar 15, 2024
  3. 3/4 t1300: add more tests for whitespace and inline commentsDragan Simic, Mar 15, 2024
  4. Eric SunshineMar 15, 2024
  5. Dragan SimicMar 15, 2024
  6. Eric SunshineMar 15, 2024
  7. Dragan SimicMar 15, 2024
  8. 2/4 config: really keep value-internal whitespace verbatimDragan Simic, Mar 15, 2024
  9. Junio C HamanoMar 15, 2024
  10. Dragan SimicMar 15, 2024
  11. 4/4 config.txt: describe handling of whitespace furtherDragan Simic, Mar 15, 2024

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.