libnotmuch logging overhaul v3

libnotmuch logging overhaul v3

[David Bremner]

changes since the last round [1]

- remove unused variable
- fix the internal usage of n_d_open to use _verbose version
- revert the hiding of query debugging.

[1]: id:1419717937-1108-2-git-send-email-david@tethera.net

diff --git a/lib/database.cc b/lib/database.cc
index 16cd940..b144885 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -377,7 +377,6 @@ _notmuch_database_log (notmuch_database_t *notmuch,
 		      ...)
 {
     va_list va_args;
-    const char *message;
 
     va_start (va_args, format);
 
@@ -693,9 +692,9 @@ notmuch_database_create_verbose (const char *path,
 	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;
 
@@ -1101,13 +1100,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/query.cc b/lib/query.cc
index 2e20ab2..9ed420c 100644
--- a/lib/query.cc
+++ b/lib/query.cc
@@ -78,7 +78,7 @@ notmuch_query_create (notmuch_database_t *notmuch,
     notmuch_query_t *query;
 
     if (_debug_query ())
-	_notmuch_database_log (notmuch, "Query string is:\n%s\n", query_string);
+	fprintf (stderr, "Query string is:\n%s\n", query_string);
 
     query = talloc (notmuch, notmuch_query_t);
     if (unlikely (query == NULL))
@@ -266,9 +266,9 @@ notmuch_query_search_messages (notmuch_query_t *query)
 	}
 
 	if (_debug_query ()) {
-	    _notmuch_database_log (notmuch, "Exclude query is:\n%s\n",
+	    fprintf (stderr, "Exclude query is:\n%s\n",
 		     exclude_query.get_description ().c_str ());
-	    _notmuch_database_log (notmuch, "Final query is:\n%s\n",
+	    fprintf (stderr, "Final query is:\n%s\n",
 		     final_query.get_description ().c_str ());
 	}
 
@@ -549,9 +549,9 @@ notmuch_query_count_messages (notmuch_query_t *query)
 	enquire.set_docid_order(Xapian::Enquire::ASCENDING);
 
 	if (_debug_query ()) {
-	    _notmuch_database_log (notmuch, "Exclude query is:\n%s\n",
+	    fprintf (stderr, "Exclude query is:\n%s\n",
 		     exclude_query.get_description ().c_str ());
-	    _notmuch_database_log (notmuch, "Final query is:\n%s\n",
+	    fprintf (stderr, "Final query is:\n%s\n",
 		     final_query.get_description ().c_str ());
 	}
 

 lib/database.cc | 14 +++++++++-----
 lib/query.cc    | 10 +++++-----
 2 files changed, 14 insertions(+), 10 deletions(-)

[Patch v3 1/6] lib: add "verbose" versions of notmuch_database_{open,create}

[David Bremner]

This has the the non-trivial side effect silencing all output to
stderror from these two routines.

The stdargs based infrastucture will be used in following commits for
a more general logging mechanism.

The changes to notmuch-{new,search} and test/symbol-test are just to
make the test suite pass. The fact that no other changes are needed is
probably a sign of weakness in the test suite.
---
 lib/database.cc     | 88 ++++++++++++++++++++++++++++++++++++++++++++---------
 lib/notmuch.h       | 21 +++++++++++++
 notmuch-new.c       |  8 +++--
 notmuch-search.c    |  8 +++--
 test/symbol-test.cc |  6 +++-
 5 files changed, 111 insertions(+), 20 deletions(-)

diff --git a/lib/database.cc b/lib/database.cc
index 3601f9d..4f4e871 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -343,6 +343,35 @@ notmuch_status_to_string (notmuch_status_t status)
 }
 
 static void
+vlog_to_string (void *ctx,
+	       char **status_string,
+	       const char *format,
+	       va_list va_args)
+{
+    if (!status_string)
+	return;
+
+    if (*status_string)
+	talloc_free (*status_string);
+
+    *status_string = talloc_vasprintf (ctx, format, va_args);
+}
+
+static void
+log_to_string (char **str,
+	       const char *format,
+	       ...)
+{
+    va_list va_args;
+
+    va_start (va_args, format);
+
+    vlog_to_string (NULL, str, format, va_args);
+
+    va_end (va_args);
+}
+
+static void
 find_doc_ids_for_term (notmuch_database_t *notmuch,
 		       const char *term,
 		       Xapian::PostingIterator *begin,
@@ -602,28 +631,37 @@ parse_references (void *ctx,
 notmuch_status_t
 notmuch_database_create (const char *path, notmuch_database_t **database)
 {
+    return notmuch_database_create_verbose (path, database, NULL);
+}
+
+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");
+	log_to_string (&message, "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",
+	log_to_string (&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",
+	log_to_string (&message, "Error: Cannot create database at %s: Not a directory.\n",
 		 path);
 	status = NOTMUCH_STATUS_FILE_ERROR;
 	goto DONE;
@@ -634,15 +672,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",
+	log_to_string (&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;
 
@@ -660,6 +698,8 @@ notmuch_database_create (const char *path, notmuch_database_t **database)
     if (notmuch_path)
 	talloc_free (notmuch_path);
 
+    if (message)
+	*status_string = strdup(message);
     if (database)
 	*database = notmuch;
     else
@@ -760,37 +800,47 @@ notmuch_database_open (const char *path,
 		       notmuch_database_mode_t mode,
 		       notmuch_database_t **database)
 {
+    return notmuch_database_open_verbose(path, mode, database, NULL);
+}
+
+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");
+	log_to_string (&message, "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");
+	log_to_string (&message, "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",
+	log_to_string (&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");
+	log_to_string (&message, "Out of memory\n");
 	status = NOTMUCH_STATUS_OUT_OF_MEMORY;
 	goto DONE;
     }
@@ -830,7 +880,7 @@ notmuch_database_open (const char *path,
 	 * means a dramatically incompatible change. */
 	version = notmuch_database_get_version (notmuch);
 	if (version > NOTMUCH_DATABASE_VERSION) {
-	    fprintf (stderr,
+	    log_to_string (&message,
 		     "Error: Notmuch database at %s\n"
 		     "       has a newer database format version (%u) than supported by this\n"
 		     "       version of notmuch (%u).\n",
@@ -849,7 +899,7 @@ notmuch_database_open (const char *path,
 	    version, mode == NOTMUCH_DATABASE_MODE_READ_WRITE ? 'w' : 'r',
 	    &incompat_features);
 	if (incompat_features) {
-	    fprintf (stderr,
+	    log_to_string (&message,
 		     "Error: Notmuch database at %s\n"
 		     "       requires features (%s)\n"
 		     "       not supported by this version of notmuch.\n",
@@ -899,7 +949,7 @@ 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",
+	log_to_string (&message, "A Xapian exception occurred opening database: %s\n",
 		 error.get_msg().c_str());
 	notmuch_database_destroy (notmuch);
 	notmuch = NULL;
@@ -909,6 +959,9 @@ notmuch_database_open (const char *path,
   DONE:
     talloc_free (local);
 
+    if (status_string && message)
+	*status_string = strdup (message);
+
     if (database)
 	*database = notmuch;
     else
@@ -1032,13 +1085,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 220839b..0dfac8f 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..93b70bf 100644
--- a/notmuch-new.c
+++ b/notmuch-new.c
@@ -985,9 +985,13 @@ 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);
+
 	    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 14b9f01..250e567 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,12 @@ _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);
 	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..9f8eea7 100644
--- a/test/symbol-test.cc
+++ b/test/symbol-test.cc
@@ -5,7 +5,11 @@
 
 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);
+
 
   try {
     (void) new Xapian::WritableDatabase("./nonexistant", Xapian::DB_OPEN);
-- 
2.1.3

[Patch v3 2/6] lib/database: add field for last error string

[David Bremner]

The idea is to have a logging function setting this string instead of
printing to stderr.
---
 lib/database-private.h | 4 ++++
 lib/database.cc        | 7 +++++++
 lib/notmuch.h          | 7 +++++++
 3 files changed, 18 insertions(+)

diff --git a/lib/database-private.h b/lib/database-private.h
index 15e03cc..7efd98b 100644
--- a/lib/database-private.h
+++ b/lib/database-private.h
@@ -146,6 +146,10 @@ struct _notmuch_database {
     unsigned int last_doc_id;
     uint64_t last_thread_id;
 
+    /* error reporting; this value persists only until the
+     * next library call. May be NULL */
+    char *status_string;
+
     Xapian::QueryParser *query_parser;
     Xapian::TermGenerator *term_gen;
     Xapian::ValueRangeProcessor *value_range_processor;
diff --git a/lib/database.cc b/lib/database.cc
index 4f4e871..18db902 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -858,6 +858,7 @@ notmuch_database_open_verbose (const char *path,
 
     notmuch = talloc_zero (NULL, notmuch_database_t);
     notmuch->exception_reported = FALSE;
+    notmuch->status_string = NULL;
     notmuch->path = talloc_strdup (notmuch, path);
 
     if (notmuch->path[strlen (notmuch->path) - 1] == '/')
@@ -2543,3 +2544,9 @@ notmuch_database_get_all_tags (notmuch_database_t *db)
 	return NULL;
     }
 }
+
+const char *
+notmuch_database_status_string (notmuch_database_t *notmuch)
+{
+    return notmuch->status_string;
+}
diff --git a/lib/notmuch.h b/lib/notmuch.h
index 0dfac8f..2ab2998 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -302,6 +302,13 @@ notmuch_database_open_verbose (const char *path,
 			       char **error_message);
 
 /**
+ * Retrieve last status string for given database.
+ *
+ */
+const char *
+notmuch_database_status_string (notmuch_database_t *notmuch);
+
+/**
  * Commit changes and close the given notmuch database.
  *
  * After notmuch_database_close has been called, calls to other
-- 
2.1.3

[Patch v3 3/6] lib: add a log function with output to a string in notmuch_database_t

[David Bremner]

This is a thin wrapper around the previously implemented string
logging infrastructure. In the future it could have more configurable
output options.
---
 lib/database.cc       | 14 ++++++++++++++
 lib/notmuch-private.h |  4 ++++
 2 files changed, 18 insertions(+)

diff --git a/lib/database.cc b/lib/database.cc
index 18db902..9af1a47 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -371,6 +371,20 @@ log_to_string (char **str,
     va_end (va_args);
 }
 
+void
+_notmuch_database_log (notmuch_database_t *notmuch,
+		      const char *format,
+		      ...)
+{
+    va_list va_args;
+
+    va_start (va_args, format);
+
+    vlog_to_string (notmuch, &notmuch->status_string, format, va_args);
+
+    va_end (va_args);
+}
+
 static void
 find_doc_ids_for_term (notmuch_database_t *notmuch,
 		       const char *term,
diff --git a/lib/notmuch-private.h b/lib/notmuch-private.h
index 012ad25..7c6cfc0 100644
--- a/lib/notmuch-private.h
+++ b/lib/notmuch-private.h
@@ -191,6 +191,10 @@ _notmuch_message_id_compressed (void *ctx, const char *message_id);
 notmuch_status_t
 _notmuch_database_ensure_writable (notmuch_database_t *notmuch);
 
+void
+_notmuch_database_log (notmuch_database_t *notmuch,
+		       const char *format, ...);
+
 const char *
 _notmuch_database_relative_path (notmuch_database_t *notmuch,
 				 const char *path);
-- 
2.1.3

[Patch v3 4/6] lib: add private function to extract the database for a message.

[David Bremner]

This is needed by logging in functions outside message.cc that take
only a notmuch_message_t object.
---
 lib/message.cc        | 6 ++++++
 lib/notmuch-private.h | 2 ++
 2 files changed, 8 insertions(+)

diff --git a/lib/message.cc b/lib/message.cc
index a7a13cc..43cc078 100644
--- a/lib/message.cc
+++ b/lib/message.cc
@@ -1625,3 +1625,9 @@ notmuch_message_destroy (notmuch_message_t *message)
 {
     talloc_free (message);
 }
+
+notmuch_database_t *
+_notmuch_message_database (notmuch_message_t *message)
+{
+    return message->notmuch;
+}
diff --git a/lib/notmuch-private.h b/lib/notmuch-private.h
index 7c6cfc0..fad3a92 100644
--- a/lib/notmuch-private.h
+++ b/lib/notmuch-private.h
@@ -473,6 +473,8 @@ _notmuch_doc_id_set_remove (notmuch_doc_id_set_t *doc_ids,
 void
 _notmuch_message_add_reply (notmuch_message_t *message,
 			    notmuch_message_t *reply);
+notmuch_database_t *
+_notmuch_message_database (notmuch_message_t *message);
 
 /* sha1.c */
 
-- 
2.1.3

[Patch v3 5/6] lib: replace almost all fprintfs in library with _n_d_log

[David Bremner]

This is not supposed to change any functionality from an end user
point of view. Note that it will eliminate some output to stderr. The
query debugging output is left as is; it doesn't really fit with the
current primitive logging model. The remaining "bad" fprintf will need
an internal API change.
---
 lib/database.cc  | 34 +++++++++++++++++-----------------
 lib/directory.cc |  4 ++--
 lib/index.cc     | 11 +++++++----
 lib/message.cc   |  6 +++---
 lib/query.cc     |  8 ++++----
 5 files changed, 33 insertions(+), 30 deletions(-)

diff --git a/lib/database.cc b/lib/database.cc
index 9af1a47..19f5f0e 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -493,7 +493,7 @@ notmuch_database_find_message (notmuch_database_t *notmuch,
 
 	return NOTMUCH_STATUS_SUCCESS;
     } catch (const Xapian::Error &error) {
-	fprintf (stderr, "A Xapian exception occurred finding message: %s.\n",
+	_notmuch_database_log (notmuch, "A Xapian exception occurred finding message: %s.\n",
 		 error.get_msg().c_str());
 	notmuch->exception_reported = TRUE;
 	*message_ret = NULL;
@@ -725,7 +725,7 @@ notmuch_status_t
 _notmuch_database_ensure_writable (notmuch_database_t *notmuch)
 {
     if (notmuch->mode == NOTMUCH_DATABASE_MODE_READ_ONLY) {
-	fprintf (stderr, "Cannot write to a read-only database.\n");
+	_notmuch_database_log (notmuch, "Cannot write to a read-only database.\n");
 	return NOTMUCH_STATUS_READ_ONLY_DATABASE;
     }
 
@@ -1010,7 +1010,7 @@ notmuch_database_close (notmuch_database_t *notmuch)
 	} catch (const Xapian::Error &error) {
 	    status = NOTMUCH_STATUS_XAPIAN_EXCEPTION;
 	    if (! notmuch->exception_reported) {
-		fprintf (stderr, "Error: A Xapian exception occurred closing database: %s\n",
+		_notmuch_database_log (notmuch, "Error: A Xapian exception occurred closing database: %s\n",
 			 error.get_msg().c_str());
 	    }
 	}
@@ -1142,12 +1142,12 @@ notmuch_database_compact (const char *path,
     }
 
     if (stat (backup_path, &statbuf) != -1) {
-	fprintf (stderr, "Path already exists: %s\n", backup_path);
+	_notmuch_database_log (notmuch, "Path already exists: %s\n", backup_path);
 	ret = NOTMUCH_STATUS_FILE_ERROR;
 	goto DONE;
     }
     if (errno != ENOENT) {
-	fprintf (stderr, "Unknown error while stat()ing path: %s\n",
+	_notmuch_database_log (notmuch, "Unknown error while stat()ing path: %s\n",
 		 strerror (errno));
 	ret = NOTMUCH_STATUS_FILE_ERROR;
 	goto DONE;
@@ -1167,20 +1167,20 @@ notmuch_database_compact (const char *path,
 	compactor.set_destdir (compact_xapian_path);
 	compactor.compact ();
     } catch (const Xapian::Error &error) {
-	fprintf (stderr, "Error while compacting: %s\n", error.get_msg().c_str());
+	_notmuch_database_log (notmuch, "Error while compacting: %s\n", error.get_msg().c_str());
 	ret = NOTMUCH_STATUS_XAPIAN_EXCEPTION;
 	goto DONE;
     }
 
     if (rename (xapian_path, backup_path)) {
-	fprintf (stderr, "Error moving %s to %s: %s\n",
+	_notmuch_database_log (notmuch, "Error moving %s to %s: %s\n",
 		 xapian_path, backup_path, strerror (errno));
 	ret = NOTMUCH_STATUS_FILE_ERROR;
 	goto DONE;
     }
 
     if (rename (compact_xapian_path, xapian_path)) {
-	fprintf (stderr, "Error moving %s to %s: %s\n",
+	_notmuch_database_log (notmuch, "Error moving %s to %s: %s\n",
 		 compact_xapian_path, xapian_path, strerror (errno));
 	ret = NOTMUCH_STATUS_FILE_ERROR;
 	goto DONE;
@@ -1188,7 +1188,7 @@ notmuch_database_compact (const char *path,
 
     if (! keep_backup) {
 	if (rmtree (backup_path)) {
-	    fprintf (stderr, "Error removing old database %s: %s\n",
+	    _notmuch_database_log (notmuch, "Error removing old database %s: %s\n",
 		     backup_path, strerror (errno));
 	    ret = NOTMUCH_STATUS_FILE_ERROR;
 	    goto DONE;
@@ -1217,7 +1217,7 @@ notmuch_database_compact (unused (const char *path),
 			  unused (notmuch_compact_status_cb_t status_cb),
 			  unused (void *closure))
 {
-    fprintf (stderr, "notmuch was compiled against a xapian version lacking compaction support.\n");
+    _notmuch_database_log (notmuch, "notmuch was compiled against a xapian version lacking compaction support.\n");
     return NOTMUCH_STATUS_UNSUPPORTED_OPERATION;
 }
 #endif
@@ -1495,7 +1495,7 @@ notmuch_database_upgrade (notmuch_database_t *notmuch,
 	    }
 
 	    if (private_status) {
-		fprintf (stderr,
+		_notmuch_database_log (notmuch,
 			 "Upgrade failed while creating ghost messages.\n");
 		status = COERCE_STATUS (private_status, "Unexpected status from _notmuch_message_initialize_ghost");
 		goto DONE;
@@ -1545,7 +1545,7 @@ notmuch_database_begin_atomic (notmuch_database_t *notmuch)
     try {
 	(static_cast <Xapian::WritableDatabase *> (notmuch->xapian_db))->begin_transaction (false);
     } catch (const Xapian::Error &error) {
-	fprintf (stderr, "A Xapian exception occurred beginning transaction: %s.\n",
+	_notmuch_database_log (notmuch, "A Xapian exception occurred beginning transaction: %s.\n",
 		 error.get_msg().c_str());
 	notmuch->exception_reported = TRUE;
 	return NOTMUCH_STATUS_XAPIAN_EXCEPTION;
@@ -1579,7 +1579,7 @@ notmuch_database_end_atomic (notmuch_database_t *notmuch)
 	if (thresh && atoi (thresh) == 1)
 	    db->flush ();
     } catch (const Xapian::Error &error) {
-	fprintf (stderr, "A Xapian exception occurred committing transaction: %s.\n",
+	_notmuch_database_log (notmuch, "A Xapian exception occurred committing transaction: %s.\n",
 		 error.get_msg().c_str());
 	notmuch->exception_reported = TRUE;
 	return NOTMUCH_STATUS_XAPIAN_EXCEPTION;
@@ -1825,7 +1825,7 @@ notmuch_database_get_directory (notmuch_database_t *notmuch,
 	*directory = _notmuch_directory_create (notmuch, path,
 						NOTMUCH_FIND_LOOKUP, &status);
     } catch (const Xapian::Error &error) {
-	fprintf (stderr, "A Xapian exception occurred getting directory: %s.\n",
+	_notmuch_database_log (notmuch, "A Xapian exception occurred getting directory: %s.\n",
 		 error.get_msg().c_str());
 	notmuch->exception_reported = TRUE;
 	status = NOTMUCH_STATUS_XAPIAN_EXCEPTION;
@@ -2407,7 +2407,7 @@ notmuch_database_add_message (notmuch_database_t *notmuch,
 
 	_notmuch_message_sync (message);
     } catch (const Xapian::Error &error) {
-	fprintf (stderr, "A Xapian exception occurred adding message: %s.\n",
+	_notmuch_database_log (notmuch, "A Xapian exception occurred adding message: %s.\n",
 		 error.get_msg().c_str());
 	notmuch->exception_reported = TRUE;
 	ret = NOTMUCH_STATUS_XAPIAN_EXCEPTION;
@@ -2499,7 +2499,7 @@ notmuch_database_find_message_by_filename (notmuch_database_t *notmuch,
 		status = NOTMUCH_STATUS_OUT_OF_MEMORY;
 	}
     } catch (const Xapian::Error &error) {
-	fprintf (stderr, "Error: A Xapian exception occurred finding message by filename: %s\n",
+	_notmuch_database_log (notmuch, "Error: A Xapian exception occurred finding message by filename: %s\n",
 		 error.get_msg().c_str());
 	notmuch->exception_reported = TRUE;
 	status = NOTMUCH_STATUS_XAPIAN_EXCEPTION;
@@ -2552,7 +2552,7 @@ notmuch_database_get_all_tags (notmuch_database_t *db)
 	_notmuch_string_list_sort (tags);
 	return _notmuch_tags_create (db, tags);
     } catch (const Xapian::Error &error) {
-	fprintf (stderr, "A Xapian exception occurred getting tags: %s.\n",
+	_notmuch_database_log (db, "A Xapian exception occurred getting tags: %s.\n",
 		 error.get_msg().c_str());
 	db->exception_reported = TRUE;
 	return NULL;
diff --git a/lib/directory.cc b/lib/directory.cc
index 8daaec8..b836ea2 100644
--- a/lib/directory.cc
+++ b/lib/directory.cc
@@ -186,7 +186,7 @@ _notmuch_directory_create (notmuch_database_t *notmuch,
 	directory->mtime = Xapian::sortable_unserialise (
 	    directory->doc.get_value (NOTMUCH_VALUE_TIMESTAMP));
     } catch (const Xapian::Error &error) {
-	fprintf (stderr,
+	_notmuch_database_log (notmuch,
 		 "A Xapian exception occurred creating a directory: %s.\n",
 		 error.get_msg().c_str());
 	notmuch->exception_reported = TRUE;
@@ -228,7 +228,7 @@ notmuch_directory_set_mtime (notmuch_directory_t *directory,
 
 	db->replace_document (directory->document_id, directory->doc);
     } catch (const Xapian::Error &error) {
-	fprintf (stderr,
+	_notmuch_database_log (notmuch,
 		 "A Xapian exception occurred setting directory mtime: %s.\n",
 		 error.get_msg().c_str());
 	notmuch->exception_reported = TRUE;
diff --git a/lib/index.cc b/lib/index.cc
index 1a2e63d..9493549 100644
--- a/lib/index.cc
+++ b/lib/index.cc
@@ -314,7 +314,8 @@ _index_mime_part (notmuch_message_t *message,
     const char *charset;
 
     if (! part) {
-	fprintf (stderr, "Warning: Not indexing empty mime part.\n");
+	_notmuch_database_log (_notmuch_message_database (message),
+			      "Warning: Not indexing empty mime part.\n");
 	return;
     }
 
@@ -334,7 +335,8 @@ _index_mime_part (notmuch_message_t *message,
 		if (i == 1)
 		    continue;
 		if (i > 1)
-		    fprintf (stderr, "Warning: Unexpected extra parts of multipart/signed. Indexing anyway.\n");
+		    _notmuch_database_log (_notmuch_message_database (message),
+					  "Warning: Unexpected extra parts of multipart/signed. Indexing anyway.\n");
 	    }
 	    if (GMIME_IS_MULTIPART_ENCRYPTED (multipart)) {
 		/* Don't index encrypted parts. */
@@ -357,8 +359,9 @@ _index_mime_part (notmuch_message_t *message,
     }
 
     if (! (GMIME_IS_PART (part))) {
-	fprintf (stderr, "Warning: Not indexing unknown mime part: %s.\n",
-		 g_type_name (G_OBJECT_TYPE (part)));
+	_notmuch_database_log (_notmuch_message_database (message),
+			      "Warning: Not indexing unknown mime part: %s.\n",
+			      g_type_name (G_OBJECT_TYPE (part)));
 	return;
     }
 
diff --git a/lib/message.cc b/lib/message.cc
index 43cc078..541eabc 100644
--- a/lib/message.cc
+++ b/lib/message.cc
@@ -252,7 +252,7 @@ _notmuch_message_create_for_message_id (notmuch_database_t *notmuch,
 
 	doc_id = _notmuch_database_generate_doc_id (notmuch);
     } catch (const Xapian::Error &error) {
-	fprintf (stderr, "A Xapian exception occurred creating message: %s\n",
+	_notmuch_database_log(_notmuch_message_database (message), "A Xapian exception occurred creating message: %s\n",
 		 error.get_msg().c_str());
 	notmuch->exception_reported = TRUE;
 	*status_ret = NOTMUCH_PRIVATE_STATUS_XAPIAN_EXCEPTION;
@@ -467,7 +467,7 @@ notmuch_message_get_header (notmuch_message_t *message, const char *header)
 		return talloc_strdup (message, value.c_str ());
 
 	} catch (Xapian::Error &error) {
-	    fprintf (stderr, "A Xapian exception occurred when reading header: %s\n",
+	    _notmuch_database_log(_notmuch_message_database (message), "A Xapian exception occurred when reading header: %s\n",
 		     error.get_msg().c_str());
 	    message->notmuch->exception_reported = TRUE;
 	    return NULL;
@@ -920,7 +920,7 @@ notmuch_message_get_date (notmuch_message_t *message)
     try {
 	value = message->doc.get_value (NOTMUCH_VALUE_TIMESTAMP);
     } catch (Xapian::Error &error) {
-	fprintf (stderr, "A Xapian exception occurred when reading date: %s\n",
+	_notmuch_database_log(_notmuch_message_database (message), "A Xapian exception occurred when reading date: %s\n",
 		 error.get_msg().c_str());
 	message->notmuch->exception_reported = TRUE;
 	return 0;
diff --git a/lib/query.cc b/lib/query.cc
index 60ff8bd..9ed420c 100644
--- a/lib/query.cc
+++ b/lib/query.cc
@@ -282,9 +282,9 @@ notmuch_query_search_messages (notmuch_query_t *query)
 	return &messages->base;
 
     } catch (const Xapian::Error &error) {
-	fprintf (stderr, "A Xapian exception occurred performing query: %s\n",
+	_notmuch_database_log (notmuch, "A Xapian exception occurred performing query: %s\n",
 		 error.get_msg().c_str());
-	fprintf (stderr, "Query string was: %s\n", query->query_string);
+	_notmuch_database_log (notmuch, "Query string was: %s\n", query->query_string);
 	notmuch->exception_reported = TRUE;
 	talloc_free (messages);
 	return NULL;
@@ -562,9 +562,9 @@ notmuch_query_count_messages (notmuch_query_t *query)
 	count = mset.get_matches_estimated();
 
     } catch (const Xapian::Error &error) {
-	fprintf (stderr, "A Xapian exception occurred: %s\n",
+	_notmuch_database_log (notmuch, "A Xapian exception occurred: %s\n",
 		 error.get_msg().c_str());
-	fprintf (stderr, "Query string was: %s\n", query->query_string);
+	_notmuch_database_log (notmuch, "Query string was: %s\n", query->query_string);
     }
 
     return count;
-- 
2.1.3

[Patch v3 6/6] lib: eliminate fprintf from _notmuch_message_file_open

[David Bremner]

You may wonder why _notmuch_message_file_open_ctx has two parameters.
This is because we need sometime to use a ctx which is a
notmuch_message_t. While we could get the database from this, there is
no easy way in C to tell type we are getting. The remaining fprintf is
removed by Jani's series un-deprecating single message mboxes.
---
 lib/database.cc       |  2 +-
 lib/message-file.c    | 11 +++++++----
 lib/message.cc        |  3 ++-
 lib/notmuch-private.h |  5 +++--
 4 files changed, 13 insertions(+), 8 deletions(-)

diff --git a/lib/database.cc b/lib/database.cc
index 19f5f0e..b144885 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -2298,7 +2298,7 @@ notmuch_database_add_message (notmuch_database_t *notmuch,
     if (ret)
 	return ret;
 
-    message_file = _notmuch_message_file_open (filename);
+    message_file = _notmuch_message_file_open (notmuch, filename);
     if (message_file == NULL)
 	return NOTMUCH_STATUS_FILE_ERROR;
 
diff --git a/lib/message-file.c b/lib/message-file.c
index eda1b74..1f55cdf 100644
--- a/lib/message-file.c
+++ b/lib/message-file.c
@@ -76,7 +76,8 @@ _notmuch_message_file_destructor (notmuch_message_file_t *message)
 /* Create a new notmuch_message_file_t for 'filename' with 'ctx' as
  * the talloc owner. */
 notmuch_message_file_t *
-_notmuch_message_file_open_ctx (void *ctx, const char *filename)
+_notmuch_message_file_open_ctx (notmuch_database_t *notmuch,
+				void *ctx, const char *filename)
 {
     notmuch_message_file_t *message;
 
@@ -98,16 +99,18 @@ _notmuch_message_file_open_ctx (void *ctx, const char *filename)
     return message;
 
   FAIL:
-    fprintf (stderr, "Error opening %s: %s\n", filename, strerror (errno));
+    _notmuch_database_log (notmuch, "Error opening %s: %s\n",
+			  filename, strerror (errno));
     _notmuch_message_file_close (message);
 
     return NULL;
 }
 
 notmuch_message_file_t *
-_notmuch_message_file_open (const char *filename)
+_notmuch_message_file_open (notmuch_database_t *notmuch,
+			    const char *filename)
 {
-    return _notmuch_message_file_open_ctx (NULL, filename);
+    return _notmuch_message_file_open_ctx (notmuch, NULL, filename);
 }
 
 void
diff --git a/lib/message.cc b/lib/message.cc
index 541eabc..4251b44 100644
--- a/lib/message.cc
+++ b/lib/message.cc
@@ -437,7 +437,8 @@ _notmuch_message_ensure_message_file (notmuch_message_t *message)
     if (unlikely (filename == NULL))
 	return;
 
-    message->message_file = _notmuch_message_file_open_ctx (message, filename);
+    message->message_file = _notmuch_message_file_open_ctx (
+	_notmuch_message_database (message), message, filename);
 }
 
 const char *
diff --git a/lib/notmuch-private.h b/lib/notmuch-private.h
index fad3a92..76a2f33 100644
--- a/lib/notmuch-private.h
+++ b/lib/notmuch-private.h
@@ -359,11 +359,12 @@ typedef struct _notmuch_message_file notmuch_message_file_t;
  * Returns NULL if any error occurs.
  */
 notmuch_message_file_t *
-_notmuch_message_file_open (const char *filename);
+_notmuch_message_file_open (notmuch_database_t *notmuch, const char *filename);
 
 /* Like notmuch_message_file_open but with 'ctx' as the talloc owner. */
 notmuch_message_file_t *
-_notmuch_message_file_open_ctx (void *ctx, const char *filename);
+_notmuch_message_file_open_ctx (notmuch_database_t *notmuch,
+				void *ctx, const char *filename);
 
 /* Close a notmuch message previously opened with notmuch_message_open. */
 void
-- 
2.1.3