gpg: Improve keydb handling in the main import function.

* g10/getkey.c (get_pubkey_byfprint_fast): Factor most code out to ...
(get_keyblock_byfprint_fast): .. new function.
* g10/import.c (revocation_present): s/int rc/gpg_error_t err/.
(import_one): Use get_keyblock_byfprint_fast to get the keyblock and a
handle.  Remove the now surplus keyblock fetch in the merge branch.

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

g10/getkey.c

@@ -1828,16 +1828,47 @@ get_pubkey_byfprint (ctrl_t ctrl, PKT_public_key *pk, kbnode_t *r_keyblock,
  *
  * Like get_pubkey_byfprint, PK may be NULL.  In that case, this
  * function effectively just checks for the existence of the key.  */
-int
+gpg_error_t
 get_pubkey_byfprint_fast (PKT_public_key * pk,
 			  const byte * fprint, size_t fprint_len)
 {
-  int rc = 0;
+  gpg_error_t err;
-  KEYDB_HANDLE hd;
   KBNODE keyblock;
+
+  err = get_keyblock_byfprint_fast (&keyblock, NULL, fprint, fprint_len, 0);
+  if (!err)
+    {
+      if (pk)
+        copy_public_key (pk, keyblock->pkt->pkt.public_key);
+      release_kbnode (keyblock);
+    }
+
+  return err;
+}
+
+
+/* This function is similar to get_pubkey_byfprint_fast but returns a
+ * keydb handle at R_HD and the keyblock at R_KEYBLOCK.  R_KEYBLOCK or
+ * R_HD may be NULL.  If LOCK is set the handle has been opend in
+ * locked mode and keydb_disable_caching () has been called.  On error
+ * R_KEYBLOCK is set to NULL but R_HD must be released by the caller;
+ * it may have a value of NULL, though.  This allows to do an insert
+ * operation on a locked keydb handle.  */
+gpg_error_t
+get_keyblock_byfprint_fast (kbnode_t *r_keyblock, KEYDB_HANDLE *r_hd,
+                            const byte *fprint, size_t fprint_len, int lock)
+{
+  gpg_error_t err;
+  KEYDB_HANDLE hd;
+  kbnode_t keyblock;
   byte fprbuf[MAX_FINGERPRINT_LEN];
   int i;
 
+  if (r_keyblock)
+    *r_keyblock = NULL;
+  if (r_hd)
+    *r_hd = NULL;
+
   for (i = 0; i < MAX_FINGERPRINT_LEN && i < fprint_len; i++)
     fprbuf[i] = fprint[i];
   while (i < MAX_FINGERPRINT_LEN)
@@ -1846,33 +1877,48 @@ get_pubkey_byfprint_fast (PKT_public_key * pk,
   hd = keydb_new ();
   if (!hd)
     return gpg_error_from_syserror ();
+  if (r_hd)
+    *r_hd = hd;
 
-  rc = keydb_search_fpr (hd, fprbuf);
+  if (lock)
-  if (gpg_err_code (rc) == GPG_ERR_NOT_FOUND)
     {
-      keydb_release (hd);
+      keydb_disable_caching (hd);
-      return GPG_ERR_NO_PUBKEY;
     }
-  rc = keydb_get_keyblock (hd, &keyblock);
+
-  keydb_release (hd);
+  err = keydb_search_fpr (hd, fprbuf);
-  if (rc)
+  if (gpg_err_code (err) == GPG_ERR_NOT_FOUND)
     {
-      log_error ("keydb_get_keyblock failed: %s\n", gpg_strerror (rc));
+      if (!r_hd)
-      return GPG_ERR_NO_PUBKEY;
+        keydb_release (hd);
+      return gpg_error (GPG_ERR_NO_PUBKEY);
+    }
+  err = keydb_get_keyblock (hd, &keyblock);
+  if (err)
+    {
+      log_error ("keydb_get_keyblock failed: %s\n", gpg_strerror (err));
+      if (!r_hd)
+        keydb_release (hd);
+      return gpg_error (GPG_ERR_NO_PUBKEY);
     }
 
   log_assert (keyblock->pkt->pkttype == PKT_PUBLIC_KEY
               || keyblock->pkt->pkttype == PKT_PUBLIC_SUBKEY);
-  if (pk)
-    copy_public_key (pk, keyblock->pkt->pkt.public_key);
-  release_kbnode (keyblock);
 
   /* Not caching key here since it won't have all of the fields
      properly set. */
 
+  if (r_keyblock)
+    *r_keyblock = keyblock;
+  else
+    release_kbnode (keyblock);
+
+  if (!r_hd)
+    keydb_release (hd);
+
   return 0;
 }
 
+
 const char *
 parse_def_secret_key (ctrl_t ctrl)
 {

g10/import.c

@@ -1618,7 +1618,6 @@ import_one (ctrl_t ctrl,
 {
   gpg_error_t err = 0;
   PKT_public_key *pk;
-  PKT_public_key *pk_orig = NULL;
   kbnode_t node, uidnode;
   kbnode_t keyblock_orig = NULL;
   byte fpr2[MAX_FINGERPRINT_LEN];
@@ -1800,16 +1799,15 @@ import_one (ctrl_t ctrl,
     goto leave;
 
   /* Do we have this key already in one of our pubrings ? */
-  hd = keydb_new ();
+  err = get_keyblock_byfprint_fast (&keyblock_orig, &hd,
-  if (!hd)
+                                    fpr2, fpr2len, 1/*locked*/);
-    return gpg_error_from_syserror ();
+  if ((err
-  keydb_disable_caching (hd);
+       && gpg_err_code (err) != GPG_ERR_NO_PUBKEY
-  pk_orig = xmalloc_clear( sizeof *pk_orig );
+       && gpg_err_code (err) != GPG_ERR_UNUSABLE_PUBKEY)
-  err = get_pubkey_byfprint_fast (pk_orig, fpr2, fpr2len);
+      || !hd)
-  if (err
-      && gpg_err_code (err) != GPG_ERR_NO_PUBKEY
-      && gpg_err_code (err) != GPG_ERR_UNUSABLE_PUBKEY )
     {
+      /* The !hd above is to catch a misbehaving function which
+       * returns NO_PUBKEY for failing to allocate a handle.  */
       if (!silent)
         log_error (_("key %s: public key not found: %s\n"),
                    keystr(keyid), gpg_strerror (err));
@@ -1823,6 +1821,7 @@ import_one (ctrl_t ctrl,
     }
   else if (err)  /* Insert this key. */
     {
+      /* Note: ERR can only be NO_PUBKEY or UNUSABLE_PUBKEY.  */
       int n_sigs_cleaned, n_uids_cleaned;
 
       err = keydb_locate_writable (hd);
@@ -1892,45 +1891,26 @@ import_one (ctrl_t ctrl,
       stats->imported++;
       new_key = 1;
     }
-  else /* Merge the key.  */
+  else /* Key already exists - merge.  */
     {
       int n_uids, n_sigs, n_subk, n_sigs_cleaned, n_uids_cleaned;
       u32 curtime = make_timestamp ();
 
       /* Compare the original against the new key; just to be sure nothing
        * weird is going on */
-      if (cmp_public_keys( pk_orig, pk ) )
+      if (cmp_public_keys (keyblock_orig->pkt->pkt.public_key, pk))
         {
           if (!silent)
             log_error( _("key %s: doesn't match our copy\n"),keystr(keyid));
           goto leave;
         }
 
-      /* Now read the original keyblock again so that we can use the
-       * handle for updating the keyblock.  FIXME: Why not let
-       * get_pubkey_byfprint_fast do that - it fetches the keyblock
-       * anyway. */
-      err = keydb_search_fpr (hd, fpr2);
-      if (err)
-        {
-          log_error (_("key %s: can't locate original keyblock: %s\n"),
-                     keystr(keyid), gpg_strerror (err));
-          goto leave;
-        }
-      err = keydb_get_keyblock (hd, &keyblock_orig);
-      if (err)
-        {
-          log_error (_("key %s: can't read original keyblock: %s\n"),
-                     keystr(keyid), gpg_strerror (err));
-          goto leave;
-        }
-
       /* Make sure the original direct key sigs are all sane.  */
       n_sigs_cleaned = fix_bad_direct_key_sigs (ctrl, keyblock_orig, keyid);
       if (n_sigs_cleaned)
         commit_kbnode (&keyblock_orig);
 
-      /* and try to merge the block */
+      /* Try to merge KEYBLOCK into KEYBLOCK_ORIG.  */
       clear_kbnode_flags( keyblock_orig );
       clear_kbnode_flags( keyblock );
       n_uids = n_sigs = n_subk = n_uids_cleaned = 0;
@@ -2091,7 +2071,6 @@ import_one (ctrl_t ctrl,
     }
 
   release_kbnode( keyblock_orig );
-  free_public_key( pk_orig );
 
   return err;
 }
@@ -3266,14 +3245,15 @@ revocation_present (ctrl_t ctrl, kbnode_t keyblock)
 		      /* Okay, we have a revocation key, and a
                        * revocation issued by it.  Do we have the key
                        * itself?  */
-		      int rc;
+                      gpg_error_t err;
 
-		      rc=get_pubkey_byfprint_fast (NULL,sig->revkey[idx].fpr,
+		      err = get_pubkey_byfprint_fast (NULL,
-                                                   MAX_FINGERPRINT_LEN);
+                                                      sig->revkey[idx].fpr,
-		      if (gpg_err_code (rc) == GPG_ERR_NO_PUBKEY
+                                                      MAX_FINGERPRINT_LEN);
-                          || gpg_err_code (rc) == GPG_ERR_UNUSABLE_PUBKEY)
+		      if (gpg_err_code (err) == GPG_ERR_NO_PUBKEY
+                          || gpg_err_code (err) == GPG_ERR_UNUSABLE_PUBKEY)
 			{
-			  char *tempkeystr=xstrdup(keystr_from_pk(pk));
+			  char *tempkeystr = xstrdup (keystr_from_pk (pk));
 
 			  /* No, so try and get it */
 			  if ((opt.keyserver_options.options
@@ -3289,13 +3269,13 @@ revocation_present (ctrl_t ctrl, kbnode_t keyblock)
                                                        opt.keyserver, 0);
 
 			      /* Do we have it now? */
-			      rc=get_pubkey_byfprint_fast (NULL,
+			      err = get_pubkey_byfprint_fast (NULL,
 						     sig->revkey[idx].fpr,
 						     MAX_FINGERPRINT_LEN);
 			    }
 
-			  if (gpg_err_code (rc) == GPG_ERR_NO_PUBKEY
+			  if (gpg_err_code (err) == GPG_ERR_NO_PUBKEY
-                              || gpg_err_code (rc) == GPG_ERR_UNUSABLE_PUBKEY)
+                              || gpg_err_code (err) == GPG_ERR_UNUSABLE_PUBKEY)
 			    log_info(_("WARNING: key %s may be revoked:"
 				       " revocation key %s not present.\n"),
 				     tempkeystr,keystr(keyid));

g10/keydb.h

@@ -339,8 +339,17 @@ int get_pubkey_byfprint (ctrl_t ctrl, PKT_public_key *pk, kbnode_t *r_keyblock,
 /* This function is similar to get_pubkey_byfprint, but it doesn't
    merge the self-signed data into the public key and subkeys or into
    the user ids.  */
-int get_pubkey_byfprint_fast (PKT_public_key *pk,
+gpg_error_t get_pubkey_byfprint_fast (PKT_public_key *pk,
-                              const byte *fprint, size_t fprint_len);
+                                      const byte *fprint, size_t fprint_len);
+
+/* This function is similar to get_pubkey_byfprint, but it doesn't
+   merge the self-signed data into the public key and subkeys or into
+   the user ids.  */
+gpg_error_t get_keyblock_byfprint_fast (kbnode_t *r_keyblock,
+                                        KEYDB_HANDLE *r_hd,
+                                        const byte *fprint, size_t fprint_len,
+                                        int lock);
+
 
 /* Returns true if a secret key is available for the public key with
    key id KEYID.  */