[PATCH] rewrite skip_prefix() as loop

[PATCH] rewrite skip_prefix() as loop

[Faiz Kothari]

From: Faiz Kothari <faiz.off93@gmail.com>


Signed-off-by: Faiz Kothari <django@dj-pc.(none)>
---
 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

Re: [PATCH] rewrite skip_prefix() as loop

[Junio C Hamano]

Faiz Kothari <faiz.off93@gmail.com> writes:

> From: Faiz Kothari <faiz.off93@gmail.com>

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

>
>
> Signed-off-by: Faiz Kothari <django@dj-pc.(none)>

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.