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

[Hans Jerry Illikainen <hji@dyntopia.com>]

On Mon, Nov 18 2019, Junio C Hamano wrote:
> I wonder if it is simpler to define it next to 'next'.  Yes, this
> new variable is used only within this block, but it also gets used
> only in conjunction with that other variable.

Done in v3 (sorry for the messed up versioning going from v0 to v2!).

> A bigger question is, when this happens, what value do we want to
> leave in sigc->primary_key_fingerprint?  As we can see from the
> original code that makes sure the old value in the field will not
> leak by first free()ing, it seems that it is possible in this code
> that the field may not be NULL, but we just saw that on _our_
> signature verification system, the primary key is not available.
> Shouldn't we be nulling it out, after free()ing possibly leftover
> value in the field?

I investigated the code paths to `primary_key_fingerprint` and deduced
that it's only ever touched when GPG_STATUS_FINGERPRINT is encountered
and a primary fingerprint is extracted.  However, v3 will NULL it even
when no primary fingerprint is found.

>> +							xmemdupz(line,
>> +								 next - line);
>> +					}
>
> Avoid such an unnatural line-wrapping that makes the result harder
> to read.

Sorry about that!  I figured that some projects prefer to always trust
in the code formatter; so I just left it be.  Now I know that human
decisions are allowed :)

> 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.