Update to library logging, version 5

David Bremner david at tethera.net
Tue Mar 24 06:24:03 PDT 2015


This takes into account (most of) Tomi's comments and adds a bunch
more tests.  We bikeshedded a bit about log_to_string on IRC, and
eventually convered on eliminating it. I guess it might be necessary
to add a compat version of asprintf for some environments (Solaris?)
but this is easy enough using talloc_asprintf and strdup

Here is the diff between the two versions

diff --git a/lib/database.cc b/lib/database.cc
index b5f3549..85054df 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -348,35 +348,6 @@ 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);
-}
-
 void
 _notmuch_database_log (notmuch_database_t *notmuch,
 		      const char *format,
@@ -386,7 +357,10 @@ _notmuch_database_log (notmuch_database_t *notmuch,
 
     va_start (va_args, format);
 
-    vlog_to_string (notmuch, &notmuch->status_string, format, va_args);
+    if (notmuch->status_string)
+	talloc_free (notmuch->status_string);
+
+    notmuch->status_string = talloc_vasprintf (notmuch, format, va_args);
 
     va_end (va_args);
 }
@@ -667,22 +641,23 @@ notmuch_database_create_verbose (const char *path,
     int err;
 
     if (path == NULL) {
-	log_to_string (&message, "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) {
-	log_to_string (&message, "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)) {
-	log_to_string (&message, "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;
     }
@@ -692,8 +667,8 @@ notmuch_database_create_verbose (const char *path,
     err = mkdir (notmuch_path, 0755);
 
     if (err) {
-	log_to_string (&message, "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;
     }
@@ -720,7 +695,7 @@ notmuch_database_create_verbose (const char *path,
 	talloc_free (notmuch_path);
 
     if (message)
-	*status_string = strdup(message);
+	*status_string = message;
     if (database)
 	*database = notmuch;
     else
@@ -849,27 +824,27 @@ notmuch_database_open_verbose (const char *path,
     static int initialized = 0;
 
     if (path == NULL) {
-	log_to_string (&message, "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"))) {
-	log_to_string (&message, "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) {
-	log_to_string (&message, "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"))) {
-	log_to_string (&message, "Out of memory\n");
+	message = strdup ("Out of memory\n");
 	status = NOTMUCH_STATUS_OUT_OF_MEMORY;
 	goto DONE;
     }
@@ -910,11 +885,11 @@ notmuch_database_open_verbose (const char *path,
 	 * means a dramatically incompatible change. */
 	version = notmuch_database_get_version (notmuch);
 	if (version > NOTMUCH_DATABASE_VERSION) {
-	    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",
-		     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;
@@ -929,11 +904,11 @@ notmuch_database_open_verbose (const char *path,
 	    version, mode == NOTMUCH_DATABASE_MODE_READ_WRITE ? 'w' : 'r',
 	    &incompat_features);
 	if (incompat_features) {
-	    log_to_string (&message,
-		     "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;
@@ -979,8 +954,8 @@ notmuch_database_open_verbose (const char *path,
 	    notmuch->query_parser->add_prefix (prefix->name, prefix->prefix);
 	}
     } catch (const Xapian::Error &error) {
-	log_to_string (&message, "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;
@@ -990,7 +965,7 @@ notmuch_database_open_verbose (const char *path,
     talloc_free (local);
 
     if (status_string && message)
-	*status_string = strdup (message);
+	*status_string = message;
 
     if (database)
 	*database = notmuch;
diff --git a/lib/query.cc b/lib/query.cc
index 7b59786..9cedb6a 100644
--- a/lib/query.cc
+++ b/lib/query.cc
@@ -296,9 +296,12 @@ notmuch_query_search_messages_st (notmuch_query_t *query,
 	return NOTMUCH_STATUS_SUCCESS;
 
     } catch (const Xapian::Error &error) {
-	_notmuch_database_log (notmuch, "A Xapian exception occurred performing query: %s\n",
-		 error.get_msg().c_str());
-	_notmuch_database_log (notmuch, "Query string was: %s\n", query->query_string);
+	_notmuch_database_log (notmuch,
+			       "A Xapian exception occurred performing query: %s\n"
+			       "Query string was: %s\n",
+			       error.get_msg().c_str(),
+			       query->query_string);
+
 	notmuch->exception_reported = TRUE;
 	talloc_free (messages);
 	return NOTMUCH_STATUS_XAPIAN_EXCEPTION;
@@ -597,9 +600,12 @@ notmuch_query_count_messages (notmuch_query_t *query)
 	count = mset.get_matches_estimated();
 
     } catch (const Xapian::Error &error) {
-	_notmuch_database_log (notmuch, "A Xapian exception occurred: %s\n",
-		 error.get_msg().c_str());
-	_notmuch_database_log (notmuch, "Query string was: %s\n", query->query_string);
+	_notmuch_database_log (notmuch,
+			       "A Xapian exception occurred performing query: %s\n"
+			       "Query string was: %s\n",
+			       error.get_msg().c_str(),
+			       query->query_string);
+
     }
 
     return count;
diff --git a/test/T560-lib-error.sh b/test/T560-lib-error.sh
index 7621e7f..ec7552a 100755
--- a/test/T560-lib-error.sh
+++ b/test/T560-lib-error.sh
@@ -3,6 +3,16 @@ test_description="error reporting for library"
 
 . ./test-lib.sh
 
+backup_database (){
+    rm -rf notmuch-dir-backup
+    cp -a ${MAIL_DIR}/.notmuch notmuch-dir-backup
+}
+restore_database (){
+    rm -rf ${MAIL_DIR}/.notmuch
+    cp -a notmuch-dir-backup ${MAIL_DIR}/.notmuch
+}
+
+
 add_email_corpus
 
 test_expect_success "building database" "NOTMUCH_NEW"
@@ -68,6 +78,31 @@ Cannot write to a read-only database.
 EOF
 test_expect_equal_file EXPECTED OUTPUT
 
+test_begin_subtest "Add non-existent file"
+test_C ${MAIL_DIR} <<EOF
+#include <stdio.h>
+#include <notmuch.h>
+int main (int argc, char** argv)
+{
+   notmuch_database_t *db;
+   notmuch_status_t stat;
+   stat = notmuch_database_open (argv[1], NOTMUCH_DATABASE_MODE_READ_WRITE, &db);
+   if (stat != NOTMUCH_STATUS_SUCCESS) {
+     fprintf (stderr, "error opening database: %d\n", stat);
+   }
+   stat = notmuch_database_add_message (db, "/nonexistent", NULL);
+   if (stat)
+       fputs (notmuch_database_status_string (db), stderr);
+
+}
+EOF
+cat <<EOF >EXPECTED
+== stdout ==
+== stderr ==
+Error opening /nonexistent: No such file or directory
+EOF
+test_expect_equal_file EXPECTED OUTPUT
+
 test_begin_subtest "compact, overwriting existing backup"
 test_C ${MAIL_DIR} <<EOF
 #include <stdio.h>
@@ -92,4 +127,128 @@ Path already exists: CWD/mail
 EOF
 test_expect_equal_file EXPECTED OUTPUT
 
+cat <<EOF > head.c
+#include <stdio.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <fcntl.h>
+#include <talloc.h>
+#include <notmuch.h>
+
+int main (int argc, char** argv)
+{
+   notmuch_database_t *db;
+   notmuch_status_t stat;
+   char *path;
+   int fd;
+
+   stat = notmuch_database_open (argv[1], NOTMUCH_DATABASE_MODE_READ_WRITE, &db);
+   if (stat != NOTMUCH_STATUS_SUCCESS) {
+     fprintf (stderr, "error opening database: %d\n", stat);
+   }
+   path = talloc_asprintf (db, "%s/.notmuch/xapian/postlist.DB", argv[1]);
+   fd = open(path,O_WRONLY|O_TRUNC);
+   if (fd < 0)
+       fprintf (stderr, "error opening %s\n");
+EOF
+cat <<EOF > tail.c
+   if (stat) {
+       const char *stat_str = notmuch_database_status_string (db);
+       if (stat_str)
+           fputs (stat_str, stderr);
+    }
+
+}
+EOF
+
+backup_database
+test_begin_subtest "Xapian exception finding message"
+cat head.c - tail.c <<EOF | test_C ${MAIL_DIR}
+   {
+       notmuch_message_t *message = NULL;
+       stat = notmuch_database_find_message (db, "id:nonexistant", &message);
+   }
+EOF
+sed 's/^\(A Xapian exception [^:]*\):.*$/\1/' < OUTPUT > OUTPUT.clean
+cat <<EOF >EXPECTED
+== stdout ==
+== stderr ==
+A Xapian exception occurred finding message
+EOF
+test_expect_equal_file EXPECTED OUTPUT.clean
+restore_database
+
+backup_database
+test_begin_subtest "Xapian exception getting tags"
+cat head.c - tail.c <<EOF | test_C ${MAIL_DIR}
+   {
+       notmuch_tags_t *tags = NULL;
+       tags = notmuch_database_get_all_tags (db);
+       stat = (tags == NULL);
+   }
+EOF
+sed 's/^\(A Xapian exception [^:]*\):.*$/\1/' < OUTPUT > OUTPUT.clean
+cat <<EOF >EXPECTED
+== stdout ==
+== stderr ==
+A Xapian exception occurred getting tags
+EOF
+test_expect_equal_file EXPECTED OUTPUT.clean
+restore_database
+
+backup_database
+test_begin_subtest "Xapian exception creating directory"
+cat head.c - tail.c <<EOF | test_C ${MAIL_DIR}
+   {
+       notmuch_directory_t *directory = NULL;
+       stat = notmuch_database_get_directory (db, "none/existing", &directory);
+   }
+EOF
+sed 's/^\(A Xapian exception [^:]*\):.*$/\1/' < OUTPUT > OUTPUT.clean
+cat <<EOF >EXPECTED
+== stdout ==
+== stderr ==
+A Xapian exception occurred creating a directory
+EOF
+test_expect_equal_file EXPECTED OUTPUT.clean
+restore_database
+
+backup_database
+test_begin_subtest "Xapian exception searching messages"
+cat head.c - tail.c <<EOF | test_C ${MAIL_DIR}
+   {
+       notmuch_messages_t *messages = NULL;
+       notmuch_query_t *query=notmuch_query_create (db, "*");
+       stat = notmuch_query_search_messages_st (query, &messages);
+   }
+EOF
+sed 's/^\(A Xapian exception [^:]*\):.*$/\1/' < OUTPUT > OUTPUT.clean
+cat <<EOF >EXPECTED
+== stdout ==
+== stderr ==
+A Xapian exception occurred performing query
+Query string was: *
+EOF
+test_expect_equal_file EXPECTED OUTPUT.clean
+restore_database
+
+backup_database
+test_begin_subtest "Xapian exception counting messages"
+cat head.c - tail.c <<EOF | test_C ${MAIL_DIR}
+   {
+       notmuch_query_t *query=notmuch_query_create (db, "id:87ocn0qh6d.fsf at yoom.home.cworth.org");
+       int count = notmuch_query_count_messages (query);
+       stat = (count == 0);
+   }
+EOF
+sed 's/^\(A Xapian exception [^:]*\):.*$/\1/' < OUTPUT > OUTPUT.clean
+cat <<EOF >EXPECTED
+== stdout ==
+== stderr ==
+A Xapian exception occurred performing query
+Query string was: id:87ocn0qh6d.fsf at yoom.home.cworth.org
+EOF
+test_expect_equal_file EXPECTED OUTPUT.clean
+restore_database
+
 test_done
diff --git a/test/test-lib.sh b/test/test-lib.sh
index c7af003..fdb84ea 100644
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -1163,16 +1163,16 @@ test_python() {
 		| $cmd -
 }
 
-test_C() {
-    test_file="test${test_count}.c"
-    exec_file=${test_file%%.c}
+test_C () {
+    exec_file="test${test_count}"
+    test_file="${exec_file}.c"
     cat > ${test_file}
     export LD_LIBRARY_PATH=${TEST_DIRECTORY}/../lib
-    ${TEST_CC} ${TEST_CFLAGS} -I${TEST_DIRECTORY}/../lib -o ${exec_file} ${test_file} -L${TEST_DIRECTORY}/../lib/ -lnotmuch
+    ${TEST_CC} ${TEST_CFLAGS} -I${TEST_DIRECTORY}/../lib -o ${exec_file} ${test_file} -L${TEST_DIRECTORY}/../lib/ -lnotmuch -ltalloc
     echo "== stdout ==" > OUTPUT.stdout
     echo "== stderr ==" > OUTPUT.stderr
     ./${exec_file} "$@" 1>>OUTPUT.stdout 2>>OUTPUT.stderr
-    cat OUTPUT.stdout OUTPUT.stderr | sed "s,$(pwd),CWD," > OUTPUT
+    sed "s,$(pwd),CWD,"  OUTPUT.stdout OUTPUT.stderr > OUTPUT
 }
 
 


More information about the notmuch mailing list