Re: [PATCH] rewrite skip_prefix() as loop

2014-02-27 Thread Junio C Hamano
Faiz Kothari  writes:

> From: Faiz Kothari 

Notice that this matches From: in your e-mail message, which means
it is unnecessary.  Drop it.

>
>
> Signed-off-by: Faiz Kothari 

And make sure this matches how you call yourself above.

> ---
>  git-compat-util.h |7 +--
>  1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/git-compat-util.h b/git-compat-util.h
> index cbd86c3..bb2582a 100644
> --- a/git-compat-util.h
> +++ b/git-compat-util.h
> @@ -357,8 +357,11 @@ 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;
> + for (; ; str++, prefix++)
> + if (!*prefix)
> + return str;//code same as strbuf.c:starts_with()

Drop that comment.  As already has been pointed out, say that in the
proposed commit log message, perhaps like this:

Subject: skip_prefix(): rewrite as a loop

Instead of letting strlen() to scan the prefix once and then
strncmp() to scan it again, rewrite it as a single loop,
using the logic similar to starts_with() in strbuf.c.

Signed-off-by: ...

> + else if (*str != *prefix)
> + return NULL;
>  }

This for() loop that does not have a loop-control condition looks a
bit weird, though.  If we were to use for() that is not "for (;;)",
it would be more natural to write something like this:

for (; *prefix && *str == *prefix; prefix++, str++)
; /* keep going while they match */
... decide what to return here ...

but that would make you check *prefix/*str twice for the final
round, so it is not very optimal.  If we want to say "the loop is
endless and we exit explicitly from inside with our own logic", then

while (1) {
... what you have inside the loop ...
prefix++;
str++;
}

would be easier on the eyes (you can do s/while (1)/for (;;)/, too).

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] rewrite skip_prefix() as loop

2014-02-27 Thread Faiz Kothari
From: Faiz Kothari 


Signed-off-by: Faiz Kothari 
---
 git-compat-util.h |7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/git-compat-util.h b/git-compat-util.h
index cbd86c3..bb2582a 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -357,8 +357,11 @@ 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;
+   for (; ; str++, prefix++)
+   if (!*prefix)
+   return str;//code same as strbuf.c:starts_with()
+   else if (*str != *prefix)
+   return NULL;
 }
 
 #if defined(NO_MMAP) || defined(USE_WIN32_MMAP)
-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html