[PATCH] fix keystrlen when no keyid-format option has been given

Neal H. Walfield neal at walfield.org
Wed Jan 6 11:13:06 CET 2016


Hi Werner,

On Thu, 10 Dec 2015 17:29:58 +0100,
Werner Koch wrote:
> On Wed,  9 Dec 2015 19:01, dkg at fifthhorseman.net said:
> > * g10/keyid.c (keystrlen): if opt.keyid_format has not been set,
> >   default to 0xLONG, as is done in format_keyid.
> 
> > gpgv: Ohhhh jeeee: ... this is a bug (keyid.c:342:keystrlen)
> 
> Neal, why did you introduced the KF_DEFAULT at all?  I can't immediately
> the the reason.

In a052c30, I introduced format_keyid, which is a generalized keystr
and changed keystr to call it (passing OPT.KEYID_FORMAT as the desired
format).

In the next commit (11ec478), I use the ability to specify the format
in keydb_search_desc_dump to format the search descriptions.  Clearly,
if the search description is a long key id, we should print out a long
key id even if OPT.KEYID_FORMAT is KF_SHORT.

I'm not entirely sure why I introduced KF_DEFAULT.  It currently isn't
used.  I suspect my thinking was that it would be useful to pass
KF_DEFAULT to format_keyid instead of OPT.KEYID_FORMAT if the default
format is preferred, but I didn't bother to do that in keystr.

> While looking at the code I also wonder why we do this:
> 
>       if (keyid[0])
> 	snprintf (buffer, len, "%08lX%08lX",
>                   (ulong)keyid[0], (ulong)keyid[1]);
>       else
> 	snprintf (buffer, len, "%08lX", (ulong)keyid[1]);
>       break;
> 
> This break the column alignment for keyis 0x00000000xxxxxxxxx . Granted,
> this is a rare case but why should be do that at all, the commit message
> from 2004 says:
> 
>     * keyid.c (keystr): If printing a keyid that lacks the high 4 bytes, print
>     the low 4 alone. (keystr_from_desc): Handle short keyids and warn on v3
>     fingerprints.
> 
> However, last year I fixed the indentation in some messages to match the
> current keyid format - this requires that keyid has a constant length.

I wondered this myself and conservatively left the code alone.

:) Neal



More information about the Gnupg-devel mailing list