Re: [PATCH v3] skip_prefix: rewrite so that prefix is scanned once

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 3, 2014, 22:43 UTC
Message-ID: <xmqq61nuoqd5.fsf@gitster.dls.corp.google.com>
In-Reply-To: <xmqqvbvvp0gj.fsf@gitster.dls.corp.google.com>

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

Show 42 quoted lines

> Siddharth Goel <siddharth98391@gmail.com> writes:
>
>> Helped-by: Eric Sunshine <sunshine@sunshineco.com>
>> Signed-off-by: Siddharth Goel <siddharth98391@gmail.com>
>> ---
>> Added a space after colon in the subject as compared to previous 
>> patch [PATCH v2].
>>
>> [PATCH v2]: http://thread.gmane.org/gmane.comp.version-control.git/243150
>
> Whenever you see "Change", "Rewrite", etc. in the subject of a patch
> that touches existing code, think twice.  The subject line is a
> scarce real estate to be wasted on a noiseword that carries no real
> information, and we already know a patch that touches existing code
> changes or rewrites things.
>
>     Subject: [PATCH v3] skip_prefix: scan prefix only once
>
> perhaps?
>
>>  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 614a5e9..550dce3 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;
>> +	while (*prefix != '\0' && *str == *prefix) {
>> +		str++;
>> +		prefix++;
>> +	}
>> +	return (*prefix == '\0' ? str : NULL);
>
> Unlike another patch I saw the other day on the same topic, this
> checks *prefix twice for the last round, even though I think this
> one is probably slightly easier to read.  I dunno.

That is, something like this instead. After looking at it again, I do not think it is less readable than the above.

 git-compat-util.h | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/git-compat-util.h b/git-compat-util.h
index cbd86c3..68ffaef 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -357,8 +357,14 @@ 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 (1) {
+		if (!*prefix)
+			return str;
+		if (*str != *prefix)
+			return NULL;
+		prefix++;
+		str++;
+	}
 }
 
 #if defined(NO_MMAP) || defined(USE_WIN32_MMAP)