From: Max Horn <max@quendi.de>
Date: Mon, 03 Mar 2014 22:59:20 GMT
Subject: Re: [PATCH V2] commit.c: Use skip_prefix() instead of starts_with()
Message-ID: <176A3D83-AFFB-4EF4-A1CB-3A953F692166@quendi.de>
In-Reply-To: <xmqqiorvoyoo.fsf@gitster.dls.corp.google.com>


On 03.03.2014, at 20:43, Junio C Hamano <gitster@pobox.com> wrote:

> Tanay Abhra <tanayabh@gmail.com> writes:
> 
>> @@ -1193,9 +1194,9 @@ static void parse_gpg_output(struct signature_check *sigc)
>> 	for (i = 0; i < ARRAY_SIZE(sigcheck_gpg_status); i++) {
>> 		const char *found, *next;
>> 
>> -		if (starts_with(buf, sigcheck_gpg_status[i].check + 1)) {
>> +		if (found = skip_prefix(buf, sigcheck_gpg_status[i].check + 1)) {
>> 			/* At the very beginning of the buffer */
>> -			found = buf + strlen(sigcheck_gpg_status[i].check + 1);
>> +			;
>> 		} else {
>> 			found = strstr(buf, sigcheck_gpg_status[i].check);
>> 			if (!found)
> 
> This hunk looks good.  It can be a separate patch but they are both
> minor changes so it is OK to have it in a single patch.

Hm, but that hunk also introduces an assignment in a conditional, and introduces an empty block. Maybe like this?


diff --git a/commit.c b/commit.c
index 6bf4fe0..0ee0725 100644
--- a/commit.c
+++ b/commit.c
@@ -1193,10 +1193,8 @@ static void parse_gpg_output(struct signature_check *sigc)
        for (i = 0; i < ARRAY_SIZE(sigcheck_gpg_status); i++) {
                const char *found, *next;

-               if (starts_with(buf, sigcheck_gpg_status[i].check + 1)) {
-                       /* At the very beginning of the buffer */
-                       found = buf + strlen(sigcheck_gpg_status[i].check + 1);
-               } else {
+               found = skip_prefix(buf, sigcheck_gpg_status[i].check + 1);
+               if (!found) {
                        found = strstr(buf, sigcheck_gpg_status[i].check);
                        if (!found)
                                continue;