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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Junio C Hamano <gitster@xxxxxxxxx> writes:

> Siddharth Goel <siddharth98391@xxxxxxxxx> writes:
>
>> Helped-by: Eric Sunshine <sunshine@xxxxxxxxxxxxxx>
>> Signed-off-by: Siddharth Goel <siddharth98391@xxxxxxxxx>
>> ---
>> 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)
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]