[Patch v6 4/8] lib: add "verbose" versions of notmuch_database_{open,create}

David Bremner david at tethera.net
Fri Mar 27 15:11:56 PDT 2015


The compatibility wrapper ensures that clients calling
notmuch_database_open will receive consistent output for now.

The changes to notmuch-{new,search} and test/symbol-test are just to
make the test suite pass.

The use of IGNORE_RESULT is justified by two things. 1) I don't know
what else to do.  2) asprintf guarantees the output string is NULL if
an error occurs, so at least we are not passing garbage back.
---
 lib/database.cc     | 109 ++++++++++++++++++++++++++++++++++++++--------------
 lib/notmuch.h       |  21 ++++++++++
 notmuch-new.c       |  11 +++++-
 notmuch-search.c    |  13 ++++++-
 test/symbol-test.cc |   9 ++++-
 5 files changed, 130 insertions(+), 33 deletions(-)

diff --git a/lib/database.cc b/lib/database.cc
index 3974e2e..5d973b9 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -608,29 +608,50 @@ parse_references (void *ctx,
 notmuch_status_t
 notmuch_database_create (const char *path, notmuch_database_t **database)
 {
+    char *status_string = NULL;
+    notmuch_status_t status;
+
+    status = notmuch_database_create_verbose (path, database,
+					      &status_string);
+
+    if (status_string) {
+	fputs (status_string, stderr);
+	free (status_string);
+    }
+
+    return status;
+}
+
+notmuch_status_t
+notmuch_database_create_verbose (const char *path,
+				 notmuch_database_t **database,
+				 char **status_string)
+{
     notmuch_status_t status = NOTMUCH_STATUS_SUCCESS;
     notmuch_database_t *notmuch = NULL;
     char *notmuch_path = NULL;
+    char *message = NULL;
     struct stat st;
     int err;
 
     if (path == NULL) {
-	fprintf (stderr, "Error: Cannot create a database for a NULL path.\n");
+	message = strdup ("Error: Cannot create a database for a NULL path.\n");
 	status = NOTMUCH_STATUS_NULL_POINTER;
 	goto DONE;
     }
 
     err = stat (path, &st);
     if (err) {
-	fprintf (stderr, "Error: Cannot create database at %s: %s.\n",
-		 path, strerror (errno));
+	IGNORE_RESULT (asprintf (&message, "Error: Cannot create database at %s: %s.\n",
+				path, strerror (errno)));
 	status = NOTMUCH_STATUS_FILE_ERROR;
 	goto DONE;
     }
 
     if (! S_ISDIR (st.st_mode)) {
-	fprintf (stderr, "Error: Cannot create database at %s: Not a directory.\n",
-		 path);
+	IGNORE_RESULT (asprintf (&message, "Error: Cannot create database at %s: "
+				 "Not a directory.\n",
+				 path));
 	status = NOTMUCH_STATUS_FILE_ERROR;
 	goto DONE;
     }
@@ -640,15 +661,15 @@ notmuch_database_create (const char *path, notmuch_database_t **database)
     err = mkdir (notmuch_path, 0755);
 
     if (err) {
-	fprintf (stderr, "Error: Cannot create directory %s: %s.\n",
-		 notmuch_path, strerror (errno));
+	IGNORE_RESULT (asprintf (&message, "Error: Cannot create directory %s: %s.\n",
+				 notmuch_path, strerror (errno)));
 	status = NOTMUCH_STATUS_FILE_ERROR;
 	goto DONE;
     }
 
-    status = notmuch_database_open (path,
-				    NOTMUCH_DATABASE_MODE_READ_WRITE,
-				    &notmuch);
+    status = notmuch_database_open_verbose (path,
+					    NOTMUCH_DATABASE_MODE_READ_WRITE,
+					    &notmuch, &message);
     if (status)
 	goto DONE;
 
@@ -667,6 +688,9 @@ notmuch_database_create (const char *path, notmuch_database_t **database)
     if (notmuch_path)
 	talloc_free (notmuch_path);
 
+    if (status_string && message)
+	*status_string = message;
+
     if (database)
 	*database = notmuch;
     else
@@ -767,37 +791,58 @@ notmuch_database_open (const char *path,
 		       notmuch_database_mode_t mode,
 		       notmuch_database_t **database)
 {
+    char *status_string = NULL;
+    notmuch_status_t status;
+
+    status = notmuch_database_open_verbose (path, mode, database,
+					   &status_string);
+
+    if (status_string) {
+	fputs (status_string, stderr);
+	free (status_string);
+    }
+
+    return status;
+}
+
+notmuch_status_t
+notmuch_database_open_verbose (const char *path,
+			       notmuch_database_mode_t mode,
+			       notmuch_database_t **database,
+			       char **status_string)
+{
     notmuch_status_t status = NOTMUCH_STATUS_SUCCESS;
     void *local = talloc_new (NULL);
     notmuch_database_t *notmuch = NULL;
     char *notmuch_path, *xapian_path, *incompat_features;
+    char *message = NULL;
     struct stat st;
     int err;
     unsigned int i, version;
     static int initialized = 0;
 
     if (path == NULL) {
-	fprintf (stderr, "Error: Cannot open a database for a NULL path.\n");
+	message = strdup ("Error: Cannot open a database for a NULL path.\n");
 	status = NOTMUCH_STATUS_NULL_POINTER;
 	goto DONE;
     }
 
     if (! (notmuch_path = talloc_asprintf (local, "%s/%s", path, ".notmuch"))) {
-	fprintf (stderr, "Out of memory\n");
+	message = strdup ("Out of memory\n");
 	status = NOTMUCH_STATUS_OUT_OF_MEMORY;
 	goto DONE;
     }
 
     err = stat (notmuch_path, &st);
     if (err) {
-	fprintf (stderr, "Error opening database at %s: %s\n",
-		 notmuch_path, strerror (errno));
+	IGNORE_RESULT (asprintf (&message, "Error opening database at %s: %s\n",
+				 notmuch_path, strerror (errno)));
 	status = NOTMUCH_STATUS_FILE_ERROR;
 	goto DONE;
     }
 
     if (! (xapian_path = talloc_asprintf (local, "%s/%s", notmuch_path, "xapian"))) {
-	fprintf (stderr, "Out of memory\n");
+	message = strdup ("Out of memory\n");
 	status = NOTMUCH_STATUS_OUT_OF_MEMORY;
 	goto DONE;
     }
@@ -837,11 +882,11 @@ notmuch_database_open (const char *path,
 	 * means a dramatically incompatible change. */
 	version = notmuch_database_get_version (notmuch);
 	if (version > NOTMUCH_DATABASE_VERSION) {
-	    fprintf (stderr,
-		     "Error: Notmuch database at %s\n"
-		     "       has a newer database format version (%u) than supported by this\n"
-		     "       version of notmuch (%u).\n",
-		     notmuch_path, version, NOTMUCH_DATABASE_VERSION);
+	    IGNORE_RESULT (asprintf (&message,
+		      "Error: Notmuch database at %s\n"
+		      "       has a newer database format version (%u) than supported by this\n"
+		      "       version of notmuch (%u).\n",
+				     notmuch_path, version, NOTMUCH_DATABASE_VERSION));
 	    notmuch->mode = NOTMUCH_DATABASE_MODE_READ_ONLY;
 	    notmuch_database_destroy (notmuch);
 	    notmuch = NULL;
@@ -856,11 +901,11 @@ notmuch_database_open (const char *path,
 	    version, mode == NOTMUCH_DATABASE_MODE_READ_WRITE ? 'w' : 'r',
 	    &incompat_features);
 	if (incompat_features) {
-	    fprintf (stderr,
-		     "Error: Notmuch database at %s\n"
-		     "       requires features (%s)\n"
-		     "       not supported by this version of notmuch.\n",
-		     notmuch_path, incompat_features);
+	    IGNORE_RESULT (asprintf (&message,
+		"Error: Notmuch database at %s\n"
+		"       requires features (%s)\n"
+		"       not supported by this version of notmuch.\n",
+				     notmuch_path, incompat_features));
 	    notmuch->mode = NOTMUCH_DATABASE_MODE_READ_ONLY;
 	    notmuch_database_destroy (notmuch);
 	    notmuch = NULL;
@@ -906,8 +951,8 @@ notmuch_database_open (const char *path,
 	    notmuch->query_parser->add_prefix (prefix->name, prefix->prefix);
 	}
     } catch (const Xapian::Error &error) {
-	fprintf (stderr, "A Xapian exception occurred opening database: %s\n",
-		 error.get_msg().c_str());
+	IGNORE_RESULT (asprintf (&message, "A Xapian exception occurred opening database: %s\n",
+				 error.get_msg().c_str()));
 	notmuch_database_destroy (notmuch);
 	notmuch = NULL;
 	status = NOTMUCH_STATUS_XAPIAN_EXCEPTION;
@@ -916,6 +961,9 @@ notmuch_database_open (const char *path,
   DONE:
     talloc_free (local);
 
+    if (status_string && message)
+	*status_string = message;
+
     if (database)
 	*database = notmuch;
     else
@@ -1039,13 +1087,18 @@ notmuch_database_compact (const char *path,
     notmuch_database_t *notmuch = NULL;
     struct stat statbuf;
     notmuch_bool_t keep_backup;
+    char *message = NULL;
 
     local = talloc_new (NULL);
     if (! local)
 	return NOTMUCH_STATUS_OUT_OF_MEMORY;
 
-    ret = notmuch_database_open (path, NOTMUCH_DATABASE_MODE_READ_WRITE, &notmuch);
+    ret = notmuch_database_open_verbose (path,
+					 NOTMUCH_DATABASE_MODE_READ_WRITE,
+					 &notmuch,
+					 &message);
     if (ret) {
+	if (status_cb) status_cb (message, closure);
 	goto DONE;
     }
 
diff --git a/lib/notmuch.h b/lib/notmuch.h
index f066245..c671d82 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -230,6 +230,16 @@ notmuch_status_t
 notmuch_database_create (const char *path, notmuch_database_t **database);
 
 /**
+ * Like notmuch_database_create, except optionally return an error
+ * message. This message is allocated by malloc and should be freed by
+ * the caller.
+ */
+notmuch_status_t
+notmuch_database_create_verbose (const char *path,
+				 notmuch_database_t **database,
+				 char **error_message);
+
+/**
  * Database open mode for notmuch_database_open.
  */
 typedef enum {
@@ -279,6 +289,17 @@ notmuch_status_t
 notmuch_database_open (const char *path,
 		       notmuch_database_mode_t mode,
 		       notmuch_database_t **database);
+/**
+ * Like notmuch_database_open, except optionally return an error
+ * message. This message is allocated by malloc and should be freed by
+ * the caller.
+ */
+
+notmuch_status_t
+notmuch_database_open_verbose (const char *path,
+			       notmuch_database_mode_t mode,
+			       notmuch_database_t **database,
+			       char **error_message);
 
 /**
  * Commit changes and close the given notmuch database.
diff --git a/notmuch-new.c b/notmuch-new.c
index ddf42c1..e6c283e 100644
--- a/notmuch-new.c
+++ b/notmuch-new.c
@@ -985,9 +985,16 @@ notmuch_new_command (notmuch_config_t *config, int argc, char *argv[])
 	    return EXIT_FAILURE;
 	add_files_state.total_files = count;
     } else {
-	if (notmuch_database_open (db_path, NOTMUCH_DATABASE_MODE_READ_WRITE,
-				   &notmuch))
+	char *status_string = NULL;
+	if (notmuch_database_open_verbose (db_path, NOTMUCH_DATABASE_MODE_READ_WRITE,
+					   &notmuch, &status_string)) {
+	    if (status_string) {
+		fputs (status_string, stderr);
+		free (status_string);
+	    }
+
 	    return EXIT_FAILURE;
+	}
 
 	if (notmuch_database_needs_upgrade (notmuch)) {
 	    time_t now = time (NULL);
diff --git a/notmuch-search.c b/notmuch-search.c
index a591d45..b81ac01 100644
--- a/notmuch-search.c
+++ b/notmuch-search.c
@@ -545,6 +545,7 @@ _notmuch_search_prepare (search_context_t *ctx, notmuch_config_t *config, int ar
 {
     char *query_str;
     unsigned int i;
+    char *status_string = NULL;
 
     switch (ctx->format_sel) {
     case NOTMUCH_FORMAT_TEXT:
@@ -570,9 +571,17 @@ _notmuch_search_prepare (search_context_t *ctx, notmuch_config_t *config, int ar
 
     notmuch_exit_if_unsupported_format ();
 
-    if (notmuch_database_open (notmuch_config_get_database_path (config),
-			       NOTMUCH_DATABASE_MODE_READ_ONLY, &ctx->notmuch))
+    if (notmuch_database_open_verbose (
+	    notmuch_config_get_database_path (config),
+	    NOTMUCH_DATABASE_MODE_READ_ONLY, &ctx->notmuch, &status_string)) {
+
+	if (status_string) {
+	    fputs (status_string, stderr);
+	    free (status_string);
+	}
+
 	return EXIT_FAILURE;
+    }
 
     query_str = query_string_from_args (ctx->notmuch, argc, argv);
     if (query_str == NULL) {
diff --git a/test/symbol-test.cc b/test/symbol-test.cc
index 3e96c03..d979f83 100644
--- a/test/symbol-test.cc
+++ b/test/symbol-test.cc
@@ -1,11 +1,18 @@
 #include <stdio.h>
+#include <stdlib.h>
 #include <xapian.h>
 #include <notmuch.h>
 
 
 int main() {
   notmuch_database_t *notmuch;
-  notmuch_database_open("fakedb", NOTMUCH_DATABASE_MODE_READ_ONLY, &notmuch);
+  char *message = NULL;
+
+  if (notmuch_database_open_verbose  ("fakedb", NOTMUCH_DATABASE_MODE_READ_ONLY, &notmuch, &message))
+      if (message) {
+	  fputs (message, stderr);
+	  free (message);
+      }
 
   try {
     (void) new Xapian::WritableDatabase("./nonexistant", Xapian::DB_OPEN);
-- 
2.1.4



More information about the notmuch mailing list