Library logging overhaul, round 6

* Library logging overhaul, round 6
@ 2015-03-27 22:11 David Bremner
  2015-03-27 22:11 ` [Patch v6 1/8] test: Add two tests for error output from notmuch_database_open David Bremner
                   ` (7 more replies)
  0 siblings, 8 replies; 11+ messages in thread
From: David Bremner @ 2015-03-27 22:11 UTC (permalink / raw)
  To: notmuch

This obsoletes

     id:1427203451-1540-1-git-send-email-david@tethera.net

I think this addresses all of Tomi's comments, except the use of
status_cb to print error output from notmuch_database_compact.
I added some tests for notmuch_database_create error output.

diff --git a/lib/database.cc b/lib/database.cc
index 85054df..9f66b5f 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -625,7 +625,18 @@ 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);
+    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
@@ -694,8 +705,9 @@ notmuch_database_create_verbose (const char *path,
     if (notmuch_path)
 	talloc_free (notmuch_path);
 
-    if (message)
+    if (status_string && message)
 	*status_string = message;
+
     if (database)
 	*database = notmuch;
     else
@@ -799,10 +811,13 @@ notmuch_database_open (const char *path,
     char *status_string = NULL;
     notmuch_status_t status;
 
-    status = notmuch_database_open_verbose(path, mode, database,
+    status = notmuch_database_open_verbose (path, mode, database,
 					   &status_string);
 
-    if (status_string) fputs(status_string, stderr);
+    if (status_string) {
+	fputs (status_string, stderr);
+	free (status_string);
+    }
 
     return status;
 }
diff --git a/notmuch-new.c b/notmuch-new.c
index 93b70bf..e6c283e 100644
--- a/notmuch-new.c
+++ b/notmuch-new.c
@@ -988,7 +988,10 @@ notmuch_new_command (notmuch_config_t *config, int argc, char *argv[])
 	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);
+	    if (status_string) {
+		fputs (status_string, stderr);
+		free (status_string);
+	    }
 
 	    return EXIT_FAILURE;
 	}
diff --git a/notmuch-search.c b/notmuch-search.c
index d012af3..b81ac01 100644
--- a/notmuch-search.c
+++ b/notmuch-search.c
@@ -574,7 +574,12 @@ _notmuch_search_prepare (search_context_t *ctx, notmuch_config_t *config, int ar
     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);
+
+	if (status_string) {
+	    fputs (status_string, stderr);
+	    free (status_string);
+	}
+
 	return EXIT_FAILURE;
     }
 
diff --git a/test/T560-lib-error.sh b/test/T560-lib-error.sh
index ec7552a..67a5e8d 100755
--- a/test/T560-lib-error.sh
+++ b/test/T560-lib-error.sh
@@ -3,13 +3,13 @@ test_description="error reporting for library"
 
 . ./test-lib.sh
 
-backup_database (){
+backup_database () {
     rm -rf notmuch-dir-backup
-    cp -a ${MAIL_DIR}/.notmuch notmuch-dir-backup
+    cp -pR ${MAIL_DIR}/.notmuch notmuch-dir-backup
 }
-restore_database (){
+restore_database () {
     rm -rf ${MAIL_DIR}/.notmuch
-    cp -a notmuch-dir-backup ${MAIL_DIR}/.notmuch
+    cp -pR notmuch-dir-backup ${MAIL_DIR}/.notmuch
 }
 
 
@@ -18,7 +18,7 @@ add_email_corpus
 test_expect_success "building database" "NOTMUCH_NEW"
 
 test_begin_subtest "Open null pointer"
-test_C <<EOF
+test_C <<'EOF'
 #include <stdio.h>
 #include <notmuch.h>
 int main (int argc, char** argv)
@@ -28,7 +28,7 @@ int main (int argc, char** argv)
     stat = notmuch_database_open (NULL, 0, 0);
 }
 EOF
-cat <<EOF >EXPECTED
+cat <<'EOF' >EXPECTED
 == stdout ==
 == stderr ==
 Error: Cannot open a database for a NULL path.
@@ -36,7 +36,7 @@ EOF
 test_expect_equal_file EXPECTED OUTPUT
 
 test_begin_subtest "Open nonexistent database"
-test_C <<EOF
+test_C <<'EOF'
 #include <stdio.h>
 #include <notmuch.h>
 int main (int argc, char** argv)
@@ -46,15 +46,50 @@ int main (int argc, char** argv)
     stat = notmuch_database_open ("/nonexistent/foo", 0, 0);
 }
 EOF
-cat <<EOF >EXPECTED
+cat <<'EOF' >EXPECTED
 == stdout ==
 == stderr ==
 Error opening database at /nonexistent/foo/.notmuch: No such file or directory
 EOF
 test_expect_equal_file EXPECTED OUTPUT
 
+test_begin_subtest "create NULL path"
+test_C <<'EOF'
+#include <stdio.h>
+#include <notmuch.h>
+int main (int argc, char** argv)
+{
+    notmuch_status_t stat;
+    stat = notmuch_database_create (NULL, NULL);
+}
+EOF
+cat <<'EOF' >EXPECTED
+== stdout ==
+== stderr ==
+Error: Cannot create a database for a NULL path.
+EOF
+test_expect_equal_file EXPECTED OUTPUT
+
+test_begin_subtest "Create database in non-existant directory"
+test_C <<'EOF'
+#include <stdio.h>
+#include <notmuch.h>
+int main (int argc, char** argv)
+{
+    notmuch_database_t *db;
+    notmuch_status_t stat;
+    stat = notmuch_database_create ("/nonexistent/foo", &db);
+}
+EOF
+cat <<'EOF' >EXPECTED
+== stdout ==
+== stderr ==
+Error: Cannot create database at /nonexistent/foo: No such file or directory.
+EOF
+test_expect_equal_file EXPECTED OUTPUT
+
 test_begin_subtest "Write to read-only database"
-test_C ${MAIL_DIR} <<EOF
+test_C ${MAIL_DIR} <<'EOF'
 #include <stdio.h>
 #include <notmuch.h>
 int main (int argc, char** argv)
@@ -71,7 +106,7 @@ int main (int argc, char** argv)
 
 }
 EOF
-cat <<EOF >EXPECTED
+cat <<'EOF' >EXPECTED
 == stdout ==
 == stderr ==
 Cannot write to a read-only database.
@@ -79,7 +114,7 @@ EOF
 test_expect_equal_file EXPECTED OUTPUT
 
 test_begin_subtest "Add non-existent file"
-test_C ${MAIL_DIR} <<EOF
+test_C ${MAIL_DIR} <<'EOF'
 #include <stdio.h>
 #include <notmuch.h>
 int main (int argc, char** argv)
@@ -96,7 +131,7 @@ int main (int argc, char** argv)
 
 }
 EOF
-cat <<EOF >EXPECTED
+cat <<'EOF' >EXPECTED
 == stdout ==
 == stderr ==
 Error opening /nonexistent: No such file or directory
@@ -104,7 +139,7 @@ EOF
 test_expect_equal_file EXPECTED OUTPUT
 
 test_begin_subtest "compact, overwriting existing backup"
-test_C ${MAIL_DIR} <<EOF
+test_C ${MAIL_DIR} <<'EOF'
 #include <stdio.h>
 #include <notmuch.h>
 static void
@@ -119,7 +154,7 @@ int main (int argc, char** argv)
    stat = notmuch_database_compact (argv[1], argv[1], status_cb, NULL);
 }
 EOF
-cat <<EOF >EXPECTED
+cat <<'EOF' >EXPECTED
 == stdout ==
 Path already exists: CWD/mail
 
@@ -127,7 +162,7 @@ Path already exists: CWD/mail
 EOF
 test_expect_equal_file EXPECTED OUTPUT
 
-cat <<EOF > head.c
+cat <<'EOF' > c_head
 #include <stdio.h>
 #include <sys/types.h>
 #include <sys/stat.h>
@@ -151,7 +186,7 @@ int main (int argc, char** argv)
    if (fd < 0)
        fprintf (stderr, "error opening %s\n");
 EOF
-cat <<EOF > tail.c
+cat <<'EOF' > c_tail
    if (stat) {
        const char *stat_str = notmuch_database_status_string (db);
        if (stat_str)
@@ -163,14 +198,14 @@ EOF
 
 backup_database
 test_begin_subtest "Xapian exception finding message"
-cat head.c - tail.c <<EOF | test_C ${MAIL_DIR}
+cat c_head - c_tail <<'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
+cat <<'EOF' >EXPECTED
 == stdout ==
 == stderr ==
 A Xapian exception occurred finding message
@@ -180,7 +215,7 @@ restore_database
 
 backup_database
 test_begin_subtest "Xapian exception getting tags"
-cat head.c - tail.c <<EOF | test_C ${MAIL_DIR}
+cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR}
    {
        notmuch_tags_t *tags = NULL;
        tags = notmuch_database_get_all_tags (db);
@@ -188,7 +223,7 @@ cat head.c - tail.c <<EOF | test_C ${MAIL_DIR}
    }
 EOF
 sed 's/^\(A Xapian exception [^:]*\):.*$/\1/' < OUTPUT > OUTPUT.clean
-cat <<EOF >EXPECTED
+cat <<'EOF' >EXPECTED
 == stdout ==
 == stderr ==
 A Xapian exception occurred getting tags
@@ -198,14 +233,14 @@ restore_database
 
 backup_database
 test_begin_subtest "Xapian exception creating directory"
-cat head.c - tail.c <<EOF | test_C ${MAIL_DIR}
+cat c_head - c_tail <<'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
+cat <<'EOF' >EXPECTED
 == stdout ==
 == stderr ==
 A Xapian exception occurred creating a directory
@@ -215,7 +250,7 @@ restore_database
 
 backup_database
 test_begin_subtest "Xapian exception searching messages"
-cat head.c - tail.c <<EOF | test_C ${MAIL_DIR}
+cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR}
    {
        notmuch_messages_t *messages = NULL;
        notmuch_query_t *query=notmuch_query_create (db, "*");
@@ -223,7 +258,7 @@ cat head.c - tail.c <<EOF | test_C ${MAIL_DIR}
    }
 EOF
 sed 's/^\(A Xapian exception [^:]*\):.*$/\1/' < OUTPUT > OUTPUT.clean
-cat <<EOF >EXPECTED
+cat <<'EOF' >EXPECTED
 == stdout ==
 == stderr ==
 A Xapian exception occurred performing query
@@ -234,7 +269,7 @@ restore_database
 
 backup_database
 test_begin_subtest "Xapian exception counting messages"
-cat head.c - tail.c <<EOF | test_C ${MAIL_DIR}
+cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR}
    {
        notmuch_query_t *query=notmuch_query_create (db, "id:87ocn0qh6d.fsf@yoom.home.cworth.org");
        int count = notmuch_query_count_messages (query);
@@ -242,7 +277,7 @@ cat head.c - tail.c <<EOF | test_C ${MAIL_DIR}
    }
 EOF
 sed 's/^\(A Xapian exception [^:]*\):.*$/\1/' < OUTPUT > OUTPUT.clean
-cat <<EOF >EXPECTED
+cat <<'EOF' >EXPECTED
 == stdout ==
 == stderr ==
 A Xapian exception occurred performing query
diff --git a/test/symbol-test.cc b/test/symbol-test.cc
index 9f8eea7..d979f83 100644
--- a/test/symbol-test.cc
+++ b/test/symbol-test.cc
@@ -1,4 +1,5 @@
 #include <stdio.h>
+#include <stdlib.h>
 #include <xapian.h>
 #include <notmuch.h>
 
@@ -8,8 +9,10 @@ int main() {
   char *message = NULL;
 
   if (notmuch_database_open_verbose  ("fakedb", NOTMUCH_DATABASE_MODE_READ_ONLY, &notmuch, &message))
-      if (message) fputs (message, stderr);
-
+      if (message) {
+	  fputs (message, stderr);
+	  free (message);
+      }
 
   try {
     (void) new Xapian::WritableDatabase("./nonexistant", Xapian::DB_OPEN);
diff --git a/test/test-lib.sh b/test/test-lib.sh
index fdb84ea..486d1c4 100644
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -1172,7 +1172,7 @@ test_C () {
     echo "== stdout ==" > OUTPUT.stdout
     echo "== stderr ==" > OUTPUT.stderr
     ./${exec_file} "$@" 1>>OUTPUT.stdout 2>>OUTPUT.stderr
-    sed "s,$(pwd),CWD,"  OUTPUT.stdout OUTPUT.stderr > OUTPUT
+    sed "s,${PWD},CWD,g"  OUTPUT.stdout OUTPUT.stderr > OUTPUT
 }
 
 

^ permalink raw reply related	[flat|nested] 11+ messages in thread