{"thread":{"id":"35971","subject":"[PATCH] rewrite skip_prefix() as loop","startedAt":"2014-02-27T12:02:42Z","lastAt":"2014-02-27T19:34:38Z","messageCount":2,"participants":["Faiz Kothari","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"235463","messageId":"1393502562-28025-1-git-send-email-faizkothari@gmail.com","threadId":"35971","inReplyTo":null,"subject":"[PATCH] rewrite skip_prefix() as loop","fromName":"Faiz Kothari","fromEmail":"faiz.off93@gmail.com","sentAt":"2014-02-27T12:02:42Z","receivedAt":"2014-02-27T12:02:42Z","isPatch":true,"sender":{"key":"faiz.off93@gmail.com","avatar":null},"body":"From: Faiz Kothari <faiz.off93@gmail.com>\n\n\nSigned-off-by: Faiz Kothari <django@dj-pc.(none)>\n---\n git-compat-util.h |    7 +++++--\n 1 file changed, 5 insertions(+), 2 deletions(-)\n\ndiff --git a/git-compat-util.h b/git-compat-util.h\nindex cbd86c3..bb2582a 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -357,8 +357,11 @@ extern int suffixcmp(const char *str, const char *suffix);\n \n static inline const char *skip_prefix(const char *str, const char *prefix)\n {\n-\tsize_t len = strlen(prefix);\n-\treturn strncmp(str, prefix, len) ? NULL : str + len;\n+\tfor (; ; str++, prefix++)\n+\t\tif (!*prefix)\n+\t\t\treturn str;//code same as strbuf.c:starts_with()\n+\t\telse if (*str != *prefix)\n+\t\t\treturn NULL;\n }\n \n #if defined(NO_MMAP) || defined(USE_WIN32_MMAP)\n-- \n1.7.9.5\n"},{"id":"235492","messageId":"xmqqha7kz6wh.fsf@gitster.dls.corp.google.com","threadId":"35971","inReplyTo":"1393502562-28025-1-git-send-email-faizkothari@gmail.com","subject":"Re: [PATCH] rewrite skip_prefix() as loop","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-02-27T19:34:38Z","receivedAt":"2014-02-27T19:34:38Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Faiz Kothari <faiz.off93@gmail.com> writes:\n\n> From: Faiz Kothari <faiz.off93@gmail.com>\n\nNotice that this matches From: in your e-mail message, which means\nit is unnecessary.  Drop it.\n\n>\n>\n> Signed-off-by: Faiz Kothari <django@dj-pc.(none)>\n\nAnd make sure this matches how you call yourself above.\n\n> ---\n>  git-compat-util.h |    7 +++++--\n>  1 file changed, 5 insertions(+), 2 deletions(-)\n>\n> diff --git a/git-compat-util.h b/git-compat-util.h\n> index cbd86c3..bb2582a 100644\n> --- a/git-compat-util.h\n> +++ b/git-compat-util.h\n> @@ -357,8 +357,11 @@ extern int suffixcmp(const char *str, const char *suffix);\n>  \n>  static inline const char *skip_prefix(const char *str, const char *prefix)\n>  {\n> -\tsize_t len = strlen(prefix);\n> -\treturn strncmp(str, prefix, len) ? NULL : str + len;\n> +\tfor (; ; str++, prefix++)\n> +\t\tif (!*prefix)\n> +\t\t\treturn str;//code same as strbuf.c:starts_with()\n\nDrop that comment.  As already has been pointed out, say that in the\nproposed commit log message, perhaps like this:\n\n\tSubject: skip_prefix(): rewrite as a loop\n\n\tInstead of letting strlen() to scan the prefix once and then\n        strncmp() to scan it again, rewrite it as a single loop,\n        using the logic similar to starts_with() in strbuf.c.\n\n\tSigned-off-by: ...\n\n> +\t\telse if (*str != *prefix)\n> +\t\t\treturn NULL;\n>  }\n\nThis for() loop that does not have a loop-control condition looks a\nbit weird, though.  If we were to use for() that is not \"for (;;)\",\nit would be more natural to write something like this:\n\n\tfor (; *prefix && *str == *prefix; prefix++, str++)\n\t\t; /* keep going while they match */\n\t... decide what to return here ...\n\nbut that would make you check *prefix/*str twice for the final\nround, so it is not very optimal.  If we want to say \"the loop is\nendless and we exit explicitly from inside with our own logic\", then\n\n\twhile (1) {\n\t\t... what you have inside the loop ...\n\t\tprefix++;\n                str++;\n        }\n\nwould be easier on the eyes (you can do s/while (1)/for (;;)/, too).\n\nThanks.\n"}]}