{"thread":{"id":"35960","subject":"[PATCH] GSoC 2014 Microproject 1 rewrite skip_prefix() as loop","startedAt":"2014-02-26T16:46:35Z","lastAt":"2014-02-27T11:40:48Z","messageCount":3,"participants":["Faiz Kothari","Michael Haggerty"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"235388","messageId":"1393433195.4629.4.camel@dj-pc","threadId":"35960","inReplyTo":null,"subject":"[PATCH] GSoC 2014 Microproject 1 rewrite skip_prefix() as loop","fromName":"Faiz Kothari","fromEmail":"faiz.off93@gmail.com","sentAt":"2014-02-26T16:46:35Z","receivedAt":"2014-02-26T16:46:35Z","isPatch":true,"sender":{"key":"faiz.off93@gmail.com","avatar":null},"body":"Hi,\nI am Faiz Kothari, I am a GSoC aspirant and want to contribute to git.\nI am submitting the patch in reponse to Microproject 1,\nrewrite git-compat-util.h:skip_prefix() as a loop.\n\nSigned-off-by: Faiz Kothari <faiz.off93@gmail.com>\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\n*suffix);\n \n static inline const char *skip_prefix(const char *str, const char\n*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.9.0.1.ge8df331\n"},{"id":"235460","messageId":"530F2238.4090303@alum.mit.edu","threadId":"35960","inReplyTo":"1393433195.4629.4.camel@dj-pc","subject":"Re: [PATCH] GSoC 2014 Microproject 1 rewrite skip_prefix() as loop","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2014-02-27T11:32:08Z","receivedAt":"2014-02-27T11:32:08Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"On 02/26/2014 05:46 PM, Faiz Kothari wrote:\n> I am Faiz Kothari, I am a GSoC aspirant and want to contribute to git.\n> I am submitting the patch in reponse to Microproject 1,\n> rewrite git-compat-util.h:skip_prefix() as a loop.\n> \n> Signed-off-by: Faiz Kothari <faiz.off93@gmail.com>\n\nThe subject of your email plus the part above the \"---\" line will be\ntaken directly to be used as the commit message.  So it should not\ninclude information that is inappropriate for a commit message.\n\nYou can put such information directly below the \"---\" line.\n\nPlease also see my comments below.\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\n> *suffix);\n>  \n>  static inline const char *skip_prefix(const char *str, const char\n> *prefix)\n\nThe line above seems to have been broken by your email program.  It is\nimportant for efficiency reasons that patches be readable directly out\nof emails (e.g., by using \"git am\").  Please practice by sending the\npatch to yourself different ways until \"git am\" works on it correctly.\n\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\nWe don't use \"//\" for comments, and please space things out the way\nother code does it.  But actually, IMO this particular comment doesn't\nreally belong permanently in the code.  It rather belongs in the commit\nmessage, or in the discussion (under the \"---\"), or maybe it should be\ntaken as an indication of a deeper problem (see below).\n\n> +\t\telse if (*str != *prefix)\n> +\t\t\treturn NULL;\n>  }\n>  \n>  #if defined(NO_MMAP) || defined(USE_WIN32_MMAP)\n> \n\nThe code itself looks correct.\n\nBut, considering your comment, would it be appropriate for one of the\nfunctions to call the other?\n\nMichael\n\n-- \nMichael Haggerty\nmhagger@alum.mit.edu\nhttp://softwareswirl.blogspot.com/\n"},{"id":"235461","messageId":"CAFbjVcmbF4MYj1t2v1_dxQ0z4mqMfFChK2HdCFiEqWL6Wo2vGg@mail.gmail.com","threadId":"35960","inReplyTo":"530F2238.4090303@alum.mit.edu","subject":"Re: [PATCH] GSoC 2014 Microproject 1 rewrite skip_prefix() as loop","fromName":"Faiz Kothari","fromEmail":"faiz.off93@gmail.com","sentAt":"2014-02-27T11:40:48Z","receivedAt":"2014-02-27T11:40:48Z","isPatch":true,"sender":{"key":"faiz.off93@gmail.com","avatar":null},"body":"Thanks for the reply,\nI was unable to get git send-email working. Now its working, I'll\nresend the patch.\nI ran all the tests, they are working properly.\nAbout the comment, I meant, there is a similar function\nstrbuf.c:starts_with() which does the exact same job, but it returns 0\nor 1.\nI just changed it to return a (const char *) accordingly.\n\nOn Thu, Feb 27, 2014 at 5:02 PM, Michael Haggerty <mhagger@alum.mit.edu> wrote:\n> On 02/26/2014 05:46 PM, Faiz Kothari wrote:\n>> I am Faiz Kothari, I am a GSoC aspirant and want to contribute to git.\n>> I am submitting the patch in reponse to Microproject 1,\n>> rewrite git-compat-util.h:skip_prefix() as a loop.\n>>\n>> Signed-off-by: Faiz Kothari <faiz.off93@gmail.com>\n>\n> The subject of your email plus the part above the \"---\" line will be\n> taken directly to be used as the commit message.  So it should not\n> include information that is inappropriate for a commit message.\n>\n> You can put such information directly below the \"---\" line.\n>\n> Please also see my comments below.\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\n>> *suffix);\n>>\n>>  static inline const char *skip_prefix(const char *str, const char\n>> *prefix)\n>\n> The line above seems to have been broken by your email program.  It is\n> important for efficiency reasons that patches be readable directly out\n> of emails (e.g., by using \"git am\").  Please practice by sending the\n> patch to yourself different ways until \"git am\" works on it correctly.\n>\n>>  {\n>> -     size_t len = strlen(prefix);\n>> -     return strncmp(str, prefix, len) ? NULL : str + len;\n>> +     for (; ; str++, prefix++)\n>> +             if (!*prefix)\n>> +                     return str;//code same as strbuf.c:starts_with()\n>\n> We don't use \"//\" for comments, and please space things out the way\n> other code does it.  But actually, IMO this particular comment doesn't\n> really belong permanently in the code.  It rather belongs in the commit\n> message, or in the discussion (under the \"---\"), or maybe it should be\n> taken as an indication of a deeper problem (see below).\n>\n>> +             else if (*str != *prefix)\n>> +                     return NULL;\n>>  }\n>>\n>>  #if defined(NO_MMAP) || defined(USE_WIN32_MMAP)\n>>\n>\n> The code itself looks correct.\n>\n> But, considering your comment, would it be appropriate for one of the\n> functions to call the other?\n>\n> Michael\n>\n> --\n> Michael Haggerty\n> mhagger@alum.mit.edu\n> http://softwareswirl.blogspot.com/\n"}]}