{"thread":{"id":"46434","subject":"[PATCH] refs: use skip_prefix() in ref_is_hidden()","startedAt":"2017-07-21T09:58:35Z","lastAt":"2017-07-21T16:59:16Z","messageCount":2,"participants":["Christian Couder","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"324856","messageId":"20170721095757.28373-1-chriscool@tuxfamily.org","threadId":"46434","inReplyTo":null,"subject":"[PATCH] refs: use skip_prefix() in ref_is_hidden()","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2017-07-21T09:57:57Z","receivedAt":"2017-07-21T09:58:35Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"This saves one line, makes the code a bit easier to understand\nand perhaps a bit faster too.\n\nSigned-off-by: Christian Couder <chriscool@tuxfamily.org>\n---\n refs.c | 7 +++----\n 1 file changed, 3 insertions(+), 4 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex ba22f4acef..15cb36d426 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -1160,7 +1160,7 @@ int ref_is_hidden(const char *refname, const char *refname_full)\n \t\tconst char *match = hide_refs->items[i].string;\n \t\tconst char *subject;\n \t\tint neg = 0;\n-\t\tint len;\n+\t\tconst char *p;\n \n \t\tif (*match == '!') {\n \t\t\tneg = 1;\n@@ -1175,10 +1175,9 @@ int ref_is_hidden(const char *refname, const char *refname_full)\n \t\t}\n \n \t\t/* refname can be NULL when namespaces are used. */\n-\t\tif (!subject || !starts_with(subject, match))\n+\t\tif (!subject || !skip_prefix(subject, match, &p))\n \t\t\tcontinue;\n-\t\tlen = strlen(match);\n-\t\tif (!subject[len] || subject[len] == '/')\n+\t\tif (!*p || *p == '/')\n \t\t\treturn !neg;\n \t}\n \treturn 0;\n-- \n2.14.0.rc0.26.g981adb928e.dirty\n\n"},{"id":"324875","messageId":"xmqqk231hgpz.fsf@gitster.mtv.corp.google.com","threadId":"46434","inReplyTo":"20170721095757.28373-1-chriscool@tuxfamily.org","subject":"Re: [PATCH] refs: use skip_prefix() in ref_is_hidden()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-07-21T16:55:52Z","receivedAt":"2017-07-21T16:59:16Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Christian Couder <christian.couder@gmail.com> writes:\n\n> This saves one line, makes the code a bit easier to understand\n> and perhaps a bit faster too.\n>\n> Signed-off-by: Christian Couder <chriscool@tuxfamily.org>\n> ---\n>  refs.c | 7 +++----\n>  1 file changed, 3 insertions(+), 4 deletions(-)\n>\n> diff --git a/refs.c b/refs.c\n> index ba22f4acef..15cb36d426 100644\n> --- a/refs.c\n> +++ b/refs.c\n> @@ -1160,7 +1160,7 @@ int ref_is_hidden(const char *refname, const char *refname_full)\n>  \t\tconst char *match = hide_refs->items[i].string;\n>  \t\tconst char *subject;\n>  \t\tint neg = 0;\n> -\t\tint len;\n> +\t\tconst char *p;\n>  \n>  \t\tif (*match == '!') {\n>  \t\t\tneg = 1;\n> @@ -1175,10 +1175,9 @@ int ref_is_hidden(const char *refname, const char *refname_full)\n>  \t\t}\n>  \n>  \t\t/* refname can be NULL when namespaces are used. */\n> -\t\tif (!subject || !starts_with(subject, match))\n> +\t\tif (!subject || !skip_prefix(subject, match, &p))\n>  \t\t\tcontinue;\n> -\t\tlen = strlen(match);\n> -\t\tif (!subject[len] || subject[len] == '/')\n> +\t\tif (!*p || *p == '/')\n>  \t\t\treturn !neg;\n>  \t}\n>  \treturn 0;\n\nIf we were to rewrite this using skip_prefix(), I would imagine we\nwould rather write it this way:\n\n\tif (subject &&\n\t    skip_prefix(subject, match, &p) &&\n\t    (*p || *p != '/'))\n\t\treturn !neg;\n\nbecause it would be shorter and make the logic easier to follow: \n\n    We make the final decision only when \"subject\" is there, its\n    early part matches \"match\", and the match is at the slash\n    boundary (or the whole thing).\n\nThe original wanted to say the same thing, but it had to split that\nlogic into two only because it needed an assignment to \"len\" in the\nmiddle, and because doing an assignment inside a conditional part of\nif() statement is frowned upon.\n\n\n\n"}]}