{"thread":{"id":"39834","subject":"[PATCH] diff: parse ws-error-highlight option more strictly","startedAt":"2015-07-11T12:58:21Z","lastAt":"2015-07-12T16:55:17Z","messageCount":2,"participants":["René Scharfe","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"266041","messageId":"55A112ED.3090004@web.de","threadId":"39834","inReplyTo":null,"subject":"[PATCH] diff: parse ws-error-highlight option more strictly","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2015-07-11T12:58:21Z","receivedAt":"2015-07-11T12:58:21Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Check if a matched token is followed by a delimiter before advancing the\npointer arg.  This avoids accepting composite words like \"allnew\" or\n\"defaultcontext\".\n\nSigned-off-by: Rene Scharfe <l.s.r@web.de>\n---\n diff.c | 7 ++++++-\n 1 file changed, 6 insertions(+), 1 deletion(-)\n\ndiff --git a/diff.c b/diff.c\nindex 87b16d5..0f17ec5 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -3653,7 +3653,12 @@ static void enable_patch_output(int *fmt) {\n \n static int parse_one_token(const char **arg, const char *token)\n {\n-\treturn skip_prefix(*arg, token, arg) && (!**arg || **arg == ',');\n+\tconst char *rest;\n+\tif (skip_prefix(*arg, token, &rest) && (!*rest || *rest == ',')) {\n+\t\t*arg = rest;\n+\t\treturn 1;\n+\t}\n+\treturn 0;\n }\n \n static int parse_ws_error_highlight(struct diff_options *opt, const char *arg)\n-- \n2.4.4\n"},{"id":"266064","messageId":"xmqq4ml9joy2.fsf@gitster.dls.corp.google.com","threadId":"39834","inReplyTo":"55A112ED.3090004@web.de","subject":"Re: [PATCH] diff: parse ws-error-highlight option more strictly","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-07-12T16:55:17Z","receivedAt":"2015-07-12T16:55:17Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"René Scharfe <l.s.r@web.de> writes:\n\n> Check if a matched token is followed by a delimiter before advancing the\n> pointer arg.  This avoids accepting composite words like \"allnew\" or\n> \"defaultcontext\".\n>\n> Signed-off-by: Rene Scharfe <l.s.r@web.de>\n> ---\n>  diff.c | 7 ++++++-\n>  1 file changed, 6 insertions(+), 1 deletion(-)\n>\n> diff --git a/diff.c b/diff.c\n> index 87b16d5..0f17ec5 100644\n> --- a/diff.c\n> +++ b/diff.c\n> @@ -3653,7 +3653,12 @@ static void enable_patch_output(int *fmt) {\n>  \n>  static int parse_one_token(const char **arg, const char *token)\n>  {\n> -\treturn skip_prefix(*arg, token, arg) && (!**arg || **arg == ',');\n> +\tconst char *rest;\n> +\tif (skip_prefix(*arg, token, &rest) && (!*rest || *rest == ',')) {\n> +\t\t*arg = rest;\n> +\t\treturn 1;\n> +\t}\n> +\treturn 0;\n>  }\n\nSo the bug is, when fed \"allnew\", calls to this function are done\nwith \"none\" and \"default\" (both of which fail skip_prefix()), then\nwith \"all\", at which point skip_prefix() advance the *arg pointer to\n\"allnew\"+3 (i.e. pointing at 'n') and the check on the \"are we at\nthe end of token?\" fails. The next call is for \"new\" and it\nincorrectly passes.\n\nThanks for spotting.\n\nAn unrelated tangent, but I misnamed this function grossly, it\nseems.  Even if this is a file-scope static, it is selfish to claim\nthat this function is the only one that will ever parse any kind of\ntoken in this file.  As this is a helper that can be used to parse\nany string with comma-separated tokens, I should have named it with\nsome word that hints that fact.\n"}]}