sm: Change keydb code to use the keybox locking.

* kbx/keybox-init.c (keybox_lock): New arg TIMEOUT.  Change all
callers to pass -1 when locking.
* sm/keydb.c (struct resource_item): Remove LOCKANDLE.
(struct keydb_handle): Add KEEP_LOCK.
(keydb_add_resource): Use keybox locking instead of a separate dotlock
for testing whether we can run a compress.
(keydb_release): Reset KEEP_LOCK.
(keydb_lock): Set KEEP_LOCK.
(unlock_all): Take care of KEEP_LOCK.
(lock_all): Use keybox_lock instead of dotlock fucntions.
(keydb_delete): Remove arg UNLOCK.
* sm/delete.c (delete_one): Adjust keydb_delete.  Due to the KEEP_LOCK
the keydb_release takes care of unlocking.
--

This aligns the code more with g10/keydb.c and avoids the separate
calls to dotlock_take.

GnuPG-bug-id: 4505
Signed-off-by: Werner Koch <[email protected]>

6 files changed, 54 insertions, 50 deletions

diff --git a/g10/keydb.c b/g10/keydb.c
index 8c067e1df..45eb4aa34 100644
--- a/g10/keydb.c
+++ b/g10/keydb.c
@@ -1,6 +1,6 @@
 /* keydb.c - key database dispatcher
  * Copyright (C) 2001-2013 Free Software Foundation, Inc.
- * Coyrright (C) 2001-2015 Werner Koch
+ * Copyright (C) 2001-2015 Werner Koch
  *
  * This file is part of GnuPG.
  *
@@ -1076,7 +1076,7 @@ lock_all (KEYDB_HANDLE hd)
           rc = keyring_lock (hd->active[i].u.kr, 1);
           break;
         case KEYDB_RESOURCE_TYPE_KEYBOX:
-          rc = keybox_lock (hd->active[i].u.kb, 1);
+          rc = keybox_lock (hd->active[i].u.kb, 1, -1);
           break;
         }
     }
@@ -1094,7 +1094,7 @@ lock_all (KEYDB_HANDLE hd)
               keyring_lock (hd->active[i].u.kr, 0);
               break;
             case KEYDB_RESOURCE_TYPE_KEYBOX:
-              keybox_lock (hd->active[i].u.kb, 0);
+              keybox_lock (hd->active[i].u.kb, 0, 0);
               break;
             }
         }
@@ -1127,7 +1127,7 @@ unlock_all (KEYDB_HANDLE hd)
           keyring_lock (hd->active[i].u.kr, 0);
           break;
         case KEYDB_RESOURCE_TYPE_KEYBOX:
-          keybox_lock (hd->active[i].u.kb, 0);
+          keybox_lock (hd->active[i].u.kb, 0, 0);
           break;
         }
     }
diff --git a/kbx/keybox-init.c b/kbx/keybox-init.c
index 6a83f7162..9a65c1345 100644
--- a/kbx/keybox-init.c
+++ b/kbx/keybox-init.c
@@ -261,10 +261,12 @@ _keybox_close_file (KEYBOX_HANDLE hd)
 
 
 /*
- * Lock the keybox at handle HD, or unlock if YES is false.
+ * Lock the keybox at handle HD, or unlock if YES is false.  TIMEOUT
+ * is the value used for dotlock_take.  In general -1 should be used
+ * when taking a lock; use 0 when releasing a lock.
  */
 gpg_error_t
-keybox_lock (KEYBOX_HANDLE hd, int yes)
+keybox_lock (KEYBOX_HANDLE hd, int yes, long timeout)
 {
   gpg_error_t err = 0;
   KB_NAME kb = hd->kb;
@@ -302,10 +304,13 @@ keybox_lock (KEYBOX_HANDLE hd, int yes)
               hd->fp = NULL;
             }
 #endif /*HAVE_W32_SYSTEM*/
-          if (dotlock_take (kb->lockhd, -1))
+          if (dotlock_take (kb->lockhd, timeout))
             {
               err = gpg_error_from_syserror ();
-              log_info ("can't lock '%s'\n", kb->fname );
+              if (!timeout && gpg_err_code (err) == GPG_ERR_EACCES)
+                ; /* No diagnostic if we only tried to lock.  */
+              else
+                log_info ("can't lock '%s'\n", kb->fname );
             }
           else
             kb->is_locked = 1;
diff --git a/kbx/keybox.h b/kbx/keybox.h
index 665b05fc0..4d941571e 100644
--- a/kbx/keybox.h
+++ b/kbx/keybox.h
@@ -76,7 +76,7 @@ void keybox_pop_found_state (KEYBOX_HANDLE hd);
 const char *keybox_get_resource_name (KEYBOX_HANDLE hd);
 int keybox_set_ephemeral (KEYBOX_HANDLE hd, int yes);
 
-gpg_error_t keybox_lock (KEYBOX_HANDLE hd, int yes);
+gpg_error_t keybox_lock (KEYBOX_HANDLE hd, int yes, long timeout);
 
 /*-- keybox-file.c --*/
 /* Fixme: This function does not belong here: Provide a better
diff --git a/sm/delete.c b/sm/delete.c
index f359cc595..b370406de 100644
--- a/sm/delete.c
+++ b/sm/delete.c
@@ -113,7 +113,8 @@ delete_one (ctrl_t ctrl, const char *username)
       goto leave;
     }
 
-  /* We need to search again to get back to the right position. */
+  /* We need to search again to get back to the right position.  Neo
+   * that the lock is kept until the KH is released.  */
   rc = keydb_lock (kh);
   if (rc)
     {
@@ -132,7 +133,7 @@ delete_one (ctrl_t ctrl, const char *username)
           goto leave;
         }
 
-      rc = keydb_delete (kh, duplicates ? 0 : 1);
+      rc = keydb_delete (kh);
       if (rc)
         goto leave;
       if (opt.verbose)
diff --git a/sm/keydb.c b/sm/keydb.c
index f66a5766d..0144a8189 100644
--- a/sm/keydb.c
+++ b/sm/keydb.c
@@ -47,7 +47,6 @@ struct resource_item {
     KEYBOX_HANDLE kr;
   } u;
   void *token;
-  dotlock_t lockhandle;
 };
 
 static struct resource_item all_resources[MAX_KEYDB_RESOURCES];
@@ -58,7 +57,14 @@ static int any_registered;
 
 
 struct keydb_handle {
+
+  /* If this flag is set the resources is locked.  */
   int locked;
+
+  /* If this flag is set a lock will only be released by
+   * keydb_release.  */
+  int keep_lock;
+
   int found;
   int saved_found;
   int current;
@@ -346,26 +352,20 @@ keydb_add_resource (ctrl_t ctrl, const char *url, int force, int *auto_created)
           err = gpg_error (GPG_ERR_RESOURCE_LIMIT);
         else
           {
+            KEYBOX_HANDLE kbxhd;
+
             all_resources[used_resources].type = rt;
             all_resources[used_resources].u.kr = NULL; /* Not used here */
             all_resources[used_resources].token = token;
 
-            all_resources[used_resources].lockhandle
-              = dotlock_create (filename, 0);
-            if (!all_resources[used_resources].lockhandle)
-              log_fatal ( _("can't create lock for '%s'\n"), filename);
-
-            /* Do a compress run if needed and the file is not locked. */
-            if (!dotlock_take (all_resources[used_resources].lockhandle, 0))
+            /* Do a compress run if needed and the keybox is not locked. */
+            kbxhd = keybox_new_x509 (token, 0);
+            if (kbxhd)
               {
-                KEYBOX_HANDLE kbxhd = keybox_new_x509 (token, 0);
-
-                if (kbxhd)
-                  {
-                    keybox_compress (kbxhd);
-                    keybox_release (kbxhd);
-                  }
-                dotlock_release (all_resources[used_resources].lockhandle);
+                if (!keybox_lock (kbxhd, 1, 0))
+                  keybox_compress (kbxhd);
+
+                keybox_release (kbxhd);
               }
 
             used_resources++;
@@ -415,7 +415,6 @@ keydb_new (void)
         case KEYDB_RESOURCE_TYPE_KEYBOX:
           hd->active[j].type   = all_resources[i].type;
           hd->active[j].token  = all_resources[i].token;
-          hd->active[j].lockhandle = all_resources[i].lockhandle;
           hd->active[j].u.kr = keybox_new_x509 (all_resources[i].token, 0);
           if (!hd->active[j].u.kr)
             {
@@ -442,6 +441,7 @@ keydb_release (KEYDB_HANDLE hd)
   assert (active_handles > 0);
   active_handles--;
 
+  hd->keep_lock = 0;
   unlock_all (hd);
   for (i=0; i < hd->used; i++)
     {
@@ -526,17 +526,22 @@ keydb_set_ephemeral (KEYDB_HANDLE hd, int yes)
 
 
 /* If the keyring has not yet been locked, lock it now.  This
-   operation is required before any update operation; it is optional
-   for an insert operation.  The lock is released with
-   keydb_released. */
+ * operation is required before any update operation; it is optional
+ * for an insert operation.  The lock is kept until a keydb_release so
+ * that internal unlock_all calls have no effect.  */
 gpg_error_t
 keydb_lock (KEYDB_HANDLE hd)
 {
+  gpg_error_t err;
+
   if (!hd)
     return gpg_error (GPG_ERR_INV_HANDLE);
-  if (hd->locked)
-    return 0; /* Already locked. */
-  return lock_all (hd);
+
+  err = lock_all (hd);
+  if (!err)
+    hd->keep_lock = 1;
+
+  return err;
 }
 
 
@@ -556,8 +561,7 @@ lock_all (KEYDB_HANDLE hd)
         case KEYDB_RESOURCE_TYPE_NONE:
           break;
         case KEYDB_RESOURCE_TYPE_KEYBOX:
-          if (hd->active[i].lockhandle)
-            rc = dotlock_take (hd->active[i].lockhandle, -1);
+          rc = keybox_lock (hd->active[i].u.kr, 1, -1);
           break;
         }
       if (rc)
@@ -566,7 +570,7 @@ lock_all (KEYDB_HANDLE hd)
 
     if (rc)
       {
-        /* revert the already set locks */
+        /* Revert the already set locks.  */
         for (i--; i >= 0; i--)
           {
             switch (hd->active[i].type)
@@ -574,8 +578,7 @@ lock_all (KEYDB_HANDLE hd)
               case KEYDB_RESOURCE_TYPE_NONE:
                 break;
               case KEYDB_RESOURCE_TYPE_KEYBOX:
-                if (hd->active[i].lockhandle)
-                  dotlock_release (hd->active[i].lockhandle);
+                keybox_lock (hd->active[i].u.kr, 0, 0);
                 break;
               }
           }
@@ -583,10 +586,7 @@ lock_all (KEYDB_HANDLE hd)
     else
       hd->locked = 1;
 
-    /* make_dotlock () does not yet guarantee that errno is set, thus
-       we can't rely on the error reason and will simply use
-       EACCES. */
-    return rc? gpg_error (GPG_ERR_EACCES) : 0;
+    return rc;
 }
 
 static void
@@ -594,7 +594,7 @@ unlock_all (KEYDB_HANDLE hd)
 {
   int i;
 
-  if (!hd->locked)
+  if (!hd->locked || hd->keep_lock)
     return;
 
   for (i=hd->used-1; i >= 0; i--)
@@ -604,8 +604,7 @@ unlock_all (KEYDB_HANDLE hd)
         case KEYDB_RESOURCE_TYPE_NONE:
           break;
         case KEYDB_RESOURCE_TYPE_KEYBOX:
-          if (hd->active[i].lockhandle)
-            dotlock_release (hd->active[i].lockhandle);
+          keybox_lock (hd->active[i].u.kr, 0, 0);
           break;
         }
     }
@@ -840,7 +839,7 @@ keydb_update_cert (KEYDB_HANDLE hd, ksba_cert_t cert)
  * The current keyblock or cert will be deleted.
  */
 int
-keydb_delete (KEYDB_HANDLE hd, int unlock)
+keydb_delete (KEYDB_HANDLE hd)
 {
   int rc = -1;
 
@@ -866,8 +865,7 @@ keydb_delete (KEYDB_HANDLE hd, int unlock)
       break;
     }
 
-  if (unlock)
-    unlock_all (hd);
+  unlock_all (hd);
   return rc;
 }
 
diff --git a/sm/keydb.h b/sm/keydb.h
index 623462553..20dcdbe4d 100644
--- a/sm/keydb.h
+++ b/sm/keydb.h
@@ -49,7 +49,7 @@ int keydb_get_cert (KEYDB_HANDLE hd, ksba_cert_t *r_cert);
 int keydb_insert_cert (KEYDB_HANDLE hd, ksba_cert_t cert);
 int keydb_update_cert (KEYDB_HANDLE hd, ksba_cert_t cert);
 
-int keydb_delete (KEYDB_HANDLE hd, int unlock);
+int keydb_delete (KEYDB_HANDLE hd);
 
 int keydb_locate_writable (KEYDB_HANDLE hd, const char *reserved);
 void keydb_rebuild_caches (void);