threads / patch / 35972

patchRewrite git-compat-util.h:skip_prefix() as a loop

Subject: [PATCH] Rewrite git-compat-util.h:skip_prefix() as a loop

## tl;dr

5 messages between Feb 27, 2014 and Feb 28, 2014. Diffs are folded; open one to read it.

replies: 4people: 4as markdown or json

Sun He· Feb 27, 2014, 12:13 UTC · lore

Signed-off-by: Sun He <sunheehnus@gmail.com>
---
 git-compat-util.h |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

Show changes to git-compat-util.h +2 −2

diff --git a/git-compat-util.h b/git-compat-util.h
index cbd86c3..4daa6cf 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -357,8 +357,8 @@ extern int suffixcmp(const char *str, const char *suffix);
 
 static inline const char *skip_prefix(const char *str, const char *prefix)
 {
-	size_t len = strlen(prefix);
-	return strncmp(str, prefix, len) ? NULL : str + len;
+    while( *prefix != '\0' && *str++ == *prefix++ );
+    return *prefix == '\0' ? str : NULL;
 }
 
 #if defined(NO_MMAP) || defined(USE_WIN32_MMAP)

-- 
1.7.1

Junio C Hamano· Feb 27, 2014, 19:35 UTC · re: Sun He · lore

Re: [PATCH] Rewrite git-compat-util.h:skip_prefix() as a loop

Sun He <sunheehnus@gmail.com> writes:

Show 17 quoted lines

> Signed-off-by: Sun He <sunheehnus@gmail.com>
> ---
>  git-compat-util.h |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/git-compat-util.h b/git-compat-util.h
> index cbd86c3..4daa6cf 100644
> --- a/git-compat-util.h
> +++ b/git-compat-util.h
> @@ -357,8 +357,8 @@ extern int suffixcmp(const char *str, const char *suffix);
>  
>  static inline const char *skip_prefix(const char *str, const char *prefix)
>  {
> -	size_t len = strlen(prefix);
> -	return strncmp(str, prefix, len) ? NULL : str + len;
> +    while( *prefix != '\0' && *str++ == *prefix++ );
> +    return *prefix == '\0' ? str : NULL;

Documentation/CodingGuidelines?

>  }
>  
>  #if defined(NO_MMAP) || defined(USE_WIN32_MMAP)

David Kastrup· Feb 27, 2014, 20:33 UTC · re: Junio C Hamano · lore

Re: [PATCH] Rewrite git-compat-util.h:skip_prefix() as a loop

Junio C Hamano <gitster@pobox.com> writes:

Show 21 quoted lines

> Sun He <sunheehnus@gmail.com> writes:
>
>> Signed-off-by: Sun He <sunheehnus@gmail.com>
>> ---
>>  git-compat-util.h |    4 ++--
>>  1 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/git-compat-util.h b/git-compat-util.h
>> index cbd86c3..4daa6cf 100644
>> --- a/git-compat-util.h
>> +++ b/git-compat-util.h
>> @@ -357,8 +357,8 @@ extern int suffixcmp(const char *str, const char *suffix);
>>  
>>  static inline const char *skip_prefix(const char *str, const char *prefix)
>>  {
>> -	size_t len = strlen(prefix);
>> -	return strncmp(str, prefix, len) ? NULL : str + len;
>> +    while( *prefix != '\0' && *str++ == *prefix++ );
>> +    return *prefix == '\0' ? str : NULL;
>
> Documentation/CodingGuidelines?

Mostly relevant for tabification here, not helping much otherwise. In particular, does not contain the advice "empty statements should appear on a line of their own" which would help with readability here.

-- 
David Kastrup

Junio C Hamano· Feb 27, 2014, 22:16 UTC · re: David Kastrup · lore

Re: [PATCH] Rewrite git-compat-util.h:skip_prefix() as a loop

David Kastrup <dak@gnu.org> writes:

Show 25 quoted lines

> Junio C Hamano <gitster@pobox.com> writes:
>
>> Sun He <sunheehnus@gmail.com> writes:
>>
>>> Signed-off-by: Sun He <sunheehnus@gmail.com>
>>> ---
>>>  git-compat-util.h |    4 ++--
>>>  1 files changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/git-compat-util.h b/git-compat-util.h
>>> index cbd86c3..4daa6cf 100644
>>> --- a/git-compat-util.h
>>> +++ b/git-compat-util.h
>>> @@ -357,8 +357,8 @@ extern int suffixcmp(const char *str, const char *suffix);
>>>  
>>>  static inline const char *skip_prefix(const char *str, const char *prefix)
>>>  {
>>> -	size_t len = strlen(prefix);
>>> -	return strncmp(str, prefix, len) ? NULL : str + len;
>>> +    while( *prefix != '\0' && *str++ == *prefix++ );
>>> +    return *prefix == '\0' ? str : NULL;
>>
>> Documentation/CodingGuidelines?
>
> Mostly relevant for tabification here, not helping much otherwise.

"Imitate existing code" would let you spot that we have SP outside the () pair, not inside, for controls like while/for/if, and we usually do not explicitly compare things with 0, NULL or '\0'.

Together with the "empty statement should occupy its own line" you mentioned, I tend to agree with you that some people may benefit from them explicitly spelled out.

Jeff King· Feb 28, 2014, 06:17 UTC · re: David Kastrup · lore

Re: [PATCH] Rewrite git-compat-util.h:skip_prefix() as a loop

On Thu, Feb 27, 2014 at 09:33:45PM +0100, David Kastrup wrote:

Show 18 quoted lines

> >> diff --git a/git-compat-util.h b/git-compat-util.h
> >> index cbd86c3..4daa6cf 100644
> >> --- a/git-compat-util.h
> >> +++ b/git-compat-util.h
> >> @@ -357,8 +357,8 @@ extern int suffixcmp(const char *str, const char *suffix);
> >>  
> >>  static inline const char *skip_prefix(const char *str, const char *prefix)
> >>  {
> >> -	size_t len = strlen(prefix);
> >> -	return strncmp(str, prefix, len) ? NULL : str + len;
> >> +    while( *prefix != '\0' && *str++ == *prefix++ );
> >> +    return *prefix == '\0' ? str : NULL;
> >
> > Documentation/CodingGuidelines?
> 
> Mostly relevant for tabification here, not helping much otherwise.  In
> particular, does not contain the advice "empty statements should appear
> on a line of their own" which would help with readability here.

Also whitespace in the "while", which I could not find mentioned in CodingGuidelines either. Maybe:

-- >8 --
Subject: [PATCH] CodingGuidelines: mention C whitespace rules

We are fairly consistent about these, so most are covered by "follow existing style", but it doesn't hurt to be explicit.

Signed-off-by: Jeff King <peff@peff.net>
---
 Documentation/CodingGuidelines | 11 +++++++++++
 1 file changed, 11 insertions(+)

Show changes to Documentation/CodingGuidelines +11 −0

diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines
index ef67b53..ed432a8 100644
--- a/Documentation/CodingGuidelines
+++ b/Documentation/CodingGuidelines
@@ -126,6 +126,17 @@ For C programs:
    "char * string".  This makes it easier to understand code
    like "char *string, c;".
 
+ - Use whitespace around operators and keywords, but not inside
+   parentheses and not around functions. So:
+
+        while (condition)
+		func(bar + 1);
+
+   and not:
+
+        while( condition )
+		func (bar+1);
+
  - We avoid using braces unnecessarily.  I.e.
 
 	if (bla) {

-- 
1.8.5.2.500.g8060133