gpg: Sort the signatures in standard key listings.

* g10/gpg.c (parse_list_options): Add "sort-sigs".
(main): Make it the default.
* g10/options.h (LIST_SORT_SIGS): New.
* g10/keylist.c (cmp_signodes): New.
(list_keyblock_print): Sort signatures and factor signature printing
code out to ...
(list_signature_print): new.
--

In particular together with --full-timestamps this makes it easier to
see the history of key signatures and their revocations.  The
self-signatures are also printed first.  To disable this

  --list-options no-sort-sigs

can be used.

Also don't print the annoying "no recocation reason specified"
message.

Signed-off-by: Werner Koch <wk@gnupg.org>

7 files changed, 247 insertions, 142 deletions

diff --git a/doc/gpg.texi b/doc/gpg.texi
index 97990a5a9..ccb0689f6 100644
--- a/doc/gpg.texi
+++ b/doc/gpg.texi
@@ -1366,6 +1366,14 @@ give the opposite meaning.  The options are:
   @opindex list-options:show-only-fpr-mbox
   For each user-id which has a valid mail address print
   only the fingerprint followed by the mail address.
+
+  @item sort-sigs
+  @opindex list-options:sort-sigs
+  With --list-sigs and --check-sigs sort the signatures by keyID and
+  creation time to make it easier to view the history of these
+  signatures.  The self-signature is also listed before other
+  signatures. Defaults to yes.
+
 @end table
 
 @item --verify-options @var{parameters}
diff --git a/g10/gpg.c b/g10/gpg.c
index c5c6f87ad..910977a6c 100644
--- a/g10/gpg.c
+++ b/g10/gpg.c
@@ -2040,6 +2040,8 @@ parse_list_options(char *str)
        NULL},
       {"show-only-fpr-mbox",LIST_SHOW_ONLY_FPR_MBOX, NULL,
        NULL},
+      {"sort-sigs", LIST_SORT_SIGS, NULL,
+       NULL},
       {NULL,0,NULL,NULL}
     };
 
@@ -2408,6 +2410,7 @@ main (int argc, char **argv)
                           | VERIFY_SHOW_STD_NOTATIONS
                           | VERIFY_SHOW_KEYSERVER_URLS);
     opt.list_options   = (LIST_SHOW_UID_VALIDITY
+                          | LIST_SORT_SIGS
                           | LIST_SHOW_USAGE);
 #ifdef NO_TRUST_MODELS
     opt.trust_model = TM_ALWAYS;
diff --git a/g10/key-clean.c b/g10/key-clean.c
index 496d0194e..9320428c8 100644
--- a/g10/key-clean.c
+++ b/g10/key-clean.c
@@ -37,8 +37,8 @@
 
 /*
  * Mark the signature of the given UID which are used to certify it.
- * To do this, we first revmove all signatures which are not valid and
- * from the remain ones we look for the latest one.  If this is not a
+ * To do this, we first remove all signatures which are not valid and
+ * from the remaining we look for the latest one.  If this is not a
  * certification revocation signature we mark the signature by setting
  * node flag bit 8.  Revocations are marked with flag 11, and sigs
  * from unavailable keys are marked with flag 12.  Note that flag bits
diff --git a/g10/keyedit.h b/g10/keyedit.h
index af5e99664..89c408b63 100644
--- a/g10/keyedit.h
+++ b/g10/keyedit.h
@@ -33,6 +33,9 @@
 #define NODFLG_SELKEY (1<<9)	/* Indicate the selected key.  */
 #define NODFLG_SELSIG (1<<10)	/* Indicate a selected signature.  */
 
+#define NODFLG_MARK_B (1<<11)   /* Temporary mark in key listing code.  */
+
+
 /*-- keyedit.c --*/
 void keyedit_menu (ctrl_t ctrl, const char *username, strlist_t locusr,
 		   strlist_t commands, int quiet, int seckey_check );
diff --git a/g10/keylist.c b/g10/keylist.c
index 9f8267e9a..cb3b61e03 100644
--- a/g10/keylist.c
+++ b/g10/keylist.c
@@ -938,14 +938,213 @@ dump_attribs (const PKT_user_id *uid, PKT_public_key *pk)
 }
 
 
+/* Order two signatures.  We first order by keyid and then by creation
+ * time.  */
+static int
+cmp_signodes (const void *av, const void *bv)
+{
+  const kbnode_t an = *(const kbnode_t *)av;
+  const kbnode_t bn = *(const kbnode_t *)bv;
+  const PKT_signature *a;
+  const PKT_signature *b;
+  int i;
+
+  /* log_assert (an->pkt->pkttype == PKT_SIGNATURE); */
+  /* log_assert (bn->pkt->pkttype == PKT_SIGNATURE); */
+
+  a = an->pkt->pkt.signature;
+  b = bn->pkt->pkt.signature;
+
+  /* Self-signatures are ordered first.  */
+  if ((an->flag & NODFLG_MARK_B) && !(bn->flag & NODFLG_MARK_B))
+    return -1;
+  if (!(an->flag & NODFLG_MARK_B) && (bn->flag & NODFLG_MARK_B))
+    return 1;
+
+  /* then the keyids.  (which are or course the same for self-sigs). */
+  i = keyid_cmp (a->keyid, b->keyid);
+  if (i)
+    return i;
+
+  /* Followed by creation time */
+  if (a->timestamp > b->timestamp)
+    return 1;
+  if (a->timestamp < b->timestamp)
+    return -1;
+
+  /* followed by the class in a way that a rev comes first.  */
+  if (a->sig_class > b->sig_class)
+    return 1;
+  if (a->sig_class < b->sig_class)
+    return -1;
+
+  /* To make the sort stable we compare the entire structure as last resort.  */
+  return memcmp (a, b, sizeof *a);
+}
+
+
+/* Helper for list_keyblock_print.  */
+static void
+list_signature_print (ctrl_t ctrl, kbnode_t keyblock, kbnode_t node,
+                      struct keylist_context *listctx)
+{
+          /* (extra indentation to keep the diff history short)  */
+	  PKT_signature *sig = node->pkt->pkt.signature;
+	  int rc, sigrc;
+	  char *sigstr;
+          char *reason_text = NULL;
+          char *reason_comment = NULL;
+          size_t reason_commentlen;
+          int reason_code = 0;
+
+	  if (listctx->check_sigs)
+	    {
+	      rc = check_key_signature (ctrl, keyblock, node, NULL);
+	      switch (gpg_err_code (rc))
+		{
+		case 0:
+		  listctx->good_sigs++;
+		  sigrc = '!';
+		  break;
+		case GPG_ERR_BAD_SIGNATURE:
+		  listctx->inv_sigs++;
+		  sigrc = '-';
+		  break;
+		case GPG_ERR_NO_PUBKEY:
+		case GPG_ERR_UNUSABLE_PUBKEY:
+		  listctx->no_key++;
+		  return;
+		default:
+		  listctx->oth_err++;
+		  sigrc = '%';
+		  break;
+		}
+
+	      /* TODO: Make sure a cached sig record here still has
+	         the pk that issued it.  See also
+	         keyedit.c:print_and_check_one_sig */
+	    }
+	  else
+	    {
+	      rc = 0;
+	      sigrc = ' ';
+	    }
+
+	  if (sig->sig_class == 0x20 || sig->sig_class == 0x28
+	      || sig->sig_class == 0x30)
+            {
+              sigstr = "rev";
+              reason_code = get_revocation_reason (sig, &reason_text,
+                                                   &reason_comment,
+                                                   &reason_commentlen);
+            }
+	  else if ((sig->sig_class & ~3) == 0x10)
+	    sigstr = "sig";
+	  else if (sig->sig_class == 0x18)
+	    sigstr = "sig";
+	  else if (sig->sig_class == 0x1F)
+	    sigstr = "sig";
+	  else
+	    {
+	      es_fprintf (es_stdout, "sig                             "
+		      "[unexpected signature class 0x%02x]\n",
+		      sig->sig_class);
+	      return;
+	    }
+
+	  es_fputs (sigstr, es_stdout);
+	  es_fprintf (es_stdout, "%c%c %c%c%c%c%c%c %s %s",
+		  sigrc, (sig->sig_class - 0x10 > 0 &&
+			  sig->sig_class - 0x10 <
+			  4) ? '0' + sig->sig_class - 0x10 : ' ',
+		  sig->flags.exportable ? ' ' : 'L',
+		  sig->flags.revocable ? ' ' : 'R',
+		  sig->flags.policy_url ? 'P' : ' ',
+		  sig->flags.notation ? 'N' : ' ',
+		  sig->flags.expired ? 'X' : ' ',
+		  (sig->trust_depth > 9) ? 'T' : (sig->trust_depth >
+						  0) ? '0' +
+		  sig->trust_depth : ' ', keystr (sig->keyid),
+		  datestr_from_sig (sig));
+	  if (opt.list_options & LIST_SHOW_SIG_EXPIRE)
+	    es_fprintf (es_stdout, " %s", expirestr_from_sig (sig));
+	  es_fprintf (es_stdout, "  ");
+	  if (sigrc == '%')
+	    es_fprintf (es_stdout, "[%s] ", gpg_strerror (rc));
+	  else if (sigrc == '?')
+	    ;
+	  else if (!opt.fast_list_mode)
+	    {
+	      size_t n;
+	      char *p = get_user_id (ctrl, sig->keyid, &n, NULL);
+	      print_utf8_buffer (es_stdout, p, n);
+	      xfree (p);
+	    }
+	  es_putc ('\n', es_stdout);
+
+	  if (sig->flags.policy_url
+	      && (opt.list_options & LIST_SHOW_POLICY_URLS))
+	    show_policy_url (sig, 3, 0);
+
+	  if (sig->flags.notation && (opt.list_options & LIST_SHOW_NOTATIONS))
+	    show_notation (sig, 3, 0,
+			   ((opt.
+			     list_options & LIST_SHOW_STD_NOTATIONS) ? 1 : 0)
+			   +
+			   ((opt.
+			     list_options & LIST_SHOW_USER_NOTATIONS) ? 2 :
+			    0));
+
+	  if (sig->flags.pref_ks
+	      && (opt.list_options & LIST_SHOW_KEYSERVER_URLS))
+	    show_keyserver_url (sig, 3, 0);
+
+          if (reason_text && (reason_code || reason_comment))
+            {
+              es_fprintf (es_stdout, "      %s%s\n",
+                          _("reason for revocation: "), reason_text);
+              if (reason_comment)
+                {
+                  const byte *s, *s_lf;
+                  size_t n, n_lf;
+
+                  s = reason_comment;
+                  n = reason_commentlen;
+                  s_lf = NULL;
+                  do
+                    {
+                      /* We don't want any empty lines, so we skip them.  */
+                      for (;n && *s == '\n'; s++, n--)
+                        ;
+                      if (n)
+                        {
+                          s_lf = memchr (s, '\n', n);
+                          n_lf = s_lf? s_lf - s : n;
+                          es_fprintf (es_stdout, "         %s",
+                                      _("revocation comment: "));
+                          es_write_sanitized (es_stdout, s, n_lf, NULL, NULL);
+                          es_putc ('\n', es_stdout);
+                          s += n_lf; n -= n_lf;
+                        }
+                    } while (s_lf);
+                }
+            }
+
+          xfree (reason_text);
+          xfree (reason_comment);
+
+	  /* fixme: check or list other sigs here */
+}
+
+
 static void
 list_keyblock_print (ctrl_t ctrl, kbnode_t keyblock, int secret, int fpr,
                      struct keylist_context *listctx)
 {
   int rc;
-  KBNODE kbctx;
-  KBNODE node;
+  kbnode_t node;
   PKT_public_key *pk;
+  u32 *mainkid;
   int skip_sigs = 0;
   char *hexgrip = NULL;
   char *serialno = NULL;
@@ -960,6 +1159,7 @@ list_keyblock_print (ctrl_t ctrl, kbnode_t keyblock, int secret, int fpr,
     }
 
   pk = node->pkt->pkt.public_key;
+  mainkid = pk_keyid (pk);
 
   if (secret || opt.with_keygrip)
     {
@@ -1012,9 +1212,11 @@ list_keyblock_print (ctrl_t ctrl, kbnode_t keyblock, int secret, int fpr,
       es_putc ('\n', es_stdout);
     }
 
-
-  for (kbctx = NULL; (node = walk_kbnode (keyblock, &kbctx, 0));)
+  for (node = keyblock; node; node = node->next)
     {
+      if (is_deleted_kbnode (node))
+        continue;
+
       if (node->pkt->pkttype == PKT_USER_ID)
 	{
 	  PKT_user_id *uid = node->pkt->pkt.user_id;
@@ -1145,149 +1347,37 @@ list_keyblock_print (ctrl_t ctrl, kbnode_t keyblock, int secret, int fpr,
       else if (opt.list_sigs
 	       && node->pkt->pkttype == PKT_SIGNATURE && !skip_sigs)
 	{
-	  PKT_signature *sig = node->pkt->pkt.signature;
-	  int sigrc;
-	  char *sigstr;
-          char *reason_text = NULL;
-          char *reason_comment = NULL;
-          size_t reason_commentlen;
-
-	  if (listctx->check_sigs)
-	    {
-	      rc = check_key_signature (ctrl, keyblock, node, NULL);
-	      switch (gpg_err_code (rc))
-		{
-		case 0:
-		  listctx->good_sigs++;
-		  sigrc = '!';
-		  break;
-		case GPG_ERR_BAD_SIGNATURE:
-		  listctx->inv_sigs++;
-		  sigrc = '-';
-		  break;
-		case GPG_ERR_NO_PUBKEY:
-		case GPG_ERR_UNUSABLE_PUBKEY:
-		  listctx->no_key++;
-		  continue;
-		default:
-		  listctx->oth_err++;
-		  sigrc = '%';
-		  break;
-		}
-
-	      /* TODO: Make sure a cached sig record here still has
-	         the pk that issued it.  See also
-	         keyedit.c:print_and_check_one_sig */
-	    }
-	  else
-	    {
-	      rc = 0;
-	      sigrc = ' ';
-	    }
-
-	  if (sig->sig_class == 0x20 || sig->sig_class == 0x28
-	      || sig->sig_class == 0x30)
+          if ((opt.list_options & LIST_SORT_SIGS))
             {
-              sigstr = "rev";
-              get_revocation_reason (sig, &reason_text,
-                                     &reason_comment, &reason_commentlen);
-            }
-	  else if ((sig->sig_class & ~3) == 0x10)
-	    sigstr = "sig";
-	  else if (sig->sig_class == 0x18)
-	    sigstr = "sig";
-	  else if (sig->sig_class == 0x1F)
-	    sigstr = "sig";
-	  else
-	    {
-	      es_fprintf (es_stdout, "sig                             "
-		      "[unexpected signature class 0x%02x]\n",
-		      sig->sig_class);
-	      continue;
-	    }
+              kbnode_t n;
+              unsigned int sigcount = 0;
+              kbnode_t *sigarray;
+              unsigned int idx;
 
-	  es_fputs (sigstr, es_stdout);
-	  es_fprintf (es_stdout, "%c%c %c%c%c%c%c%c %s %s",
-		  sigrc, (sig->sig_class - 0x10 > 0 &&
-			  sig->sig_class - 0x10 <
-			  4) ? '0' + sig->sig_class - 0x10 : ' ',
-		  sig->flags.exportable ? ' ' : 'L',
-		  sig->flags.revocable ? ' ' : 'R',
-		  sig->flags.policy_url ? 'P' : ' ',
-		  sig->flags.notation ? 'N' : ' ',
-		  sig->flags.expired ? 'X' : ' ',
-		  (sig->trust_depth > 9) ? 'T' : (sig->trust_depth >
-						  0) ? '0' +
-		  sig->trust_depth : ' ', keystr (sig->keyid),
-		  datestr_from_sig (sig));
-	  if (opt.list_options & LIST_SHOW_SIG_EXPIRE)
-	    es_fprintf (es_stdout, " %s", expirestr_from_sig (sig));
-	  es_fprintf (es_stdout, "  ");
-	  if (sigrc == '%')
-	    es_fprintf (es_stdout, "[%s] ", gpg_strerror (rc));
-	  else if (sigrc == '?')
-	    ;
-	  else if (!opt.fast_list_mode)
-	    {
-	      size_t n;
-	      char *p = get_user_id (ctrl, sig->keyid, &n, NULL);
-	      print_utf8_buffer (es_stdout, p, n);
-	      xfree (p);
-	    }
-	  es_putc ('\n', es_stdout);
-
-	  if (sig->flags.policy_url
-	      && (opt.list_options & LIST_SHOW_POLICY_URLS))
-	    show_policy_url (sig, 3, 0);
-
-	  if (sig->flags.notation && (opt.list_options & LIST_SHOW_NOTATIONS))
-	    show_notation (sig, 3, 0,
-			   ((opt.
-			     list_options & LIST_SHOW_STD_NOTATIONS) ? 1 : 0)
-			   +
-			   ((opt.
-			     list_options & LIST_SHOW_USER_NOTATIONS) ? 2 :
-			    0));
+              for (n=node; n && n->pkt->pkttype == PKT_SIGNATURE; n = n->next)
+                sigcount++;
+              sigarray = xcalloc (sigcount, sizeof *sigarray);
 
-	  if (sig->flags.pref_ks
-	      && (opt.list_options & LIST_SHOW_KEYSERVER_URLS))
-	    show_keyserver_url (sig, 3, 0);
-
-          if (reason_text)
-            {
-              es_fprintf (es_stdout, "      %s%s\n",
-                          _("reason for revocation: "), reason_text);
-              if (reason_comment)
+              sigcount = 0;
+              for (n=node; n && n->pkt->pkttype == PKT_SIGNATURE; n = n->next)
                 {
-                  const byte *s, *s_lf;
-                  size_t n, n_lf;
+                  if (!keyid_cmp (mainkid, n->pkt->pkt.signature->keyid))
+                    n->flag |= NODFLG_MARK_B;  /* Is a self-sig.  */
+                  else
+                    n->flag &= ~NODFLG_MARK_B;
 
-                  s = reason_comment;
-                  n = reason_commentlen;
-                  s_lf = NULL;
-                  do
-                    {
-                      /* We don't want any empty lines, so we skip them.  */
-                      for (;n && *s == '\n'; s++, n--)
-                        ;
-                      if (n)
-                        {
-                          s_lf = memchr (s, '\n', n);
-                          n_lf = s_lf? s_lf - s : n;
-                          es_fprintf (es_stdout, "         %s",
-                                      _("revocation comment: "));
-                          es_write_sanitized (es_stdout, s, n_lf, NULL, NULL);
-                          es_putc ('\n', es_stdout);
-                          s += n_lf; n -= n_lf;
-                        }
-                    } while (s_lf);
+                  sigarray[sigcount++] = node = n;
                 }
-            }
+              /* Note that NODE is now at the last signature.  */
 
-          xfree (reason_text);
-          xfree (reason_comment);
+              qsort (sigarray, sigcount, sizeof *sigarray, cmp_signodes);
 
-	  /* fixme: check or list other sigs here */
+              for (idx=0; idx < sigcount; idx++)
+                list_signature_print (ctrl, keyblock, sigarray[idx], listctx);
+              xfree (sigarray);
+            }
+          else
+            list_signature_print (ctrl, keyblock, node, listctx);
 	}
     }
   es_putc ('\n', es_stdout);
diff --git a/g10/options.h b/g10/options.h
index dfa94d4e2..b014514e5 100644
--- a/g10/options.h
+++ b/g10/options.h
@@ -398,6 +398,7 @@ EXTERN_UNLESS_MAIN_MODULE int memory_stat_debug_mode;
 #define LIST_SHOW_SIG_SUBPACKETS         (1<<10)
 #define LIST_SHOW_USAGE                  (1<<11)
 #define LIST_SHOW_ONLY_FPR_MBOX          (1<<12)
+#define LIST_SORT_SIGS                   (1<<13)
 
 #define VERIFY_SHOW_PHOTOS               (1<<0)
 #define VERIFY_SHOW_POLICY_URLS          (1<<1)
diff --git a/g10/trustdb.c b/g10/trustdb.c
index 4669ac0e8..a313a423c 100644
--- a/g10/trustdb.c
+++ b/g10/trustdb.c
@@ -1834,7 +1834,7 @@ search_skipfnc (void *opaque, u32 *kid, int dummy_uid_no)
 
 /*
  * Scan all keys and return a key_array of all suitable keys from
- * kllist.  The caller has to pass keydb handle so that we don't use
+ * klist.  The caller has to pass keydb handle so that we don't use
  * to create our own.  Returns either a key_array or NULL in case of
  * an error.  No results found are indicated by an empty array.
  * Caller hast to release the returned array.