unofficial mirror of notmuch@notmuchmail.org
 help / color / mirror / code / Atom feed
* Final (v2) batch of patches in API exception handling cleanup
@ 2020-08-01 12:25 David Bremner
  2020-08-01 12:25 ` [PATCH 01/14] lib: return NULL from n_d_get_default_indexopts on error David Bremner
                   ` (14 more replies)
  0 siblings, 15 replies; 16+ messages in thread
From: David Bremner @ 2020-08-01 12:25 UTC (permalink / raw)
  To: notmuch

I'm going to mark these ready since the changes from the previous
batch are trivial. Feel free to ping me if you want time to review.
I'm sending again mainly to collect into one series
for the historical record (patch 1/14 used to be separate)

The non-test changes are

 [PATCH 01/14] lib: return NULL from n_d_get_default_indexopts on
 [PATCH 06/14] lib: catch exceptions in n_directory_get_child_directories
 [PATCH 08/14] lib: catch exceptions in n_directory_get_child_files
 [PATCH 10/14] lib: fix return value for n_directory_delete

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

* [PATCH 01/14] lib: return NULL from n_d_get_default_indexopts on error
  2020-08-01 12:25 Final (v2) batch of patches in API exception handling cleanup David Bremner
@ 2020-08-01 12:25 ` David Bremner
  2020-08-01 12:25 ` [PATCH 02/14] test: add regression test for n_messages_collect_tags David Bremner
                   ` (13 subsequent siblings)
  14 siblings, 0 replies; 16+ messages in thread
From: David Bremner @ 2020-08-01 12:25 UTC (permalink / raw)
  To: notmuch; +Cc: David Bremner

This is a rare and probably serious programming error, so better not
to silently return a default value.
---
 lib/indexopts.c           | 2 +-
 lib/notmuch.h             | 1 +
 test/T562-lib-database.sh | 1 -
 3 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/indexopts.c b/lib/indexopts.c
index 1e8d180e..82a0026f 100644
--- a/lib/indexopts.c
+++ b/lib/indexopts.c
@@ -32,7 +32,7 @@ notmuch_database_get_default_indexopts (notmuch_database_t *db)
     char *decrypt_policy;
     notmuch_status_t err = notmuch_database_get_config (db, "index.decrypt", &decrypt_policy);
     if (err)
-	return ret;
+	return NULL;
 
     if (decrypt_policy) {
 	if ((! (strcasecmp (decrypt_policy, "true"))) ||
diff --git a/lib/notmuch.h b/lib/notmuch.h
index f9e9cc41..aaf92470 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -2320,6 +2320,7 @@ notmuch_config_list_destroy (notmuch_config_list_t *config_list);
  * added to the index.  At the moment it is a featureless stub.
  *
  * @since libnotmuch 5.1 (notmuch 0.26)
+ * @retval NULL in case of error
  */
 notmuch_indexopts_t *
 notmuch_database_get_default_indexopts (notmuch_database_t *db);
diff --git a/test/T562-lib-database.sh b/test/T562-lib-database.sh
index 32fda72e..ce62eaa8 100755
--- a/test/T562-lib-database.sh
+++ b/test/T562-lib-database.sh
@@ -373,7 +373,6 @@ EOF
 test_expect_equal_file EXPECTED OUTPUT
 
 test_begin_subtest "get indexopts from closed database"
-test_subtest_known_broken
 cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR}
     {
         notmuch_indexopts_t *result;
-- 
2.27.0

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

* [PATCH 02/14] test: add regression test for n_messages_collect_tags
  2020-08-01 12:25 Final (v2) batch of patches in API exception handling cleanup David Bremner
  2020-08-01 12:25 ` [PATCH 01/14] lib: return NULL from n_d_get_default_indexopts on error David Bremner
@ 2020-08-01 12:25 ` David Bremner
  2020-08-01 12:25 ` [PATCH 03/14] test: split header for lib-message tests David Bremner
                   ` (12 subsequent siblings)
  14 siblings, 0 replies; 16+ messages in thread
From: David Bremner @ 2020-08-01 12:25 UTC (permalink / raw)
  To: notmuch; +Cc: David Bremner

Also test n_messages_destroy.
---
 test/T568-lib-thread.sh | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/test/T568-lib-thread.sh b/test/T568-lib-thread.sh
index 66066854..ac13d986 100755
--- a/test/T568-lib-thread.sh
+++ b/test/T568-lib-thread.sh
@@ -285,6 +285,37 @@ unread
 EOF
 test_expect_equal_file EXPECTED OUTPUT
 
+test_begin_subtest "collect tags with closed database"
+cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR}
+    {
+      notmuch_messages_t *messages = notmuch_thread_get_messages (thread);
+
+      notmuch_tags_t *tags = notmuch_messages_collect_tags (messages);
+
+      const char *tag;
+      for (tags = notmuch_thread_get_tags (thread);
+           notmuch_tags_valid (tags);
+           notmuch_tags_move_to_next (tags))
+        {
+          tag = notmuch_tags_get (tags);
+          printf ("%s\n", tag);
+        }
+      notmuch_tags_destroy (tags);
+      notmuch_messages_destroy (messages);
+
+      printf("SUCCESS\n");
+    }
+EOF
+cat <<EOF > EXPECTED
+== stdout ==
+inbox
+signed
+unread
+SUCCESS
+== stderr ==
+EOF
+test_expect_equal_file EXPECTED OUTPUT
+
 test_begin_subtest "destroy thread with closed database"
 cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR}
     {
-- 
2.27.0

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

* [PATCH 03/14] test: split header for lib-message tests.
  2020-08-01 12:25 Final (v2) batch of patches in API exception handling cleanup David Bremner
  2020-08-01 12:25 ` [PATCH 01/14] lib: return NULL from n_d_get_default_indexopts on error David Bremner
  2020-08-01 12:25 ` [PATCH 02/14] test: add regression test for n_messages_collect_tags David Bremner
@ 2020-08-01 12:25 ` David Bremner
  2020-08-01 12:25 ` [PATCH 04/14] test: regression test for n_m_get_filenames David Bremner
                   ` (11 subsequent siblings)
  14 siblings, 0 replies; 16+ messages in thread
From: David Bremner @ 2020-08-01 12:25 UTC (permalink / raw)
  To: notmuch; +Cc: David Bremner

This allows finer control over when to close the database.
---
 test/T566-lib-message.sh | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/test/T566-lib-message.sh b/test/T566-lib-message.sh
index 4cf35810..3b0e85b5 100755
--- a/test/T566-lib-message.sh
+++ b/test/T566-lib-message.sh
@@ -18,7 +18,7 @@ cat <<'EOF' > c_tail
 }
 EOF
 
-cat <<EOF > c_head
+cat <<EOF > c_head0
 #include <stdio.h>
 #include <notmuch.h>
 #include <notmuch-test.h>
@@ -36,9 +36,11 @@ int main (int argc, char** argv)
      exit (1);
    }
    EXPECT0(notmuch_database_find_message (db, id, &message));
-   EXPECT0(notmuch_database_close (db));
 EOF
 
+cp c_head0 c_head
+echo "   EXPECT0(notmuch_database_close (db));" >> c_head
+
 test_begin_subtest "Handle getting message-id from closed database"
 cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR}
     {
-- 
2.27.0

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

* [PATCH 04/14] test: regression test for n_m_get_filenames
  2020-08-01 12:25 Final (v2) batch of patches in API exception handling cleanup David Bremner
                   ` (2 preceding siblings ...)
  2020-08-01 12:25 ` [PATCH 03/14] test: split header for lib-message tests David Bremner
@ 2020-08-01 12:25 ` David Bremner
  2020-08-01 12:25 ` [PATCH 05/14] test: add known broken test for n_directory_get_child_directories David Bremner
                   ` (10 subsequent siblings)
  14 siblings, 0 replies; 16+ messages in thread
From: David Bremner @ 2020-08-01 12:25 UTC (permalink / raw)
  To: notmuch; +Cc: David Bremner

Closing the database after the iterator is created is not a problem.
---
 test/T566-lib-message.sh | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/test/T566-lib-message.sh b/test/T566-lib-message.sh
index 3b0e85b5..0ba601f9 100755
--- a/test/T566-lib-message.sh
+++ b/test/T566-lib-message.sh
@@ -138,6 +138,29 @@ cat <<EOF > EXPECTED
 EOF
 test_expect_equal_file EXPECTED OUTPUT
 
+test_begin_subtest "iterate over all message filenames from closed database"
+cat c_head0 - c_tail <<'EOF' | test_C ${MAIL_DIR}
+    {
+        notmuch_filenames_t *filenames;
+        filenames = notmuch_message_get_filenames (message);
+        EXPECT0(notmuch_database_close (db));
+        for (; notmuch_filenames_valid (filenames);
+               notmuch_filenames_move_to_next (filenames)) {
+            const char *filename = notmuch_filenames_get (filenames);
+            printf("%s\n", filename);
+        }
+        notmuch_filenames_destroy (filenames);
+        printf("SUCCESS\n");
+    }
+EOF
+cat <<EOF > EXPECTED
+== stdout ==
+MAIL_DIR/01:2,
+SUCCESS
+== stderr ==
+EOF
+test_expect_equal_file EXPECTED OUTPUT
+
 test_begin_subtest "Handle getting ghost flag from closed database"
 cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR}
     {
-- 
2.27.0

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

* [PATCH 05/14] test: add known broken test for n_directory_get_child_directories
  2020-08-01 12:25 Final (v2) batch of patches in API exception handling cleanup David Bremner
                   ` (3 preceding siblings ...)
  2020-08-01 12:25 ` [PATCH 04/14] test: regression test for n_m_get_filenames David Bremner
@ 2020-08-01 12:25 ` David Bremner
  2020-08-01 12:25 ` [PATCH 06/14] lib: catch exceptions in n_directory_get_child_directories David Bremner
                   ` (9 subsequent siblings)
  14 siblings, 0 replies; 16+ messages in thread
From: David Bremner @ 2020-08-01 12:25 UTC (permalink / raw)
  To: notmuch; +Cc: David Bremner

Start a new test file (for the notmuch_directory_* API group) to hold
this test.
---
 test/T563-lib-directory.sh | 61 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 61 insertions(+)
 create mode 100755 test/T563-lib-directory.sh

diff --git a/test/T563-lib-directory.sh b/test/T563-lib-directory.sh
new file mode 100755
index 00000000..15fb8835
--- /dev/null
+++ b/test/T563-lib-directory.sh
@@ -0,0 +1,61 @@
+#!/usr/bin/env bash
+test_description="notmuch_directory_* API"
+
+. $(dirname "$0")/test-lib.sh || exit 1
+
+add_email_corpus
+
+test_begin_subtest "building database"
+test_expect_success "NOTMUCH_NEW"
+
+cat <<EOF > c_head
+#include <stdio.h>
+#include <notmuch.h>
+#include <notmuch-test.h>
+#include <talloc.h>
+int main (int argc, char** argv)
+{
+   notmuch_database_t *db;
+   notmuch_directory_t *dir;
+   notmuch_status_t stat = NOTMUCH_STATUS_SUCCESS;
+   char *msg = NULL;
+
+   stat = notmuch_database_open_verbose (argv[1], NOTMUCH_DATABASE_MODE_READ_WRITE, &db, &msg);
+   if (stat != NOTMUCH_STATUS_SUCCESS) {
+     fprintf (stderr, "error opening database: %d %s\n", stat, msg ? msg : "");
+     exit (1);
+   }
+
+   EXPECT0(notmuch_database_get_directory (db, "bar", &dir));
+   EXPECT0(notmuch_database_close (db));
+EOF
+
+cat <<'EOF' > c_tail
+   if (stat) {
+       const char *stat_str = notmuch_database_status_string (db);
+       if (stat_str)
+           fputs (stat_str, stderr);
+    }
+
+}
+EOF
+
+test_begin_subtest "get child directories for a closed db"
+test_subtest_known_broken
+cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR}
+    {
+        notmuch_filenames_t *children;
+        children = notmuch_directory_get_child_directories (dir);
+        printf ("%d\n", children == NULL);
+        stat = NOTMUCH_STATUS_XAPIAN_EXCEPTION;
+    }
+EOF
+cat <<EOF > EXPECTED
+== stdout ==
+1
+== stderr ==
+A Xapian exception occurred at lib/directory.cc:XXX: Database has been closed
+EOF
+test_expect_equal_file EXPECTED OUTPUT
+
+test_done
-- 
2.27.0

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

* [PATCH 06/14] lib: catch exceptions in n_directory_get_child_directories
  2020-08-01 12:25 Final (v2) batch of patches in API exception handling cleanup David Bremner
                   ` (4 preceding siblings ...)
  2020-08-01 12:25 ` [PATCH 05/14] test: add known broken test for n_directory_get_child_directories David Bremner
@ 2020-08-01 12:25 ` David Bremner
  2020-08-01 12:25 ` [PATCH 07/14] test: add known broken test for n_directory_get_child_files David Bremner
                   ` (8 subsequent siblings)
  14 siblings, 0 replies; 16+ messages in thread
From: David Bremner @ 2020-08-01 12:25 UTC (permalink / raw)
  To: notmuch; +Cc: David Bremner

Also clarify API in error case.
---
 lib/directory.cc           | 23 ++++++++++++++++++++---
 lib/notmuch.h              |  2 ++
 test/T563-lib-directory.sh |  1 -
 3 files changed, 22 insertions(+), 4 deletions(-)

diff --git a/lib/directory.cc b/lib/directory.cc
index b9c3d77f..5d13afec 100644
--- a/lib/directory.cc
+++ b/lib/directory.cc
@@ -49,6 +49,19 @@ struct _notmuch_directory {
     time_t mtime;
 };
 
+#define LOG_XAPIAN_EXCEPTION(directory, error) _log_xapian_exception (__location__, directory, error)
+
+static void
+_log_xapian_exception (const char *where, notmuch_directory_t *dir,  const Xapian::Error error) {
+    notmuch_database_t *notmuch = dir->notmuch;
+    _notmuch_database_log (notmuch,
+			   "A Xapian exception occurred at %s: %s\n",
+			   where,
+			   error.get_msg ().c_str ());
+    notmuch->exception_reported = true;
+}
+
+
 /* We end up having to call the destructor explicitly because we had
  * to use "placement new" in order to initialize C++ objects within a
  * block that we allocated with talloc. So C++ is making talloc
@@ -267,14 +280,18 @@ notmuch_filenames_t *
 notmuch_directory_get_child_directories (notmuch_directory_t *directory)
 {
     char *term;
-    notmuch_filenames_t *child_directories;
+    notmuch_filenames_t *child_directories = NULL;
 
     term = talloc_asprintf (directory, "%s%u:",
 			    _find_prefix ("directory-direntry"),
 			    directory->document_id);
 
-    child_directories = _create_filenames_for_terms_with_prefix (directory,
-								 directory->notmuch, term);
+    try {
+	child_directories = _create_filenames_for_terms_with_prefix (directory,
+								     directory->notmuch, term);
+    } catch (Xapian::Error &error) {
+	LOG_XAPIAN_EXCEPTION (directory, error);
+    }
 
     talloc_free (term);
 
diff --git a/lib/notmuch.h b/lib/notmuch.h
index aaf92470..e59fc571 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -2158,6 +2158,8 @@ notmuch_directory_get_child_files (notmuch_directory_t *directory);
  *
  * The returned filenames will be the basename-entries only (not
  * complete paths).
+ *
+ * Returns NULL if it triggers a Xapian exception
  */
 notmuch_filenames_t *
 notmuch_directory_get_child_directories (notmuch_directory_t *directory);
diff --git a/test/T563-lib-directory.sh b/test/T563-lib-directory.sh
index 15fb8835..5e7da676 100755
--- a/test/T563-lib-directory.sh
+++ b/test/T563-lib-directory.sh
@@ -41,7 +41,6 @@ cat <<'EOF' > c_tail
 EOF
 
 test_begin_subtest "get child directories for a closed db"
-test_subtest_known_broken
 cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR}
     {
         notmuch_filenames_t *children;
-- 
2.27.0

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

* [PATCH 07/14] test: add known broken test for n_directory_get_child_files
  2020-08-01 12:25 Final (v2) batch of patches in API exception handling cleanup David Bremner
                   ` (5 preceding siblings ...)
  2020-08-01 12:25 ` [PATCH 06/14] lib: catch exceptions in n_directory_get_child_directories David Bremner
@ 2020-08-01 12:25 ` David Bremner
  2020-08-01 12:25 ` [PATCH 08/14] lib: catch exceptions in n_directory_get_child_files David Bremner
                   ` (7 subsequent siblings)
  14 siblings, 0 replies; 16+ messages in thread
From: David Bremner @ 2020-08-01 12:25 UTC (permalink / raw)
  To: notmuch; +Cc: David Bremner

This is a clone of the one for get_child_directories
---
 test/T563-lib-directory.sh | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/test/T563-lib-directory.sh b/test/T563-lib-directory.sh
index 5e7da676..d71c884d 100755
--- a/test/T563-lib-directory.sh
+++ b/test/T563-lib-directory.sh
@@ -57,4 +57,22 @@ A Xapian exception occurred at lib/directory.cc:XXX: Database has been closed
 EOF
 test_expect_equal_file EXPECTED OUTPUT
 
+test_begin_subtest "get child filenames for a closed db"
+test_subtest_known_broken
+cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR}
+    {
+        notmuch_filenames_t *children;
+        children = notmuch_directory_get_child_files (dir);
+        printf ("%d\n", children == NULL);
+        stat = NOTMUCH_STATUS_XAPIAN_EXCEPTION;
+    }
+EOF
+cat <<EOF > EXPECTED
+== stdout ==
+1
+== stderr ==
+A Xapian exception occurred at lib/directory.cc:XXX: Database has been closed
+EOF
+test_expect_equal_file EXPECTED OUTPUT
+
 test_done
-- 
2.27.0

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

* [PATCH 08/14] lib: catch exceptions in n_directory_get_child_files
  2020-08-01 12:25 Final (v2) batch of patches in API exception handling cleanup David Bremner
                   ` (6 preceding siblings ...)
  2020-08-01 12:25 ` [PATCH 07/14] test: add known broken test for n_directory_get_child_files David Bremner
@ 2020-08-01 12:25 ` David Bremner
  2020-08-01 12:25 ` [PATCH 09/14] test: known broken test for n_directory_delete with closed db David Bremner
                   ` (6 subsequent siblings)
  14 siblings, 0 replies; 16+ messages in thread
From: David Bremner @ 2020-08-01 12:25 UTC (permalink / raw)
  To: notmuch; +Cc: David Bremner

Also clarify API in error case.
---
 lib/directory.cc           | 12 ++++++++----
 lib/notmuch.h              |  2 ++
 test/T563-lib-directory.sh |  1 -
 3 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/lib/directory.cc b/lib/directory.cc
index 5d13afec..79ceea31 100644
--- a/lib/directory.cc
+++ b/lib/directory.cc
@@ -261,15 +261,19 @@ notmuch_filenames_t *
 notmuch_directory_get_child_files (notmuch_directory_t *directory)
 {
     char *term;
-    notmuch_filenames_t *child_files;
+    notmuch_filenames_t *child_files = NULL;
 
     term = talloc_asprintf (directory, "%s%u:",
 			    _find_prefix ("file-direntry"),
 			    directory->document_id);
 
-    child_files = _create_filenames_for_terms_with_prefix (directory,
-							   directory->notmuch,
-							   term);
+    try {
+	child_files = _create_filenames_for_terms_with_prefix (directory,
+							       directory->notmuch,
+							       term);
+    } catch (Xapian::Error &error) {
+	LOG_XAPIAN_EXCEPTION (directory, error);
+    }
 
     talloc_free (term);
 
diff --git a/lib/notmuch.h b/lib/notmuch.h
index e59fc571..9a2a3b1f 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -2148,6 +2148,8 @@ notmuch_directory_get_mtime (notmuch_directory_t *directory);
  *
  * The returned filenames will be the basename-entries only (not
  * complete paths).
+ *
+ * Returns NULL if it triggers a Xapian exception
  */
 notmuch_filenames_t *
 notmuch_directory_get_child_files (notmuch_directory_t *directory);
diff --git a/test/T563-lib-directory.sh b/test/T563-lib-directory.sh
index d71c884d..61ed8add 100755
--- a/test/T563-lib-directory.sh
+++ b/test/T563-lib-directory.sh
@@ -58,7 +58,6 @@ EOF
 test_expect_equal_file EXPECTED OUTPUT
 
 test_begin_subtest "get child filenames for a closed db"
-test_subtest_known_broken
 cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR}
     {
         notmuch_filenames_t *children;
-- 
2.27.0

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

* [PATCH 09/14] test: known broken test for n_directory_delete with closed db.
  2020-08-01 12:25 Final (v2) batch of patches in API exception handling cleanup David Bremner
                   ` (7 preceding siblings ...)
  2020-08-01 12:25 ` [PATCH 08/14] lib: catch exceptions in n_directory_get_child_files David Bremner
@ 2020-08-01 12:25 ` David Bremner
  2020-08-01 12:25 ` [PATCH 10/14] lib: fix return value for n_directory_delete David Bremner
                   ` (5 subsequent siblings)
  14 siblings, 0 replies; 16+ messages in thread
From: David Bremner @ 2020-08-01 12:25 UTC (permalink / raw)
  To: notmuch; +Cc: David Bremner

There is a return value bug in notmuch_directory_delete that is hiding
the exception.
---
 test/T563-lib-directory.sh | 36 ++++++++++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)

diff --git a/test/T563-lib-directory.sh b/test/T563-lib-directory.sh
index 61ed8add..b91a1c87 100755
--- a/test/T563-lib-directory.sh
+++ b/test/T563-lib-directory.sh
@@ -74,4 +74,40 @@ A Xapian exception occurred at lib/directory.cc:XXX: Database has been closed
 EOF
 test_expect_equal_file EXPECTED OUTPUT
 
+backup_database
+test_begin_subtest "delete directory document for a closed db"
+test_subtest_known_broken
+cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR}
+    {
+        stat = notmuch_directory_delete (dir);
+        printf ("%d\n", stat == NOTMUCH_STATUS_XAPIAN_EXCEPTION);
+    }
+EOF
+cat <<EOF > EXPECTED
+== stdout ==
+1
+== stderr ==
+A Xapian exception occurred deleting directory entry: Database has been closed.
+EOF
+test_expect_equal_file EXPECTED OUTPUT
+restore_database
+
+backup_database
+test_begin_subtest "get/set mtime of directory for a closed db"
+cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR}
+    {
+        time_t stamp = notmuch_directory_get_mtime (dir);
+        stat = notmuch_directory_set_mtime (dir, stamp);
+        printf ("%d\n", stat == NOTMUCH_STATUS_XAPIAN_EXCEPTION);
+    }
+EOF
+cat <<EOF > EXPECTED
+== stdout ==
+1
+== stderr ==
+A Xapian exception occurred setting directory mtime: Database has been closed.
+EOF
+test_expect_equal_file EXPECTED OUTPUT
+restore_database
+
 test_done
-- 
2.27.0

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

* [PATCH 10/14] lib: fix return value for n_directory_delete
  2020-08-01 12:25 Final (v2) batch of patches in API exception handling cleanup David Bremner
                   ` (8 preceding siblings ...)
  2020-08-01 12:25 ` [PATCH 09/14] test: known broken test for n_directory_delete with closed db David Bremner
@ 2020-08-01 12:25 ` David Bremner
  2020-08-01 12:26 ` [PATCH 11/14] test: regression test for n_directory_{get,set}_mtime David Bremner
                   ` (4 subsequent siblings)
  14 siblings, 0 replies; 16+ messages in thread
From: David Bremner @ 2020-08-01 12:25 UTC (permalink / raw)
  To: notmuch; +Cc: David Bremner

Falling out of the catch meant the error return was lost
---
 lib/directory.cc           | 2 +-
 test/T563-lib-directory.sh | 1 -
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/lib/directory.cc b/lib/directory.cc
index 79ceea31..eee8254e 100644
--- a/lib/directory.cc
+++ b/lib/directory.cc
@@ -323,7 +323,7 @@ notmuch_directory_delete (notmuch_directory_t *directory)
     }
     notmuch_directory_destroy (directory);
 
-    return NOTMUCH_STATUS_SUCCESS;
+    return status;
 }
 
 void
diff --git a/test/T563-lib-directory.sh b/test/T563-lib-directory.sh
index b91a1c87..7e44e805 100755
--- a/test/T563-lib-directory.sh
+++ b/test/T563-lib-directory.sh
@@ -76,7 +76,6 @@ test_expect_equal_file EXPECTED OUTPUT
 
 backup_database
 test_begin_subtest "delete directory document for a closed db"
-test_subtest_known_broken
 cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR}
     {
         stat = notmuch_directory_delete (dir);
-- 
2.27.0

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

* [PATCH 11/14] test: regression test for n_directory_{get,set}_mtime
  2020-08-01 12:25 Final (v2) batch of patches in API exception handling cleanup David Bremner
                   ` (9 preceding siblings ...)
  2020-08-01 12:25 ` [PATCH 10/14] lib: fix return value for n_directory_delete David Bremner
@ 2020-08-01 12:26 ` David Bremner
  2020-08-01 12:26 ` [PATCH 12/14] test: regression test for n_d_get_config_list on closed db David Bremner
                   ` (3 subsequent siblings)
  14 siblings, 0 replies; 16+ messages in thread
From: David Bremner @ 2020-08-01 12:26 UTC (permalink / raw)
  To: notmuch; +Cc: David Bremner

The mtime is cached, so closing the db is not a problem. Writing the
mtime throws an exception, which is caught.
---
 test/T563-lib-directory.sh | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/test/T563-lib-directory.sh b/test/T563-lib-directory.sh
index 7e44e805..28325ff2 100755
--- a/test/T563-lib-directory.sh
+++ b/test/T563-lib-directory.sh
@@ -109,4 +109,22 @@ EOF
 test_expect_equal_file EXPECTED OUTPUT
 restore_database
 
+backup_database
+test_begin_subtest "get/set mtime of directory for a closed db"
+cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR}
+    {
+        time_t stamp = notmuch_directory_get_mtime (dir);
+        stat = notmuch_directory_set_mtime (dir, stamp);
+        printf ("%d\n", stat == NOTMUCH_STATUS_XAPIAN_EXCEPTION);
+    }
+EOF
+cat <<EOF > EXPECTED
+== stdout ==
+1
+== stderr ==
+A Xapian exception occurred setting directory mtime: Database has been closed.
+EOF
+test_expect_equal_file EXPECTED OUTPUT
+restore_database
+
 test_done
-- 
2.27.0

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

* [PATCH 12/14] test: regression test for n_d_get_config_list on closed db.
  2020-08-01 12:25 Final (v2) batch of patches in API exception handling cleanup David Bremner
                   ` (10 preceding siblings ...)
  2020-08-01 12:26 ` [PATCH 11/14] test: regression test for n_directory_{get,set}_mtime David Bremner
@ 2020-08-01 12:26 ` David Bremner
  2020-08-01 12:26 ` [PATCH 13/14] test: regression test for traversing config list with " David Bremner
                   ` (2 subsequent siblings)
  14 siblings, 0 replies; 16+ messages in thread
From: David Bremner @ 2020-08-01 12:26 UTC (permalink / raw)
  To: notmuch; +Cc: David Bremner

Exception is caught.
---
 test/T590-libconfig.sh | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/test/T590-libconfig.sh b/test/T590-libconfig.sh
index 46f3a76d..602fad0b 100755
--- a/test/T590-libconfig.sh
+++ b/test/T590-libconfig.sh
@@ -61,6 +61,21 @@ valid = 0
 EOF
 test_expect_equal_file EXPECTED OUTPUT
 
+test_begin_subtest "notmuch_database_get_config_list: closed db"
+cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR}
+{
+   notmuch_config_list_t *list;
+   EXPECT0(notmuch_database_close (db));
+   stat = notmuch_database_get_config_list (db, "nonexistent", &list);
+   printf("%d\n", stat == NOTMUCH_STATUS_XAPIAN_EXCEPTION);
+}
+EOF
+cat <<'EOF' >EXPECTED
+== stdout ==
+1
+== stderr ==
+EOF
+test_expect_equal_file EXPECTED OUTPUT
 
 test_begin_subtest "notmuch_database_get_config_list: all pairs"
 cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR}
-- 
2.27.0

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

* [PATCH 13/14] test: regression test for traversing config list with closed db
  2020-08-01 12:25 Final (v2) batch of patches in API exception handling cleanup David Bremner
                   ` (11 preceding siblings ...)
  2020-08-01 12:26 ` [PATCH 12/14] test: regression test for n_d_get_config_list on closed db David Bremner
@ 2020-08-01 12:26 ` David Bremner
  2020-08-01 12:26 ` [PATCH 14/14] test: regression tests for n_indexopts_{get,set}_decrypt_policy David Bremner
  2020-08-04  1:38 ` Final (v2) batch of patches in API exception handling cleanup David Bremner
  14 siblings, 0 replies; 16+ messages in thread
From: David Bremner @ 2020-08-01 12:26 UTC (permalink / raw)
  To: notmuch; +Cc: David Bremner

Also mention error return in API docs
---
 lib/notmuch.h          |  1 +
 test/T590-libconfig.sh | 22 ++++++++++++++++++++++
 2 files changed, 23 insertions(+)

diff --git a/lib/notmuch.h b/lib/notmuch.h
index 9a2a3b1f..f3cb0fe2 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -2291,6 +2291,7 @@ notmuch_config_list_key (notmuch_config_list_t *config_list);
  * next call to notmuch_config_list_value or notmuch config_list_destroy
  *
  * @since libnotmuch 4.4 (notmuch 0.23)
+ * @retval NULL for errors
  */
 const char *
 notmuch_config_list_value (notmuch_config_list_t *config_list);
diff --git a/test/T590-libconfig.sh b/test/T590-libconfig.sh
index 602fad0b..360e45b0 100755
--- a/test/T590-libconfig.sh
+++ b/test/T590-libconfig.sh
@@ -100,6 +100,28 @@ zzzafter afterval
 EOF
 test_expect_equal_file EXPECTED OUTPUT
 
+test_begin_subtest "notmuch_database_get_config_list: all pairs (closed db)"
+cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR}
+{
+   notmuch_config_list_t *list;
+   EXPECT0(notmuch_database_get_config_list (db, "", &list));
+   EXPECT0(notmuch_database_close (db));
+   for (; notmuch_config_list_valid (list); notmuch_config_list_move_to_next (list)) {
+      printf("%s %d\n", notmuch_config_list_key (list), NULL == notmuch_config_list_value(list));
+   }
+   notmuch_config_list_destroy (list);
+}
+EOF
+cat <<'EOF' >EXPECTED
+== stdout ==
+aaabefore 1
+testkey1 1
+testkey2 1
+zzzafter 1
+== stderr ==
+EOF
+test_expect_equal_file EXPECTED OUTPUT
+
 test_begin_subtest "notmuch_database_get_config_list: one prefix"
 cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR}
 {
-- 
2.27.0

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

* [PATCH 14/14] test: regression tests for n_indexopts_{get,set}_decrypt_policy
  2020-08-01 12:25 Final (v2) batch of patches in API exception handling cleanup David Bremner
                   ` (12 preceding siblings ...)
  2020-08-01 12:26 ` [PATCH 13/14] test: regression test for traversing config list with " David Bremner
@ 2020-08-01 12:26 ` David Bremner
  2020-08-04  1:38 ` Final (v2) batch of patches in API exception handling cleanup David Bremner
  14 siblings, 0 replies; 16+ messages in thread
From: David Bremner @ 2020-08-01 12:26 UTC (permalink / raw)
  To: notmuch; +Cc: David Bremner

The main criteria is that they don't crash. Working with a closed
database is a bonus.
---
 test/T562-lib-database.sh | 39 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)

diff --git a/test/T562-lib-database.sh b/test/T562-lib-database.sh
index ce62eaa8..d6097418 100755
--- a/test/T562-lib-database.sh
+++ b/test/T562-lib-database.sh
@@ -388,4 +388,43 @@ cat <<EOF > EXPECTED
 EOF
 test_expect_equal_file EXPECTED OUTPUT
 
+test_begin_subtest "get decryption policy from closed database"
+cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR}
+    {
+        notmuch_indexopts_t *result;
+        result = notmuch_database_get_default_indexopts (db);
+        EXPECT0(notmuch_database_close (db));
+        notmuch_decryption_policy_t policy = notmuch_indexopts_get_decrypt_policy (result);
+        printf ("%d\n",  policy == NOTMUCH_DECRYPT_AUTO);
+        notmuch_indexopts_destroy (result);
+        printf ("SUCCESS\n");
+    }
+EOF
+cat <<EOF > EXPECTED
+== stdout ==
+1
+SUCCESS
+== stderr ==
+EOF
+test_expect_equal_file EXPECTED OUTPUT
+
+test_begin_subtest "set decryption policy with closed database"
+cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR}
+    {
+        notmuch_indexopts_t *result;
+        result = notmuch_database_get_default_indexopts (db);
+        EXPECT0(notmuch_database_close (db));
+        notmuch_decryption_policy_t policy = notmuch_indexopts_get_decrypt_policy (result);
+        stat = notmuch_indexopts_set_decrypt_policy (result, policy);
+        printf("%d\n%d\n",  policy == NOTMUCH_DECRYPT_AUTO, stat == NOTMUCH_STATUS_SUCCESS);
+    }
+EOF
+cat <<EOF > EXPECTED
+== stdout ==
+1
+1
+== stderr ==
+EOF
+test_expect_equal_file EXPECTED OUTPUT
+
 test_done
-- 
2.27.0

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

* Re: Final (v2) batch of patches in API exception handling cleanup
  2020-08-01 12:25 Final (v2) batch of patches in API exception handling cleanup David Bremner
                   ` (13 preceding siblings ...)
  2020-08-01 12:26 ` [PATCH 14/14] test: regression tests for n_indexopts_{get,set}_decrypt_policy David Bremner
@ 2020-08-04  1:38 ` David Bremner
  14 siblings, 0 replies; 16+ messages in thread
From: David Bremner @ 2020-08-04  1:38 UTC (permalink / raw)
  To: notmuch

David Bremner <david@tethera.net> writes:

>
> The non-test changes are
>
>  [PATCH 01/14] lib: return NULL from n_d_get_default_indexopts on
>  [PATCH 06/14] lib: catch exceptions in n_directory_get_child_directories
>  [PATCH 08/14] lib: catch exceptions in n_directory_get_child_files
>  [PATCH 10/14] lib: fix return value for n_directory_delete
>

Series applied to master.

d

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

end of thread, other threads:[~2020-08-04  1:39 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-01 12:25 Final (v2) batch of patches in API exception handling cleanup David Bremner
2020-08-01 12:25 ` [PATCH 01/14] lib: return NULL from n_d_get_default_indexopts on error David Bremner
2020-08-01 12:25 ` [PATCH 02/14] test: add regression test for n_messages_collect_tags David Bremner
2020-08-01 12:25 ` [PATCH 03/14] test: split header for lib-message tests David Bremner
2020-08-01 12:25 ` [PATCH 04/14] test: regression test for n_m_get_filenames David Bremner
2020-08-01 12:25 ` [PATCH 05/14] test: add known broken test for n_directory_get_child_directories David Bremner
2020-08-01 12:25 ` [PATCH 06/14] lib: catch exceptions in n_directory_get_child_directories David Bremner
2020-08-01 12:25 ` [PATCH 07/14] test: add known broken test for n_directory_get_child_files David Bremner
2020-08-01 12:25 ` [PATCH 08/14] lib: catch exceptions in n_directory_get_child_files David Bremner
2020-08-01 12:25 ` [PATCH 09/14] test: known broken test for n_directory_delete with closed db David Bremner
2020-08-01 12:25 ` [PATCH 10/14] lib: fix return value for n_directory_delete David Bremner
2020-08-01 12:26 ` [PATCH 11/14] test: regression test for n_directory_{get,set}_mtime David Bremner
2020-08-01 12:26 ` [PATCH 12/14] test: regression test for n_d_get_config_list on closed db David Bremner
2020-08-01 12:26 ` [PATCH 13/14] test: regression test for traversing config list with " David Bremner
2020-08-01 12:26 ` [PATCH 14/14] test: regression tests for n_indexopts_{get,set}_decrypt_policy David Bremner
2020-08-04  1:38 ` Final (v2) batch of patches in API exception handling cleanup David Bremner

Code repositories for project(s) associated with this public inbox

	https://yhetil.org/notmuch.git/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).