libnotmuch logging overhaul, round 2

libnotmuch logging overhaul, round 2

[David Bremner]

This obsoletes

     id:1419615761-21581-1-git-send-email-david@tethera.net

I added the promised "save a string in notmuch_database_t" logging
backend, and ended up reorganizing things a bit, since the special
casing required for _{create,open} a database seemed to naturally come
first.

One thing that I can't overemphasize here is that this series does not
(yet) include most of the changes needed to the CLI to recover these error messages. afaict that will involve adding lots (?) of fputs to error handling paths.

[PATCH v2 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     | 75 +++++++++++++++++++++++++++++++++++++++++++++--------
 lib/notmuch.h       | 21 +++++++++++++++
 notmuch-new.c       |  8 ++++--
 notmuch-search.c    |  8 ++++--
 test/symbol-test.cc |  6 ++++-
 5 files changed, 102 insertions(+), 16 deletions(-)

diff --git a/lib/database.cc b/lib/database.cc
index 3601f9d..06e6fc5 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,7 +672,7 @@ 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;
@@ -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
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 v2 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 06e6fc5..249eb68 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] == '/')
@@ -2538,3 +2539,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 v2 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       | 15 +++++++++++++++
 lib/notmuch-private.h |  4 ++++
 2 files changed, 19 insertions(+)

diff --git a/lib/database.cc b/lib/database.cc
index 249eb68..ee1c982 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -371,6 +371,21 @@ log_to_string (char **str,
     va_end (va_args);
 }
 
+void
+_notmuch_database_log (notmuch_database_t *notmuch,
+		      const char *format,
+		      ...)
+{
+    va_list va_args;
+    const char *message;
+
+    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 v2 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 v2 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
remaining 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     | 18 +++++++++---------
 5 files changed, 38 insertions(+), 35 deletions(-)

diff --git a/lib/database.cc b/lib/database.cc
index ee1c982..6906750 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -494,7 +494,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;
@@ -726,7 +726,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;
     }
 
@@ -1011,7 +1011,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());
 	    }
 	}
@@ -1138,12 +1138,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;
@@ -1163,20 +1163,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;
@@ -1184,7 +1184,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;
@@ -1213,7 +1213,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
@@ -1491,7 +1491,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;
@@ -1541,7 +1541,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;
@@ -1575,7 +1575,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;
@@ -1821,7 +1821,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;
@@ -2403,7 +2403,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;
@@ -2495,7 +2495,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;
@@ -2548,7 +2548,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..2e20ab2 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 ())
-	fprintf (stderr, "Query string is:\n%s\n", query_string);
+	_notmuch_database_log (notmuch, "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 ()) {
-	    fprintf (stderr, "Exclude query is:\n%s\n",
+	    _notmuch_database_log (notmuch, "Exclude query is:\n%s\n",
 		     exclude_query.get_description ().c_str ());
-	    fprintf (stderr, "Final query is:\n%s\n",
+	    _notmuch_database_log (notmuch, "Final query is:\n%s\n",
 		     final_query.get_description ().c_str ());
 	}
 
@@ -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;
@@ -549,9 +549,9 @@ notmuch_query_count_messages (notmuch_query_t *query)
 	enquire.set_docid_order(Xapian::Enquire::ASCENDING);
 
 	if (_debug_query ()) {
-	    fprintf (stderr, "Exclude query is:\n%s\n",
+	    _notmuch_database_log (notmuch, "Exclude query is:\n%s\n",
 		     exclude_query.get_description ().c_str ());
-	    fprintf (stderr, "Final query is:\n%s\n",
+	    _notmuch_database_log (notmuch, "Final query is:\n%s\n",
 		     final_query.get_description ().c_str ());
 	}
 
@@ -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 v2 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 6906750..16cd940 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -2294,7 +2294,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