Subject: Re: [PATCH 1/1] gpg-interface: limit search for primary key fingerprint

From: Junio C Hamano <[email-masked]>
Date: Fri, 22 Nov 2019 12:44:34 +0900

Junio C Hamano <[email-masked]> writes:

> Hans Jerry Illikainen <[email-masked]> writes:
>
>> On Mon, Nov 18 2019, Junio C Hamano wrote:
>> ...
>>> A short helper
>>>
>>> 	static void replace_cstring(const char **field,
>>> 				    const char *line, const char *next)
>>> 	{
>>> 		free(*field);
>>> 		if (line && next)
>>> 			*field = xmemdupz(line, next - line);
>>> 		else
>>> 			*field = NULL;
>>> 	}
>>>
>>> may have quite a lot of uses in this function, not only for this
>>> field.
>>
>> Implemented.  I wasn't sure whether to do it in a separate commit or
>> not, but #git-devel suggested that I do; so that's what I did.
>
> If such a refactoring helped the readability of _existing_ code that
> can also use this helper, then I agree it is the right approach to
> make that into a separate prelimimary commit.

I did not expect the "how about doing it along this line...?"
suggestion written in my MUA without even compilation testing would
work well, and acually I do not think the above would work without
further tweaks on the types.  Wouldn't some of the fields this
helper works on be of type "char *"?

Perhaps something like this squashed in...

 gpg-interface.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/gpg-interface.c b/gpg-interface.c
index 4269937b83..b481b0811b 100644
--- a/gpg-interface.c
+++ b/gpg-interface.c
@@ -105,7 +105,7 @@ static struct {
 	{ 0, "VALIDSIG ", GPG_STATUS_FINGERPRINT },
 };
 
-static void replace_cstring(const char **field, const char *line,
+static void replace_cstring(char **field, const char *line,
 			    const char *next)
 {
 	free(*field);
@@ -120,7 +120,6 @@ static void parse_gpg_output(struct signature_check *sigc)
 {
 	const char *buf = sigc->gpg_status;
 	const char *line, *next, *limit;
-	const char **field;
 	int i, j;
 	int seen_exclusive_status = 0;
 
@@ -158,6 +157,8 @@ static void parse_gpg_output(struct signature_check *sigc)
 				}
 				/* Do we have fingerprint? */
 				if (sigcheck_gpg_status[i].flags & GPG_STATUS_FINGERPRINT) {
+					char **field;
+
 					next = strchrnul(line, ' ');
 					replace_cstring(&sigc->fingerprint, line, next);