git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / mirror / code / Atom feed
* [PATCH] diff: parse ws-error-highlight option more strictly
@ 2015-07-11 12:58 René Scharfe
  2015-07-12 16:55 ` Junio C Hamano
  0 siblings, 1 reply; 2+ messages in thread
From: René Scharfe @ 2015-07-11 12:58 UTC (permalink / raw)
  To: Git Mailing List; +Cc: Junio C Hamano

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

Signed-off-by: Rene Scharfe <l.s.r@web.de>
---
 diff.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/diff.c b/diff.c
index 87b16d5..0f17ec5 100644
--- a/diff.c
+++ b/diff.c
@@ -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)
-- 
2.4.4

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH] diff: parse ws-error-highlight option more strictly
  2015-07-11 12:58 [PATCH] diff: parse ws-error-highlight option more strictly René Scharfe
@ 2015-07-12 16:55 ` Junio C Hamano
  0 siblings, 0 replies; 2+ messages in thread
From: Junio C Hamano @ 2015-07-12 16:55 UTC (permalink / raw)
  To: René Scharfe; +Cc: Git Mailing List

René Scharfe <l.s.r@web.de> writes:

> Check if a matched token is followed by a delimiter before advancing the
> pointer arg.  This avoids accepting composite words like "allnew" or
> "defaultcontext".
>
> Signed-off-by: Rene Scharfe <l.s.r@web.de>
> ---
>  diff.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/diff.c b/diff.c
> index 87b16d5..0f17ec5 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -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;
>  }

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.

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2015-07-12 16:55 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-07-11 12:58 [PATCH] diff: parse ws-error-highlight option more strictly René Scharfe
2015-07-12 16:55 ` Junio C Hamano

Code repositories for project(s) associated with this inbox:

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).