Skip to contentSkip to conversation

diff: parse ws-error-highlight option more strictly

PATCH In review René Scharfe 2 participants 2 messages Jun 15, 2016

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 diff: parse ws-error-highlight option more strictly

Check if a matched token is followed by a delimiter before advancing the pointer arg. This avoids accepting composite words like "allnew" or "defaultcontext".

---
 diff.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)
diff.c +6 -1
index 87b16d5..0f17ec5 100644
@@ -3653,7 +3653,12 @@ static void enable_patch_output(int *fmt) {
 
 static int parse_one_token(const char **arg, const char *token)
 {
-	return skip_prefix(*arg, token, arg) && (!**arg || **arg == ',');
+	const char *rest;
+	if (skip_prefix(*arg, token, &rest) && (!*rest || *rest == ',')) {
+		*arg = rest;
+		return 1;
+	}
+	return 0;
 }
 
 static int parse_ws_error_highlight(struct diff_options *opt, const char *arg)
Signature · 1 line
2.4.4
#1 lore raw
Quoted 15 lines · René Scharfe

Check if a matched token is followed by a delimiter before advancing the pointer arg. This avoids accepting composite words like "allnew" or "defaultcontext".

---
 diff.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)
diff.c +0 -0
index 87b16d5..0f17ec5 100644
@@ -3653,7 +3653,12 @@ static void enable_patch_output(int *fmt) {
 

diff.c static void enable_patch_output(int *fmt) { · line 3662

Quoted 10 lines
diff.c +6 -1
 static int parse_one_token(const char **arg, const char *token)
 {
-	return skip_prefix(*arg, token, arg) && (!**arg || **arg == ',');
+	const char *rest;
+	if (skip_prefix(*arg, token, &rest) && (!*rest || *rest == ',')) {
+		*arg = rest;
+		return 1;
+	}
+	return 0;
 }

So the bug is, when fed "allnew", calls to this function are done with "none" and "default" (both of which fail skip_prefix()), then with "all", at which point skip_prefix() advance the *arg pointer to "allnew"+3 (i.e. pointing at 'n') and the check on the "are we at the end of token?" fails. The next call is for "new" and it incorrectly passes.

Thanks for spotting.

An unrelated tangent, but I misnamed this function grossly, it seems. Even if this is a file-scope static, it is selfish to claim that this function is the only one that will ever parse any kind of token in this file. As this is a helper that can be used to parse any string with comma-separated tokens, I should have named it with some word that hints that fact.