{"thread":{"id":"35972","subject":"[PATCH] Rewrite git-compat-util.h:skip_prefix() as a loop","startedAt":"2014-02-27T12:13:17Z","lastAt":"2014-02-28T06:17:25Z","messageCount":5,"participants":["Sun He","Junio C Hamano","David Kastrup","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"235465","messageId":"1393503197-29669-1-git-send-email-sunheehnus@gmail.com","threadId":"35972","inReplyTo":null,"subject":"[PATCH] Rewrite git-compat-util.h:skip_prefix() as a loop","fromName":"Sun He","fromEmail":"sunheehnus@gmail.com","sentAt":"2014-02-27T12:13:17Z","receivedAt":"2014-02-27T12:13:17Z","isPatch":true,"sender":{"key":"sunheehnus@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2889804?v=4"},"body":"\nSigned-off-by: Sun He \n---\n git-compat-util.h | 4 ++--\n 1 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/git-compat-util.h b/git-compat-util.h\nindex cbd86c3..4daa6cf 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -357,8 +357,8 @@ 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+ while( *prefix != '\\0' && *str++ == *prefix++ );\n+ return *prefix == '\\0' ? str : NULL;\n }\n \n #if defined(NO_MMAP) || defined(USE_WIN32_MMAP)\n-- \n1.7.1\n"},{"id":"235493","messageId":"xmqqd2i8z6um.fsf@gitster.dls.corp.google.com","threadId":"35972","inReplyTo":"1393503197-29669-1-git-send-email-sunheehnus@gmail.com","subject":"Re: [PATCH] Rewrite git-compat-util.h:skip_prefix() as a loop","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-02-27T19:35:45Z","receivedAt":"2014-02-27T19:35:45Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Sun He writes:\n\n> Signed-off-by: Sun He \n> ---\n> git-compat-util.h | 4 ++--\n> 1 files changed, 2 insertions(+), 2 deletions(-)\n>\n> diff --git a/git-compat-util.h b/git-compat-util.h\n> index cbd86c3..4daa6cf 100644\n> --- a/git-compat-util.h\n> +++ b/git-compat-util.h\n> @@ -357,8 +357,8 @@ 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> + while( *prefix != '\\0' && *str++ == *prefix++ );\n> + return *prefix == '\\0' ? str : NULL;\n\nDocumentation/CodingGuidelines?\n\n> }\n> \n> #if defined(NO_MMAP) || defined(USE_WIN32_MMAP)\n"},{"id":"235506","messageId":"87k3cg47o6.fsf@fencepost.gnu.org","threadId":"35972","inReplyTo":"xmqqd2i8z6um.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH] Rewrite git-compat-util.h:skip_prefix() as a loop","fromName":"David Kastrup","fromEmail":"dak@gnu.org","sentAt":"2014-02-27T20:33:45Z","receivedAt":"2014-02-27T20:33:45Z","isPatch":true,"sender":{"key":"dak@gnu.org","avatar":"https://avatars.githubusercontent.com/u/52141349?v=4"},"body":"Junio C Hamano writes:\n\n> Sun He writes:\n>\n>> Signed-off-by: Sun He \n>> ---\n>> git-compat-util.h | 4 ++--\n>> 1 files changed, 2 insertions(+), 2 deletions(-)\n>>\n>> diff --git a/git-compat-util.h b/git-compat-util.h\n>> index cbd86c3..4daa6cf 100644\n>> --- a/git-compat-util.h\n>> +++ b/git-compat-util.h\n>> @@ -357,8 +357,8 @@ 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>> + while( *prefix != '\\0' && *str++ == *prefix++ );\n>> + return *prefix == '\\0' ? str : NULL;\n>\n> Documentation/CodingGuidelines?\n\nMostly relevant for tabification here, not helping much otherwise. In\nparticular, does not contain the advice \"empty statements should appear\non a line of their own\" which would help with readability here.\n\n-- \nDavid Kastrup\n"},{"id":"235518","messageId":"xmqq38j4xkuq.fsf@gitster.dls.corp.google.com","threadId":"35972","inReplyTo":"87k3cg47o6.fsf@fencepost.gnu.org","subject":"Re: [PATCH] Rewrite git-compat-util.h:skip_prefix() as a loop","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-02-27T22:16:13Z","receivedAt":"2014-02-27T22:16:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"David Kastrup writes:\n\n> Junio C Hamano writes:\n>\n>> Sun He writes:\n>>\n>>> Signed-off-by: Sun He \n>>> ---\n>>> git-compat-util.h | 4 ++--\n>>> 1 files changed, 2 insertions(+), 2 deletions(-)\n>>>\n>>> diff --git a/git-compat-util.h b/git-compat-util.h\n>>> index cbd86c3..4daa6cf 100644\n>>> --- a/git-compat-util.h\n>>> +++ b/git-compat-util.h\n>>> @@ -357,8 +357,8 @@ 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>>> + while( *prefix != '\\0' && *str++ == *prefix++ );\n>>> + return *prefix == '\\0' ? str : NULL;\n>>\n>> Documentation/CodingGuidelines?\n>\n> Mostly relevant for tabification here, not helping much otherwise.\n\n\"Imitate existing code\" would let you spot that we have SP outside\nthe () pair, not inside, for controls like while/for/if, and we\nusually do not explicitly compare things with 0, NULL or '\\0'.\n\nTogether with the \"empty statement should occupy its own line\" you\nmentioned, I tend to agree with you that some people may benefit\nfrom them explicitly spelled out.\n\n\n \n"},{"id":"235539","messageId":"20140228061725.GD32556@sigill.intra.peff.net","threadId":"35972","inReplyTo":"87k3cg47o6.fsf@fencepost.gnu.org","subject":"Re: [PATCH] Rewrite git-compat-util.h:skip_prefix() as a loop","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-02-28T06:17:25Z","receivedAt":"2014-02-28T06:17:25Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Feb 27, 2014 at 09:33:45PM +0100, David Kastrup wrote:\n\n> >> diff --git a/git-compat-util.h b/git-compat-util.h\n> >> index cbd86c3..4daa6cf 100644\n> >> --- a/git-compat-util.h\n> >> +++ b/git-compat-util.h\n> >> @@ -357,8 +357,8 @@ 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> >> + while( *prefix != '\\0' && *str++ == *prefix++ );\n> >> + return *prefix == '\\0' ? str : NULL;\n> >\n> > Documentation/CodingGuidelines?\n> \n> Mostly relevant for tabification here, not helping much otherwise. In\n> particular, does not contain the advice \"empty statements should appear\n> on a line of their own\" which would help with readability here.\n\nAlso whitespace in the \"while\", which I could not find mentioned in\nCodingGuidelines either. Maybe:\n\n-- >8 --\nSubject: [PATCH] CodingGuidelines: mention C whitespace rules\n\nWe are fairly consistent about these, so most are covered by\n\"follow existing style\", but it doesn't hurt to be explicit.\n\nSigned-off-by: Jeff King \n---\n Documentation/CodingGuidelines | 11 +++++++++++\n 1 file changed, 11 insertions(+)\n\ndiff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines\nindex ef67b53..ed432a8 100644\n--- a/Documentation/CodingGuidelines\n+++ b/Documentation/CodingGuidelines\n@@ -126,6 +126,17 @@ For C programs:\n \"char * string\". This makes it easier to understand code\n like \"char *string, c;\".\n \n+ - Use whitespace around operators and keywords, but not inside\n+ parentheses and not around functions. So:\n+\n+ while (condition)\n+\t\tfunc(bar + 1);\n+\n+ and not:\n+\n+ while( condition )\n+\t\tfunc (bar+1);\n+\n - We avoid using braces unnecessarily. I.e.\n \n \tif (bla) {\n-- \n1.8.5.2.500.g8060133\n"}]}