Report better error messages when queue locking fails

Put the full error message in the Kerberos context, and change an
internal function to be able to returna proper status.

diff --git a/plugin/queue.c b/plugin/queue.c
index 96d3ca453a1e74434d9de0b10b2d70f1542a036f..5d4cc9ce9ec6ae2d421384f09f493e3010267df9 100644 (file)
--- a/plugin/queue.c
+++ b/plugin/queue.c
@@ -46,35 +46,40 @@
 
 
 /*
- * Lock the queue directory.  Returns a file handle to the lock file, which
- * must then be passed into unlock_queue when the queue should be unlocked, or
- * -1 on failure to lock.
+ * Lock the queue directory and stores the file descriptor of the lock in the
+ * secon argument.  This must be passed into unlock_queue when the queue
+ * should be unlocked.  Returns a Kerberos status code.
  *
  * We have to use flock for compatibility with the Perl krb5-sync-backend
  * script.  Perl makes it very annoying to use fcntl locking on Linux.
  */
-static int
-lock_queue(kadm5_hook_modinfo *config)
+static krb5_error_code
+lock_queue(kadm5_hook_modinfo *config, krb5_context ctx, int *result)
 {
     char *lockpath = NULL;
     int fd = -1;
+    krb5_error_code code;
 
     if (asprintf(&lockpath, "%s/.lock", config->queue_dir) < 0)
-        return -1;
+        return sync_error_system(ctx, "cannot allocate memory");
     fd = open(lockpath, O_RDWR | O_CREAT, 0644);
-    if (fd < 0)
+    if (fd < 0) {
+        code = sync_error_system(ctx, "cannot open lock file %s", lockpath);
         goto fail;
-    free(lockpath);
-    lockpath = NULL;
-    if (flock(fd, LOCK_EX) < 0)
+    }
+    if (flock(fd, LOCK_EX) < 0) {
+        code = sync_error_system(ctx, "cannot flock lock file %s", lockpath);
         goto fail;
-    return fd;
+    }
+    free(lockpath);
+    *result = fd;
+    return 0;
 
 fail:
     free(lockpath);
     if (fd >= 0)
         close(fd);
-    return -1;
+    return code;
 }
 
 
@@ -167,14 +172,15 @@ sync_queue_conflict(kadm5_hook_modinfo *config, krb5_context ctx,
     DIR *queue = NULL;
     struct dirent *entry;
     int found = 0;
+    krb5_error_code code;
 
     if (config->queue_dir == NULL)
         return -1;
     prefix = queue_prefix(ctx, principal, domain, operation);
     if (prefix == NULL)
         return -1;
-    lock = lock_queue(config);
-    if (lock < 0)
+    code = lock_queue(config, ctx, &lock);
+    if (code != 0)
         goto fail;
     queue = opendir(config->queue_dir);
     if (queue == NULL)
@@ -227,11 +233,9 @@ sync_queue_write(kadm5_hook_modinfo *config, krb5_context ctx,
      * Lock the queue before the timestamp so that another writer coming up
      * at the same time can't get an earlier timestamp.
      */
-    lock = lock_queue(config);
-    if (lock < 0) {
-        code = sync_error_system(ctx, "cannot lock queue");
+    code = lock_queue(config, ctx, &lock);
+    if (code != 0)
         goto fail;
-    }
     timestamp = queue_timestamp();
     if (timestamp == NULL) {
         code = sync_error_system(ctx, "cannot generate timestamp");

diff --git a/tests/plugin/heimdal-t.c b/tests/plugin/heimdal-t.c
index 58010217af558654478d28b8cd965fdf2773cf20..8cea07032fbd81e85c5f1f6cbbfde5a0cbf02e09 100644 (file)
--- a/tests/plugin/heimdal-t.c
+++ b/tests/plugin/heimdal-t.c
@@ -63,6 +63,7 @@ main(void)
     struct kadm5_hook *hook = NULL;
     kadm5_principal_ent_rec entity;
     const char *message;
+    char *wanted;
 
     krb5conf = test_file_path("data/krb5.conf");
     if (krb5conf == NULL)
@@ -120,15 +121,15 @@ main(void)
     if (hook == NULL)
         ok_block(8, false, "No symbol in plugin");
     else {
+        basprintf(&wanted, "cannot open lock file queue/.lock: %s",
+                  strerror(ENOENT));
         is_int(0, hook->init(ctx, &config), "init");
         ok(config != NULL, "...and config is not NULL");
         code = hook->chpass(ctx, config, KADM5_HOOK_STAGE_PRECOMMIT, princ,
                             "test");
         is_int(ENOENT, code, "chpass");
         message = krb5_get_error_message(ctx, code);
-        is_int(strncmp("cannot lock queue", message,
-                       strlen("cannot lock queue")),
-               0, "...with correct error message");
+        is_string(wanted, message, "...with correct error message");
         krb5_free_error_message(ctx, message);
 
         /* Test chpass with a NULL password. */
@@ -147,17 +148,13 @@ main(void)
                             0, "test");
         is_int(ENOENT, code, "create");
         message = krb5_get_error_message(ctx, code);
-        is_int(strncmp("cannot lock queue", message,
-                       strlen("cannot lock queue")),
-               0, "...with correct error message");
+        is_string(wanted, message, "...with correct error message");
         krb5_free_error_message(ctx, message);
         code = hook->modify(ctx, config, KADM5_HOOK_STAGE_POSTCOMMIT, &entity,
                             KADM5_ATTRIBUTES);
         is_int(ENOENT, code, "modify");
         message = krb5_get_error_message(ctx, code);
-        is_int(strncmp("cannot lock queue", message,
-                       strlen("cannot lock queue")),
-               0, "...with correct error message");
+        is_string(wanted, message, "...with correct error message");
         krb5_free_error_message(ctx, message);
 
         /* Test create with a NULL password. */
@@ -167,6 +164,7 @@ main(void)
 
         /* Close down the module. */
         hook->fini(ctx, config);
+        free(wanted);
     }
 
     /* Clean up. */

diff --git a/tests/plugin/mit-t.c b/tests/plugin/mit-t.c
index 09631a078116aed43a531e7a9c468dca004c28af..1dd55cc8ef25f1c08ba9891b532cb56ca59ddf19 100644 (file)
--- a/tests/plugin/mit-t.c
+++ b/tests/plugin/mit-t.c
@@ -49,6 +49,7 @@ main(void)
     kadm5_hook_modinfo *data = NULL;
     kadm5_principal_ent_rec entity;
     const char *message;
+    char *wanted;
 
     krb5conf = test_file_path("data/krb5.conf");
     if (krb5conf == NULL)
@@ -112,15 +113,15 @@ main(void)
     if (hook.name == NULL)
         ok_block(8, false, "No vtable");
     else {
+        basprintf(&wanted, "cannot open lock file queue/.lock: %s",
+                  strerror(ENOENT));
         is_int(0, hook.init(ctx, &data), "init");
         ok(data != NULL, "...and data is not NULL");
         code = hook.chpass(ctx, data, KADM5_HOOK_STAGE_PRECOMMIT, princ,
                            false, 0, NULL, "test");
         is_int(ENOENT, code, "chpass");
         message = krb5_get_error_message(ctx, code);
-        is_int(strncmp("cannot lock queue", message,
-                       strlen("cannot lock queue")),
-               0, "...with correct error message");
+        is_string(wanted, message, "...with correct error message");
         krb5_free_error_message(ctx, message);
 
         /* Test chpass with a NULL password. */
@@ -139,17 +140,13 @@ main(void)
                            0, 0, NULL, "test");
         is_int(ENOENT, code, "create");
         message = krb5_get_error_message(ctx, code);
-        is_int(strncmp("cannot lock queue", message,
-                       strlen("cannot lock queue")),
-               0, "...with correct error message");
+        is_string(wanted, message, "...with correct error message");
         krb5_free_error_message(ctx, message);
         code = hook.modify(ctx, data, KADM5_HOOK_STAGE_POSTCOMMIT, &entity,
                            KADM5_ATTRIBUTES);
         is_int(ENOENT, code, "modify");
         message = krb5_get_error_message(ctx, code);
-        is_int(strncmp("cannot lock queue", message,
-                       strlen("cannot lock queue")),
-               0, "...with correct error message");
+        is_string(wanted, message, "...with correct error message");
         krb5_free_error_message(ctx, message);
 
         /* Test create with a NULL password. */
@@ -159,6 +156,7 @@ main(void)
 
         /* Close down the module. */
         hook.fini(ctx, data);
+        free(wanted);
     }
 
     /* Clean up. */

diff --git a/tests/plugin/queuing-t.c b/tests/plugin/queuing-t.c
index db53a10c148fbc261a21a3a3b767dc6c36901712..bef061f6e5f2a18db8d2680f707e0f66c7d5ecc0 100644 (file)
--- a/tests/plugin/queuing-t.c
+++ b/tests/plugin/queuing-t.c
@@ -43,6 +43,7 @@ main(void)
     FILE *file;
     struct stat st;
     const char *message;
+    char *wanted;
 
     tmpdir = test_tmpdir();
     if (chdir(tmpdir) < 0)
@@ -208,21 +209,22 @@ main(void)
     ok(rmdir("queue") == 0, "No other files in queue directory");
 
     /* Check failure when there's no queue directory. */
+    basprintf(&wanted, "cannot open lock file queue/.lock: %s",
+              strerror(ENOENT));
     code = sync_chpass(data, ctx, princ, "foobar");
     is_int(ENOENT, code, "sync_chpass fails with no queue");
     message = krb5_get_error_message(ctx, code);
-    is_int(strncmp("cannot lock queue", message, strlen("cannot lock queue")),
-           0, "...with correct error message");
+    is_string(wanted, message, "...with correct error message");
     krb5_free_error_message(ctx, message);
     code = sync_status(data, ctx, princ, false);
     is_int(ENOENT, code, "sync_status disable fails with no queue");
     message = krb5_get_error_message(ctx, code);
-    is_int(strncmp("cannot lock queue", message, strlen("cannot lock queue")),
-           0, "...with correct error message");
+    is_string(wanted, message, "...with correct error message");
     krb5_free_error_message(ctx, message);
 
     /* Shut down the plugin. */
     sync_close(data);
+    free(wanted);
 
     /*
      * Change to an empty Kerberos configuration file, and then make sure the