Add 'cmd_cleanup' trigger

The new 'cmd_cleanup' trigger replaces the old command_cleanup()
function, which was causing linking problems on OS X and inverted strict
dependency.  The keyring cmd_cleanup calls keyring_free(global_keyring),
instead of merely asserting keyring == NULL, so the error exit cases of
many CLI functions have been simplified.

android.c

@@ -19,11 +19,7 @@ Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
 
 #include <stdio.h>
 #include "debug.h"
-#include "conf.h"
-#include "keyring.h"
-#include "commandline.h"
-
-__thread keyring_file *keyring = NULL;
+#include "conf.h" // for IF_DEBUG() macro
 
 // We don't want to call any at_exit functions from the dalvik VM
 void _exit(int status);
@@ -33,11 +29,3 @@ void exit(int status)
   fflush(stdout);
   _exit(status);
 }
-
-void command_cleanup()
-{
-  // This function is called after every CLI command has finished.
-  rhizome_close_db();
-  free_subscribers();
-  assert(keyring==NULL);
-}

commandline.c

@@ -74,7 +74,7 @@ int commandline_main(struct cli_context *context, const char *argv0, int argc, c
     break;
   }
 
-  command_cleanup();
+  CALL_TRIGGER(cmd_cleanup, context);
 
   OUT();
 
@@ -83,6 +83,15 @@ int commandline_main(struct cli_context *context, const char *argv0, int argc, c
   return result;
 }
 
+// Put a dummy no-op trigger callback into the "cmd_cleanup" trigger section,
+// otherwise if no other object provides one, the link will fail with errors like:
+// undefined reference to `__start_tr_cmd_cleanup'
+// undefined reference to `__stop_tr_cmd_cleanup'
+
+static void __dummy_on_cmd_cleanup();
+DEFINE_TRIGGER(cmd_cleanup, __dummy_on_cmd_cleanup);
+static void __dummy_on_cmd_cleanup() {}
+
 int commandline_main_stdio(FILE *output, const char *argv0, int argc, const char *const *args)
 {
   struct cli_context_stdio cli_context_stdio = {

commandline.h

@@ -23,6 +23,7 @@ Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
 
 #include <stdio.h> // for FILE
 #include "section.h"
+#include "trigger.h"
 #include "cli.h"
 
 #define KEYRING_PIN_OPTION	  ,"[--keyring-pin=<pin>]"
@@ -49,9 +50,9 @@ DECLARE_SECTION(struct cli_schema, commands);
 int commandline_main(struct cli_context *context, const char *argv0, int argc, const char *const *args);
 int commandline_main_stdio(FILE *output, const char *argv0, int argc, const char *const *args);
 
-/* Called after every command has finished.  Is not supplied by the
- * command-line object; the caller must define this function.
- */
-void command_cleanup();
+// Trigger that is called after every command has finished.  Different
+// sub-systems (eg, keyring, Rhizome) use this to reset their global state
+// ready for the next command.
+DECLARE_TRIGGER(cmd_cleanup, struct cli_context *context);
 
 #endif // __SERVAL_DNA__COMMANDLINE_H

keyring.c

@@ -1,6 +1,6 @@
 /*
-Serval DNA keyring
-Copyright (C) 2013 Serval Project Inc.
+Copyright (C) 2016 Flinders University
+Copyright (C) 2013-2015 Serval Project Inc.
 Copyright (C) 2010-2012 Paul Gardner-Stephen
  
 This program is free software; you can redistribute it and/or
@@ -32,6 +32,7 @@ Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
 #include "mem.h"
 #include "rotbuf.h"
 #include "route_link.h"
+#include "commandline.h"
 
 static keyring_file *keyring_open_or_create(const char *path, int writeable);
 static int keyring_initialise(keyring_file *k);
@@ -1938,7 +1939,6 @@ struct nm_record nm_cache[NM_CACHE_SLOTS];
 unsigned char *keyring_get_nm_bytes(const uint8_t *box_sk, const sid_t *box_pk, const sid_t *unknown_sidp)
 {
   IN();
-  assert(keyring != NULL);
 
   /* See if we have it cached already */
   unsigned i;
@@ -2109,3 +2109,16 @@ int keyring_load_from_dump(keyring_file *k, unsigned entry_pinc, const char **en
     return WHYF_perror("fscanf");
   return 0;
 }
+
+/* Free the global keyring after every CLI command.
+ */
+
+__thread keyring_file *keyring = NULL;
+
+static void keyring_on_cmd_cleanup();
+DEFINE_TRIGGER(cmd_cleanup, keyring_on_cmd_cleanup);
+static void keyring_on_cmd_cleanup()
+{
+  keyring_free(keyring);
+  keyring = NULL;
+}

keyring.h

@@ -1,6 +1,6 @@
 /* 
 Serval DNA keyring
-Copyright (C) 2013 Serval Project Inc.
+Copyright (C) 2013-2015 Serval Project Inc.
 Copyright (C) 2010-2012 Paul Gardner-Stephen
 
 This program is free software; you can redistribute it and/or
@@ -115,7 +115,7 @@ int keyring_release_subscriber(keyring_file *k, const sid_t *sid);
 // Combined signing / encryption key data
 #define KEYTYPE_CRYPTOCOMBINED 0x06
 
-/* handle to keyring file for use in running instance */
+/* per-thread global handle to keyring file for use in running commands and server */
 extern __thread keyring_file *keyring;
 
 /* Public calls to keyring management */

keyring_cli.c

@@ -54,22 +54,15 @@ static int app_keyring_dump(const struct cli_parsed *parsed, struct cli_context
   if (cli_arg(parsed, "file", &path, cli_path_regular, NULL) == -1)
     return -1;
   int include_secret = 0 == cli_arg(parsed, "--secret", NULL, NULL, NULL);
-  keyring_file *k = keyring_open_instance_cli(parsed);
-  if (!k)
+  assert(keyring == NULL);
+  if (!(keyring = keyring_open_instance_cli(parsed)))
     return -1;
   FILE *fp = path ? fopen(path, "w") : stdout;
-  if (fp == NULL) {
-    WHYF_perror("fopen(%s, \"w\")", alloca_str_toprint(path));
-    keyring_free(k);
-    return -1;
-  }
-  int ret = keyring_dump(k, XPRINTF_STDIO(fp), include_secret);
-  if (fp != stdout && fclose(fp) == EOF) {
-    WHYF_perror("fclose(%s)", alloca_str_toprint(path));
-    keyring_free(k);
-    return -1;
-  }
-  keyring_free(k);
+  if (fp == NULL)
+    return WHYF_perror("fopen(%s, \"w\")", alloca_str_toprint(path));
+  int ret = keyring_dump(keyring, XPRINTF_STDIO(fp), include_secret);
+  if (fp != stdout && fclose(fp) == EOF)
+    return WHYF_perror("fclose(%s)", alloca_str_toprint(path));
   return ret;
 }
 
@@ -94,24 +87,16 @@ static int app_keyring_load(const struct cli_parsed *parsed, struct cli_context
       assert(pc < pinc);
       pinv[pc++] = parsed->labelv[i].text;
     }
-  keyring_file *k = keyring_open_instance_cli(parsed);
-  if (!k)
+  assert(keyring == NULL);
+  if (!(keyring = keyring_open_instance_cli(parsed)))
     return -1;
   FILE *fp = path && strcmp(path, "-") != 0 ? fopen(path, "r") : stdin;
-  if (fp == NULL) {
-    WHYF_perror("fopen(%s, \"r\")", alloca_str_toprint(path));
-    keyring_free(k);
+  if (fp == NULL)
+    return WHYF_perror("fopen(%s, \"r\")", alloca_str_toprint(path));
+  if (keyring_load_from_dump(keyring, pinc, pinv, fp) == -1)
     return -1;
-  }
-  if (keyring_load_from_dump(k, pinc, pinv, fp) == -1) {
-    keyring_free(k);
-    return -1;
-  }
-  if (keyring_commit(k) == -1) {
-    keyring_free(k);
+  if (keyring_commit(keyring) == -1)
     return WHY("Could not write new identity");
-  }
-  keyring_free(k);
   return 0;
 }
 
@@ -121,8 +106,8 @@ DEFINE_CMD(app_keyring_list, 0,
 static int app_keyring_list(const struct cli_parsed *parsed, struct cli_context *context)
 {
   DEBUG_cli_parsed(verbose, parsed);
-  keyring_file *k = keyring_open_instance_cli(parsed);
-  if (!k)
+  assert(keyring == NULL);
+  if (!(keyring = keyring_open_instance_cli(parsed)))
     return -1;
     
   const char *names[]={
@@ -135,7 +120,7 @@ static int app_keyring_list(const struct cli_parsed *parsed, struct cli_context
   size_t rowcount = 0;
   
   keyring_iterator it;
-  keyring_iterator_start(k, &it);
+  keyring_iterator_start(keyring, &it);
   const keyring_identity *id;
   while((id = keyring_next_identity(&it))){
     const char *did = NULL;
@@ -147,7 +132,6 @@ static int app_keyring_list(const struct cli_parsed *parsed, struct cli_context
     cli_put_string(context, name, "\n");
     rowcount++;
   }
-  keyring_free(k);
   cli_end_table(context, rowcount);
   return 0;
 }
@@ -197,12 +181,12 @@ DEFINE_CMD(app_keyring_list2, 0, "List the full details of identities that can b
   "keyring", "list", "--full" KEYRING_PIN_OPTIONS);
 static int app_keyring_list2(const struct cli_parsed *parsed, struct cli_context *context)
 {
-  keyring_file *k = keyring_open_instance_cli(parsed);
-  if (!k)
+  assert(keyring == NULL);
+  if (!(keyring = keyring_open_instance_cli(parsed)))
     return -1;
     
   keyring_iterator it;
-  keyring_iterator_start(k, &it);
+  keyring_iterator_start(keyring, &it);
   const keyring_identity *id;
   while((id = keyring_next_identity(&it))){
     unsigned fields=0;
@@ -224,7 +208,6 @@ static int app_keyring_list2(const struct cli_parsed *parsed, struct cli_context
     cli_put_long(context, fields, "\n");
     cli_output_identity(context, id);
   }
-  keyring_free(k);
   return 0;
 }
 
@@ -236,22 +219,16 @@ static int app_keyring_add(const struct cli_parsed *parsed, struct cli_context *
   DEBUG_cli_parsed(verbose, parsed);
   const char *pin;
   cli_arg(parsed, "pin", &pin, NULL, "");
-  keyring_file *k = keyring_open_instance_cli(parsed);
-  if (!k)
+  assert(keyring == NULL);
+  if (!(keyring = keyring_open_instance_cli(parsed)))
     return -1;
-  keyring_enter_pin(k, pin);
-  
-  const keyring_identity *id = keyring_create_identity(k, pin);
-  if (id == NULL) {
-    keyring_free(k);
+  keyring_enter_pin(keyring, pin);
+  const keyring_identity *id = keyring_create_identity(keyring, pin);
+  if (id == NULL)
     return WHY("Could not create new identity");
-  }
-  if (keyring_commit(k) == -1) {
-    keyring_free(k);
+  if (keyring_commit(keyring) == -1)
     return WHY("Could not write new identity");
-  }
   cli_output_identity(context, id);
-  keyring_free(k);
   return 0;
 }
 
@@ -265,27 +242,20 @@ static int app_keyring_remove(const struct cli_parsed *parsed, struct cli_contex
   if (cli_arg(parsed, "sid", &sidhex, str_is_subscriber_id, "") == -1)
     return -1;
   sid_t sid;
-  if (str_to_sid_t(&sid, sidhex) == -1){
-    keyring_free(keyring);
-    keyring = NULL;
+  if (str_to_sid_t(&sid, sidhex) == -1)
     return WHY("str_to_sid_t() failed");
-  }
+  assert(keyring == NULL);
   if (!(keyring = keyring_open_instance_cli(parsed)))
     return -1;
   keyring_identity *id = keyring_find_identity_sid(keyring, &sid);
-  int r=0;
   if (!id)
-    r=WHY("No matching SID");
+    return WHY("No matching SID");
   keyring_destroy_identity(keyring, id);
-  if (keyring_commit(keyring) == -1) {
-    keyring_free(keyring);
+  if (keyring_commit(keyring) == -1)
     return WHY("Could not destroy identity");
-  }
   cli_output_identity(context, id);
   keyring_free_identity(id);
-  keyring_free(keyring);
-  keyring = NULL;
-  return r;
+  return 0;
 }
 
 DEFINE_CMD(app_keyring_set_did, 0,
@@ -306,31 +276,24 @@ static int app_keyring_set_did(const struct cli_parsed *parsed, struct cli_conte
 
   sid_t sid;
   if (str_to_sid_t(&sid, sidhex) == -1){
-    keyring_free(keyring);
-    keyring = NULL;
     return WHY("str_to_sid_t() failed");
   }
 
+  assert(keyring == NULL);
   if (!(keyring = keyring_open_instance_cli(parsed)))
     return -1;
 
   keyring_identity *id = keyring_find_identity_sid(keyring, &sid);
-
-  int r=0;
   if (!id)
-    r=WHY("No matching SID");
-  else if (keyring_set_did(id, did, name))
-      r=WHY("Could not set DID");
-  else if (set_pin && keyring_set_pin(id, new_pin))
-      r=WHY("Could not set new pin");
-  else if (keyring_commit(keyring))
-    r=WHY("Could not write updated keyring record");
-  else
-    cli_output_identity(context, id);
-
-  keyring_free(keyring);
-  keyring = NULL;
-  return r;
+    return WHY("No matching SID");
+  if (keyring_set_did(id, did, name))
+    return WHY("Could not set DID");
+  if (set_pin && keyring_set_pin(id, new_pin))
+    return WHY("Could not set new pin");
+  if (keyring_commit(keyring))
+    return WHY("Could not write updated keyring record");
+  cli_output_identity(context, id);
+  return 0;
 }
 
 DEFINE_CMD(app_keyring_set_tag, 0,
@@ -345,6 +308,7 @@ static int app_keyring_set_tag(const struct cli_parsed *parsed, struct cli_conte
       cli_arg(parsed, "value", &value, NULL, "") == -1 )
     return -1;
   
+  assert(keyring == NULL);
   if (!(keyring = keyring_open_instance_cli(parsed)))
     return -1;
 
@@ -353,25 +317,15 @@ static int app_keyring_set_tag(const struct cli_parsed *parsed, struct cli_conte
     return WHY("str_to_sid_t() failed");
 
   keyring_identity *id = keyring_find_identity_sid(keyring, &sid);
-  int r=0;
   if (!id)
-    r=WHY("No matching SID");
-  else{
-    int length = strlen(value);
-    if (keyring_set_public_tag(id, tag, (const unsigned char*)value, length))
-      r=WHY("Could not set tag value");
-    else{
-      if (keyring_commit(keyring))
-	r=WHY("Could not write updated keyring record");
-      else{
-	cli_output_identity(context, id);
-      }
-    }
-  }
-  
-  keyring_free(keyring);
-  keyring = NULL;
-  return r;
+    return WHY("No matching SID");
+  int length = strlen(value);
+  if (keyring_set_public_tag(id, tag, (const unsigned char*)value, length))
+    return WHY("Could not set tag value");
+  if (keyring_commit(keyring))
+    return WHY("Could not write updated keyring record");
+  cli_output_identity(context, id);
+  return 0;
 }
 
 static int handle_pins(const struct cli_parsed *parsed, struct cli_context *UNUSED(context), int revoke)

meshmb.c

@@ -59,21 +59,15 @@ static int app_meshmb_send(const struct cli_parsed *parsed, struct cli_context *
     return -1;
   if (rhizome_opendb() == -1)
     return -1;
+  assert(keyring == NULL);
   if (!(keyring = keyring_open_instance_cli(parsed)))
     return -1;
 
-  int ret = -1;
   keyring_identity *id = keyring_find_identity(keyring, &identity);
-  if (!id){
-    WHY("Invalid identity");
-    goto end;
-  }
+  if (!id)
+    return WHY("Invalid identity");
 
-  ret = meshmb_send(id, message, strlen(message)+1, nfields, fields);
-end:
-  keyring_free(keyring);
-  keyring = NULL;
-  return ret;
+  return meshmb_send(id, message, strlen(message)+1, nfields, fields);
 }
 
 DEFINE_CMD(app_meshmb_read, 0,

meshms_cli.c

@@ -36,6 +36,7 @@ static int app_meshms_conversations(const struct cli_parsed *parsed, struct cli_
 
   if (create_serval_instance_dir() == -1)
     goto end;
+  assert(keyring == NULL);
   if (!(keyring = keyring_open_instance_cli(parsed)))
     goto end;
 
@@ -89,9 +90,6 @@ static int app_meshms_conversations(const struct cli_parsed *parsed, struct cli_
 end:
   if (conv)
     meshms_free_conversations(conv);
-  if (keyring)
-    keyring_free(keyring);
-  keyring = NULL;
   return status;
 }
 
@@ -114,18 +112,14 @@ static int app_meshms_send_message(const struct cli_parsed *parsed, struct cli_c
 
   if (create_serval_instance_dir() == -1)
     return -1;
+  assert(keyring == NULL);
   if (!(keyring = keyring_open_instance_cli(parsed)))
     return -1;
-  if (rhizome_opendb() == -1){
-    keyring_free(keyring);
-    keyring = NULL;
+  if (rhizome_opendb() == -1)
     return -1;
-  }
 
   // include terminating NUL
   enum meshms_status status = meshms_send_message(&my_sid, &their_sid, message, strlen(message) + 1);
-  keyring_free(keyring);
-  keyring = NULL;
   return meshms_failed(status) ? status : 0;
 }
 
@@ -140,31 +134,20 @@ static int app_meshms_list_messages(const struct cli_parsed *parsed, struct cli_
     return -1;
   if (create_serval_instance_dir() == -1)
     return -1;
+  assert(keyring == NULL);
   if (!(keyring = keyring_open_instance_cli(parsed)))
     return -1;
-  if (rhizome_opendb() == -1){
-    keyring_free(keyring);
-    keyring = NULL;
+  if (rhizome_opendb() == -1)
     return -1;
-  }
   sid_t my_sid, their_sid;
-  if (str_to_sid_t(&my_sid, my_sidhex) == -1){
-    keyring_free(keyring);
-    keyring = NULL;
+  if (str_to_sid_t(&my_sid, my_sidhex) == -1)
     return WHY("invalid sender SID");
-  }
-  if (str_to_sid_t(&their_sid, their_sidhex) == -1){
-    keyring_free(keyring);
-    keyring = NULL;
+  if (str_to_sid_t(&their_sid, their_sidhex) == -1)
     return WHY("invalid recipient SID");
-  }
   struct meshms_message_iterator iter;
   enum meshms_status status;
-  if (meshms_failed(status = meshms_message_iterator_open(&iter, &my_sid, &their_sid))) {
-    keyring_free(keyring);
-    keyring = NULL;
+  if (meshms_failed(status = meshms_message_iterator_open(&iter, &my_sid, &their_sid)))
     return status;
-  }
   const char *names[]={
     "_id","my_offset","their_offset","age","type","message"
   };
@@ -218,8 +201,6 @@ static int app_meshms_list_messages(const struct cli_parsed *parsed, struct cli_
   if (!meshms_failed(status))
     cli_end_table(context, id);
   meshms_message_iterator_close(&iter);
-  keyring_free(keyring);
-  keyring = NULL;
   return status;
 }
 
@@ -236,35 +217,23 @@ static int app_meshms_mark_read(const struct cli_parsed *parsed, struct cli_cont
 
   if (create_serval_instance_dir() == -1)
     return -1;
+  assert(keyring == NULL);
   if (!(keyring = keyring_open_instance_cli(parsed)))
     return -1;
-  int ret = -1;
   if (rhizome_opendb() == -1)
-    goto done;
+    return -1;
   sid_t my_sid, their_sid;
-  if (str_to_sid_t(&my_sid, my_sidhex) == -1) {
-    ret = WHYF("my_sidhex=%s", my_sidhex);
-    goto done;
-  }
-  if (their_sidhex && str_to_sid_t(&their_sid, their_sidhex) == -1) {
-    ret = WHYF("their_sidhex=%s", their_sidhex);
-    goto done;
-  }
+  if (str_to_sid_t(&my_sid, my_sidhex) == -1)
+    return WHYF("my_sidhex=%s", my_sidhex);
+  if (their_sidhex && str_to_sid_t(&their_sid, their_sidhex) == -1)
+    return WHYF("their_sidhex=%s", their_sidhex);
   uint64_t offset = UINT64_MAX;
   if (offset_str) {
-    if (!their_sidhex) {
-      ret = WHY("missing recipient_sid");
-      goto done;
-    }
-    if (!str_to_uint64(offset_str, 10, &offset, NULL)) {
-      ret = WHYF("offset_str=%s", offset_str);
-      goto done;
-    }
+    if (!their_sidhex)
+      return WHY("missing recipient_sid");
+    if (!str_to_uint64(offset_str, 10, &offset, NULL))
+      return WHYF("offset_str=%s", offset_str);
   }
   enum meshms_status status = meshms_mark_read(&my_sid, their_sidhex ? &their_sid : NULL, offset);
-  ret = (status == MESHMS_STATUS_UPDATED) ? MESHMS_STATUS_OK : status;
-done:
-  keyring_free(keyring);
-  keyring = NULL;
-  return ret;
+  return (status == MESHMS_STATUS_UPDATED) ? MESHMS_STATUS_OK : status;
 }

overlay_address.c

@@ -38,6 +38,7 @@ Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
 #include "overlay_packet.h"
 #include "server.h"
 #include "route_link.h"
+#include "commandline.h"
 
 #define MAX_BPIS 1024
 #define BPI_MASK 0x3ff
@@ -106,6 +107,16 @@ void free_subscribers()
   tree_walk(&root, NULL, 0, free_node, NULL);
 }
 
+/* Free the subscribers tree after every CLI command.
+ */
+
+static void subscriber_on_cmd_cleanup();
+DEFINE_TRIGGER(cmd_cleanup, subscriber_on_cmd_cleanup);
+static void subscriber_on_cmd_cleanup()
+{
+  free_subscribers();
+}
+
 static void *create_subscriber(void *UNUSED(context), const uint8_t *binary, size_t bin_length)
 {
   assert(bin_length == SID_SIZE);

rhizome_cli.c

@@ -189,6 +189,7 @@ static int app_rhizome_add_file(const struct cli_parsed *parsed, struct cli_cont
   if (create_serval_instance_dir() == -1)
     return -1;
   
+  assert(keyring == NULL);
   if (!(keyring = keyring_open_instance_cli(parsed)))
     return -1;
   
@@ -339,8 +340,6 @@ static int app_rhizome_add_file(const struct cli_parsed *parsed, struct cli_cont
 finish:
   rhizome_bundle_result_free(&result);
   rhizome_manifest_free(m);
-  keyring_free(keyring);
-  keyring = NULL;
   return ret;
 }
 
@@ -435,48 +434,32 @@ static int app_rhizome_delete(const struct cli_parsed *parsed, struct cli_contex
     return -1;
   if (rhizome_opendb() == -1)
     return -1;
+  assert(keyring == NULL);
   if (!(keyring = keyring_open_instance_cli(parsed)))
     return -1;
   int ret=0;
   if (cli_arg(parsed, "file", NULL, NULL, NULL) == 0) {
-    if (!fileid){
-      keyring_free(keyring);
-      keyring = NULL;
+    if (!fileid)
       return WHY("missing <fileid> argument");
-    }
     rhizome_filehash_t hash;
-    if (str_to_rhizome_filehash_t(&hash, fileid) == -1){
-      keyring_free(keyring);
-      keyring = NULL;
+    if (str_to_rhizome_filehash_t(&hash, fileid) == -1)
       return WHYF("invalid <fileid> argument: %s", alloca_str_toprint(fileid));
-    }
     ret = rhizome_delete_file(&hash);
   } else {
-    if (!manifestid){
-      keyring_free(keyring);
-      keyring = NULL;
+    if (!manifestid)
       return WHY("missing <manifestid> argument");
-    }
     rhizome_bid_t bid;
-    if (str_to_rhizome_bid_t(&bid, manifestid) == -1){
-      keyring_free(keyring);
-      keyring = NULL;
+    if (str_to_rhizome_bid_t(&bid, manifestid) == -1)
       return WHY("Invalid manifest ID");
-    }
     if (cli_arg(parsed, "bundle", NULL, NULL, NULL) == 0)
       ret = rhizome_delete_bundle(&bid);
     else if (cli_arg(parsed, "manifest", NULL, NULL, NULL) == 0)
       ret = rhizome_delete_manifest(&bid);
     else if (cli_arg(parsed, "payload", NULL, NULL, NULL) == 0)
       ret = rhizome_delete_payload(&bid);
-    else{
-      keyring_free(keyring);
-      keyring = NULL;
+    else
       return WHY("unrecognised command");
-    }
   }
-  keyring_free(keyring);
-  keyring = NULL;
   return ret;
 }
 
@@ -545,6 +528,7 @@ static int app_rhizome_extract(const struct cli_parsed *parsed, struct cli_conte
   if (rhizome_opendb() == -1)
     return -1;
   
+  assert(keyring == NULL);
   if (!(keyring = keyring_open_instance_cli(parsed)))
     return -1;
   
@@ -640,8 +624,6 @@ static int app_rhizome_extract(const struct cli_parsed *parsed, struct cli_conte
   }
 finish:
   rhizome_manifest_free(m);
-  keyring_free(keyring);
-  keyring = NULL;
   return ret;
 }
 
@@ -704,13 +686,11 @@ static int app_rhizome_list(const struct cli_parsed *parsed, struct cli_context
   /* Create the instance directory if it does not yet exist */
   if (create_serval_instance_dir() == -1)
     return -1;
+  assert(keyring == NULL);
   if (!(keyring = keyring_open_instance_cli(parsed)))
     return -1;
-  if (rhizome_opendb() == -1) {
-    keyring_free(keyring);
-    keyring = NULL;
+  if (rhizome_opendb() == -1)
     return -1;
-  }
   size_t rowlimit = atoi(limit_ascii);
   size_t rowoffset = atoi(offset_ascii);
   struct rhizome_list_cursor cursor;
@@ -727,11 +707,8 @@ static int app_rhizome_list(const struct cli_parsed *parsed, struct cli_context
       return WHYF("Invalid <recipient: %s", recipient_hex);
     cursor.is_recipient_set = 1;
   }
-  if (rhizome_list_open(&cursor) == -1) {
-    keyring_free(keyring);
-    keyring = NULL;
+  if (rhizome_list_open(&cursor) == -1)
     return -1;
-  }
   const char *headers[]={
     "_id",
     "service",
@@ -796,8 +773,6 @@ static int app_rhizome_list(const struct cli_parsed *parsed, struct cli_context
     }
   }
   rhizome_list_release(&cursor);
-  keyring_free(keyring);
-  keyring = NULL;
   if (n == -1)
     return -1;
   cli_end_table(context, rowcount);

rhizome_database.c

@@ -30,6 +30,7 @@ Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
 #include "str.h"
 #include "keyring.h"
 #include "server.h"
+#include "commandline.h"
 
 static int rhizome_delete_manifest_retry(sqlite_retry_state *retry, const rhizome_bid_t *bidp);
 
@@ -398,6 +399,16 @@ int rhizome_close_db()
   OUT();
 }
 
+/* Close the Rhizome database after every CLI command.
+ */
+
+static void rhizome_on_cmd_cleanup();
+DEFINE_TRIGGER(cmd_cleanup, rhizome_on_cmd_cleanup);
+static void rhizome_on_cmd_cleanup()
+{
+  rhizome_close_db();
+}
+
 /* SQL query retry logic.
 
    The common retry-on-busy logic is factored into this function.  This logic encapsulates the

servald_features.c

@@ -59,19 +59,3 @@ void servald_features()
   USE_FEATURE(http_rest_meshms);
   USE_FEATURE(http_rest_meshmb);
 }
-
-#include <assert.h>
-#include "overlay_address.h"
-#include "rhizome.h"
-#include "cli.h"
-#include "keyring.h"
-
-__thread keyring_file *keyring = NULL;
-
-void command_cleanup()
-{
-  // This function is called after every CLI command has finished.
-  rhizome_close_db();
-  free_subscribers();
-  assert(keyring==NULL);
-}