unofficial mirror of notmuch@notmuchmail.org
 help / color / mirror / code / Atom feed
* notmuch_message* error handling cleanup part II
@ 2022-05-25 10:51 David Bremner
  2022-05-25 10:51 ` [PATCH 1/6] test: error handling for n_m_tags_to_maildir_flags David Bremner
                   ` (6 more replies)
  0 siblings, 7 replies; 8+ messages in thread
From: David Bremner @ 2022-05-25 10:51 UTC (permalink / raw)
  To: notmuch

This is a continuation of the series [1]. It should in principle be
applicable to master, but I have not tried resolving the resulting
conflicts.

The big picture change here is having
_notmuch_database_ensure_writable check for the database being
open. The "open" flag was added for some other reason (config related
iirc), but now that we have it, we may as well use it to make certain
failures more atomic.


[1]: id:20220523233901.3506880-1-david@tethera.net


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

* [PATCH 1/6] test: error handling for n_m_tags_to_maildir_flags
  2022-05-25 10:51 notmuch_message* error handling cleanup part II David Bremner
@ 2022-05-25 10:51 ` David Bremner
  2022-05-25 10:51 ` [PATCH 2/6] test: add known broken test for notmuch_tags_valid (NULL) David Bremner
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: David Bremner @ 2022-05-25 10:51 UTC (permalink / raw)
  To: notmuch

The closed database case should fail gracefully, but currently it
segfaults.
---
 test/T566-lib-message.sh | 54 +++++++++++++++++++++++++++++++++++++---
 1 file changed, 51 insertions(+), 3 deletions(-)

diff --git a/test/T566-lib-message.sh b/test/T566-lib-message.sh
index 30040634..2ad24543 100755
--- a/test/T566-lib-message.sh
+++ b/test/T566-lib-message.sh
@@ -27,7 +27,7 @@ int main (int argc, char** argv)
    notmuch_status_t stat;
    char *msg = NULL;
    notmuch_message_t *message = NULL;
-   const char *id = "1258471718-6781-1-git-send-email-dottedmag@dottedmag.net";
+   const char *id = "87pr7gqidx.fsf@yoom.home.cworth.org";
 
    stat = notmuch_database_open_verbose (argv[1], NOTMUCH_DATABASE_MODE_READ_WRITE, &db, &msg);
    if (stat != NOTMUCH_STATUS_SUCCESS) {
@@ -82,7 +82,7 @@ cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR}
 EOF
 cat <<EOF > EXPECTED
 == stdout ==
-1258471718-6781-1-git-send-email-dottedmag@dottedmag.net
+87pr7gqidx.fsf@yoom.home.cworth.org
 1
 == stderr ==
 EOF
@@ -154,7 +154,7 @@ cat c_head0 - c_tail <<'EOF' | test_C ${MAIL_DIR}
 EOF
 cat <<EOF > EXPECTED
 == stdout ==
-MAIL_DIR/01:2,
+MAIL_DIR/cur/40:2,
 SUCCESS
 == stderr ==
 EOF
@@ -372,6 +372,54 @@ cat <<EOF > EXPECTED
 EOF
 test_expect_equal_file EXPECTED OUTPUT
 
+test_begin_subtest "Handle converting tags to maildir flags with closed db"
+test_subtest_known_broken
+cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR}
+    {
+	notmuch_status_t status;
+	status = notmuch_message_tags_to_maildir_flags (message);
+	printf("%d\n%d\n", message != NULL, status != NOTMUCH_STATUS_SUCCESS);
+    }
+EOF
+cat <<EOF > EXPECTED
+== stdout ==
+1
+1
+== stderr ==
+EOF
+test_expect_equal_file EXPECTED OUTPUT
+
+POSTLIST_PATH=(${MAIL_DIR}/.notmuch/xapian/postlist.*)
+test_begin_subtest "Handle converting tags to maildir flags with corrupted db"
+backup_database
+cat c_head0 - c_tail <<'EOF' | test_C ${MAIL_DIR} ${POSTLIST_PATH}
+    {
+        notmuch_status_t status;
+
+        status = notmuch_message_add_tag (message, "draft");
+        if (status) exit(1);
+
+        int fd = open(argv[2],O_WRONLY|O_TRUNC);
+        if (fd < 0) {
+            fprintf (stderr, "error opening %s\n", argv[1]);
+            exit (1);
+        }
+
+        status = notmuch_message_tags_to_maildir_flags (message);
+        printf("%d\n%d\n", message != NULL, status != NOTMUCH_STATUS_SUCCESS);
+    }
+EOF
+cat <<EOF > EXPECTED
+== stdout ==
+1
+1
+== stderr ==
+EOF
+restore_database
+notmuch new
+notmuch tag -draft id:87pr7gqidx.fsf@yoom.home.cworth.org
+test_expect_equal_file EXPECTED OUTPUT
+
 test_begin_subtest "Handle removing all tags with closed db"
 cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR}
     {
-- 
2.35.2

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

* [PATCH 2/6] test: add known broken test for notmuch_tags_valid (NULL)
  2022-05-25 10:51 notmuch_message* error handling cleanup part II David Bremner
  2022-05-25 10:51 ` [PATCH 1/6] test: error handling for n_m_tags_to_maildir_flags David Bremner
@ 2022-05-25 10:51 ` David Bremner
  2022-05-25 10:51 ` [PATCH 3/6] lib/tag: handle NULL argument to notmuch_tags_valid David Bremner
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: David Bremner @ 2022-05-25 10:51 UTC (permalink / raw)
  To: notmuch

This should return false, but currently segfaults.

Start a new file for tags library API related tests. This is maybe
overkill, but new C boilerplate which doesn't corrupt the database is
needed anyway.
---
 test/T560-lib-error.sh | 18 ---------
 test/T565-lib-tags.sh  | 86 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 86 insertions(+), 18 deletions(-)
 create mode 100755 test/T565-lib-tags.sh

diff --git a/test/T560-lib-error.sh b/test/T560-lib-error.sh
index 1f4482cb..a6d89346 100755
--- a/test/T560-lib-error.sh
+++ b/test/T560-lib-error.sh
@@ -245,24 +245,6 @@ EOF
 test_expect_equal_file EXPECTED OUTPUT.clean
 restore_database
 
-backup_database
-test_begin_subtest "Xapian exception getting tags"
-cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR} ${POSTLIST_PATH}
-   {
-       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 c_head - c_tail <<'EOF' | test_C ${MAIL_DIR} ${POSTLIST_PATH}
diff --git a/test/T565-lib-tags.sh b/test/T565-lib-tags.sh
new file mode 100755
index 00000000..0f3595d2
--- /dev/null
+++ b/test/T565-lib-tags.sh
@@ -0,0 +1,86 @@
+#!/usr/bin/env bash
+test_description="API tests for tags"
+
+. $(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 <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;
+   char *msg = NULL;
+   int fd;
+
+   stat = notmuch_database_open_with_config (argv[1],
+					      NOTMUCH_DATABASE_MODE_READ_WRITE,
+					      NULL, NULL, &db, &msg);
+   if (stat != NOTMUCH_STATUS_SUCCESS) {
+     fprintf (stderr, "error opening database\n%s\n%s\n", notmuch_status_to_string (stat), msg ? msg : "");
+     exit (1);
+   }
+EOF
+cat <<'EOF' > c_tail
+   if (stat) {
+       const char *stat_str = notmuch_database_status_string (db);
+       if (stat_str)
+	   fputs (stat_str, stderr);
+    }
+
+}
+EOF
+
+POSTLIST_PATH=(${MAIL_DIR}/.notmuch/xapian/postlist.*)
+
+backup_database
+test_begin_subtest "Xapian exception getting tags"
+cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR} ${POSTLIST_PATH}
+   {
+      notmuch_tags_t *tags = NULL;
+      fd = open(argv[2],O_WRONLY|O_TRUNC);
+      if (fd < 0) {
+	  fprintf (stderr, "error opening %s\n", argv[1]);
+	  exit (1);
+       }
+       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
+
+test_begin_subtest "NULL tags are not valid"
+test_subtest_known_broken
+cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR}
+   {
+       notmuch_bool_t valid = TRUE;
+       valid = notmuch_tags_valid (NULL);
+       fprintf(stdout, "valid = %d\n", valid);
+   }
+EOF
+cat <<'EOF' >EXPECTED
+== stdout ==
+valid = 0
+== stderr ==
+EOF
+test_expect_equal_file EXPECTED OUTPUT
+
+test_done
-- 
2.35.2

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

* [PATCH 3/6] lib/tag: handle NULL argument to notmuch_tags_valid
  2022-05-25 10:51 notmuch_message* error handling cleanup part II David Bremner
  2022-05-25 10:51 ` [PATCH 1/6] test: error handling for n_m_tags_to_maildir_flags David Bremner
  2022-05-25 10:51 ` [PATCH 2/6] test: add known broken test for notmuch_tags_valid (NULL) David Bremner
@ 2022-05-25 10:51 ` David Bremner
  2022-05-25 10:51 ` [PATCH 4/6] lib: Add missing private status values David Bremner
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: David Bremner @ 2022-05-25 10:51 UTC (permalink / raw)
  To: notmuch

Make the behaviour when passed NULL consistent with
notmuch_filenames_valid. The library already passes the result of
notmuch_message_get_tags without checking for NULL, so it should be
handled.
---
 lib/notmuch.h         | 3 +++
 lib/tags.c            | 2 +-
 test/T565-lib-tags.sh | 1 -
 3 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/lib/notmuch.h b/lib/notmuch.h
index 2e6ec2af..44263a66 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -2262,6 +2262,9 @@ notmuch_message_properties_destroy (notmuch_message_properties_t *properties);
  * valid string. Whereas when this function returns FALSE,
  * notmuch_tags_get will return NULL.
  *
+ * It is acceptable to pass NULL for 'tags', in which case this
+ * function will always return FALSE.
+
  * See the documentation of notmuch_message_get_tags for example code
  * showing how to iterate over a notmuch_tags_t object.
  */
diff --git a/lib/tags.c b/lib/tags.c
index c7d3f66f..ec5366ff 100644
--- a/lib/tags.c
+++ b/lib/tags.c
@@ -48,7 +48,7 @@ _notmuch_tags_create (const void *ctx, notmuch_string_list_t *list)
 notmuch_bool_t
 notmuch_tags_valid (notmuch_tags_t *tags)
 {
-    return tags->iterator != NULL;
+    return tags && (tags->iterator != NULL);
 }
 
 const char *
diff --git a/test/T565-lib-tags.sh b/test/T565-lib-tags.sh
index 0f3595d2..2a59f8dd 100755
--- a/test/T565-lib-tags.sh
+++ b/test/T565-lib-tags.sh
@@ -68,7 +68,6 @@ test_expect_equal_file EXPECTED OUTPUT.clean
 restore_database
 
 test_begin_subtest "NULL tags are not valid"
-test_subtest_known_broken
 cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR}
    {
        notmuch_bool_t valid = TRUE;
-- 
2.35.2

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

* [PATCH 4/6] lib: Add missing private status values.
  2022-05-25 10:51 notmuch_message* error handling cleanup part II David Bremner
                   ` (2 preceding siblings ...)
  2022-05-25 10:51 ` [PATCH 3/6] lib/tag: handle NULL argument to notmuch_tags_valid David Bremner
@ 2022-05-25 10:51 ` David Bremner
  2022-05-25 10:51 ` [PATCH 5/6] lib: add NOTMUCH_STATUS_CLOSED_DATABASE, use in _n_d_ensure_writable David Bremner
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: David Bremner @ 2022-05-25 10:51 UTC (permalink / raw)
  To: notmuch

These were missed when the corresponding status codes were added.
---
 lib/notmuch-private.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/lib/notmuch-private.h b/lib/notmuch-private.h
index 1c2290b2..69debcfe 100644
--- a/lib/notmuch-private.h
+++ b/lib/notmuch-private.h
@@ -144,6 +144,8 @@ typedef enum {
     NOTMUCH_PRIVATE_STATUS_NO_CONFIG				= NOTMUCH_STATUS_NO_CONFIG,
     NOTMUCH_PRIVATE_STATUS_NO_DATABASE				= NOTMUCH_STATUS_NO_DATABASE,
     NOTMUCH_PRIVATE_STATUS_DATABASE_EXISTS			= NOTMUCH_STATUS_DATABASE_EXISTS,
+    NOTMUCH_PRIVATE_STATUS_NO_MAIL_ROOT				= NOTMUCH_STATUS_NO_MAIL_ROOT,
+    NOTMUCH_PRIVATE_STATUS_BAD_QUERY_SYNTAX			= NOTMUCH_STATUS_BAD_QUERY_SYNTAX,
 
     /* Then add our own private values. */
     NOTMUCH_PRIVATE_STATUS_TERM_TOO_LONG		= NOTMUCH_STATUS_LAST_STATUS,
-- 
2.35.2

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

* [PATCH 5/6] lib: add NOTMUCH_STATUS_CLOSED_DATABASE, use in _n_d_ensure_writable
  2022-05-25 10:51 notmuch_message* error handling cleanup part II David Bremner
                   ` (3 preceding siblings ...)
  2022-05-25 10:51 ` [PATCH 4/6] lib: Add missing private status values David Bremner
@ 2022-05-25 10:51 ` David Bremner
  2022-05-25 10:51 ` [PATCH 6/6] lib: check for writable db in n_m_tags_maildir_flags David Bremner
  2022-06-25 19:38 ` notmuch_message* error handling cleanup part II David Bremner
  6 siblings, 0 replies; 8+ messages in thread
From: David Bremner @ 2022-05-25 10:51 UTC (permalink / raw)
  To: notmuch

In order for a database to actually be writeable, it must be the case that it
is open, not just the correct type of Xapian object. By explicitely
checking, we are able to provide better error reporting, in particular
for the previously broken test in T566-lib-message.
---
 bindings/python-cffi/notmuch2/_build.py |  1 +
 lib/database.cc                         | 10 +++++++---
 lib/notmuch-private.h                   |  1 +
 lib/notmuch.h                           |  4 ++++
 test/T562-lib-database.sh               |  8 ++++----
 test/T563-lib-directory.sh              | 26 ++++---------------------
 test/T566-lib-message.sh                | 10 +++++-----
 7 files changed, 26 insertions(+), 34 deletions(-)

diff --git a/bindings/python-cffi/notmuch2/_build.py b/bindings/python-cffi/notmuch2/_build.py
index 349bb79d..65d7dcb6 100644
--- a/bindings/python-cffi/notmuch2/_build.py
+++ b/bindings/python-cffi/notmuch2/_build.py
@@ -55,6 +55,7 @@ ffibuilder.cdef(
         NOTMUCH_STATUS_DATABASE_EXISTS,
         NOTMUCH_STATUS_BAD_QUERY_SYNTAX,
         NOTMUCH_STATUS_NO_MAIL_ROOT,
+        NOTMUCH_STATUS_CLOSED_DATABASE,
         NOTMUCH_STATUS_LAST_STATUS
     } notmuch_status_t;
     typedef enum {
diff --git a/lib/database.cc b/lib/database.cc
index df83e204..c05d70d3 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -476,6 +476,11 @@ _notmuch_database_ensure_writable (notmuch_database_t *notmuch)
 	return NOTMUCH_STATUS_READ_ONLY_DATABASE;
     }
 
+    if (! notmuch->open) {
+	_notmuch_database_log (notmuch, "Cannot write to a closed database.\n");
+	return NOTMUCH_STATUS_CLOSED_DATABASE;
+    }
+
     return NOTMUCH_STATUS_SUCCESS;
 }
 
@@ -852,9 +857,8 @@ notmuch_database_upgrade (notmuch_database_t *notmuch,
     notmuch_query_t *query = NULL;
     unsigned int count = 0, total = 0;
 
-    status = _notmuch_database_ensure_writable (notmuch);
-    if (status)
-	return status;
+    if (_notmuch_database_mode (notmuch) != NOTMUCH_DATABASE_MODE_READ_WRITE)
+	return NOTMUCH_STATUS_READ_ONLY_DATABASE;
 
     db = notmuch->writable_xapian_db;
 
diff --git a/lib/notmuch-private.h b/lib/notmuch-private.h
index 69debcfe..1d3d2b0c 100644
--- a/lib/notmuch-private.h
+++ b/lib/notmuch-private.h
@@ -146,6 +146,7 @@ typedef enum {
     NOTMUCH_PRIVATE_STATUS_DATABASE_EXISTS			= NOTMUCH_STATUS_DATABASE_EXISTS,
     NOTMUCH_PRIVATE_STATUS_NO_MAIL_ROOT				= NOTMUCH_STATUS_NO_MAIL_ROOT,
     NOTMUCH_PRIVATE_STATUS_BAD_QUERY_SYNTAX			= NOTMUCH_STATUS_BAD_QUERY_SYNTAX,
+    NOTMUCH_PRIVATE_STATUS_CLOSED_DATABASE			= NOTMUCH_STATUS_CLOSED_DATABASE,
 
     /* Then add our own private values. */
     NOTMUCH_PRIVATE_STATUS_TERM_TOO_LONG		= NOTMUCH_STATUS_LAST_STATUS,
diff --git a/lib/notmuch.h b/lib/notmuch.h
index 44263a66..0b0540b1 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -228,6 +228,10 @@ typedef enum {
      * No mail root could be deduced from parameters and environment
      */
     NOTMUCH_STATUS_NO_MAIL_ROOT,
+    /**
+     * Database is not fully opened, or has been closed
+     */
+    NOTMUCH_STATUS_CLOSED_DATABASE,
     /**
      * Not an actual status value. Just a way to find out how many
      * valid status values there are.
diff --git a/test/T562-lib-database.sh b/test/T562-lib-database.sh
index 2314efd2..324b4544 100755
--- a/test/T562-lib-database.sh
+++ b/test/T562-lib-database.sh
@@ -241,14 +241,14 @@ cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR}
         const char *path = talloc_asprintf(db, "%s/01:2,", argv[1]);
         EXPECT0(notmuch_database_close (db));
         stat = notmuch_database_index_file (db, path, NULL, &msg);
-        printf ("%d\n", stat == NOTMUCH_STATUS_XAPIAN_EXCEPTION);
+        printf ("%d\n", stat == NOTMUCH_STATUS_CLOSED_DATABASE);
     }
 EOF
 cat <<EOF > EXPECTED
 == stdout ==
 1
 == stderr ==
-A Xapian exception occurred finding message: Database has been closed.
+Cannot write to a closed database.
 EOF
 test_expect_equal_file EXPECTED OUTPUT
 
@@ -356,14 +356,14 @@ cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR}
     {
         EXPECT0(notmuch_database_close (db));
         stat = notmuch_database_set_config (db, "foo", "bar");
-        printf("%d\n", stat == NOTMUCH_STATUS_XAPIAN_EXCEPTION);
+        printf("%d\n", stat == NOTMUCH_STATUS_CLOSED_DATABASE);
     }
 EOF
 cat <<EOF > EXPECTED
 == stdout ==
 1
 == stderr ==
-Error: A Xapian exception occurred setting metadata: Database has been closed
+Cannot write to a closed database.
 EOF
 test_expect_equal_file EXPECTED OUTPUT
 
diff --git a/test/T563-lib-directory.sh b/test/T563-lib-directory.sh
index ebd7fcb2..9f07101b 100755
--- a/test/T563-lib-directory.sh
+++ b/test/T563-lib-directory.sh
@@ -77,14 +77,14 @@ test_begin_subtest "delete directory document for a closed db"
 cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR}
     {
         stat = notmuch_directory_delete (dir);
-        printf ("%d\n", stat == NOTMUCH_STATUS_XAPIAN_EXCEPTION);
+        printf ("%d\n", stat == NOTMUCH_STATUS_CLOSED_DATABASE);
     }
 EOF
 cat <<EOF > EXPECTED
 == stdout ==
 1
 == stderr ==
-A Xapian exception occurred deleting directory entry: Database has been closed.
+Cannot write to a closed database.
 EOF
 test_expect_equal_file EXPECTED OUTPUT
 restore_database
@@ -95,32 +95,14 @@ 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);
+        printf ("%d\n", stat == NOTMUCH_STATUS_CLOSED_DATABASE);
     }
 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
-
-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.
+Cannot write to a closed database.
 EOF
 test_expect_equal_file EXPECTED OUTPUT
 restore_database
diff --git a/test/T566-lib-message.sh b/test/T566-lib-message.sh
index 2ad24543..3a5e9607 100755
--- a/test/T566-lib-message.sh
+++ b/test/T566-lib-message.sh
@@ -229,7 +229,7 @@ cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR}
     {
         notmuch_status_t status;
         status = notmuch_message_add_tag (message, "boom");
-        printf("%d\n%d\n", message != NULL, status == NOTMUCH_STATUS_XAPIAN_EXCEPTION);
+        printf("%d\n%d\n", message != NULL, status == NOTMUCH_STATUS_CLOSED_DATABASE);
     }
 EOF
 cat <<EOF > EXPECTED
@@ -245,7 +245,7 @@ cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR}
     {
         notmuch_status_t status;
         status = notmuch_message_remove_tag (message, "boom");
-        printf("%d\n%d\n", message != NULL, status == NOTMUCH_STATUS_XAPIAN_EXCEPTION);
+        printf("%d\n%d\n", message != NULL, status == NOTMUCH_STATUS_CLOSED_DATABASE);
     }
 EOF
 cat <<EOF > EXPECTED
@@ -425,7 +425,7 @@ cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR}
     {
         notmuch_status_t status;
         status = notmuch_message_remove_all_tags (message);
-        printf("%d\n%d\n", message != NULL, status == NOTMUCH_STATUS_XAPIAN_EXCEPTION);
+        printf("%d\n%d\n", message != NULL, status == NOTMUCH_STATUS_CLOSED_DATABASE);
     }
 EOF
 cat <<EOF > EXPECTED
@@ -441,7 +441,7 @@ cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR}
     {
         notmuch_status_t status;
         status = notmuch_message_freeze (message);
-        printf("%d\n%d\n", message != NULL, status == NOTMUCH_STATUS_SUCCESS);
+        printf("%d\n%d\n", message != NULL, status == NOTMUCH_STATUS_CLOSED_DATABASE);
     }
 EOF
 cat <<EOF > EXPECTED
@@ -457,7 +457,7 @@ cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR}
     {
         notmuch_status_t status;
         status = notmuch_message_thaw (message);
-        printf("%d\n%d\n", message != NULL, status == NOTMUCH_STATUS_UNBALANCED_FREEZE_THAW);
+        printf("%d\n%d\n", message != NULL, status == NOTMUCH_STATUS_CLOSED_DATABASE);
     }
 EOF
 cat <<EOF > EXPECTED
-- 
2.35.2

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

* [PATCH 6/6] lib: check for writable db in n_m_tags_maildir_flags
  2022-05-25 10:51 notmuch_message* error handling cleanup part II David Bremner
                   ` (4 preceding siblings ...)
  2022-05-25 10:51 ` [PATCH 5/6] lib: add NOTMUCH_STATUS_CLOSED_DATABASE, use in _n_d_ensure_writable David Bremner
@ 2022-05-25 10:51 ` David Bremner
  2022-06-25 19:38 ` notmuch_message* error handling cleanup part II David Bremner
  6 siblings, 0 replies; 8+ messages in thread
From: David Bremner @ 2022-05-25 10:51 UTC (permalink / raw)
  To: notmuch

The database needs to be writable because the list of stored file
names will change in general.
---
 lib/message.cc           | 4 ++++
 test/T566-lib-message.sh | 1 -
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/lib/message.cc b/lib/message.cc
index ae152df7..1df804bc 100644
--- a/lib/message.cc
+++ b/lib/message.cc
@@ -2038,6 +2038,10 @@ notmuch_message_tags_to_maildir_flags (notmuch_message_t *message)
     char *to_set, *to_clear;
     notmuch_status_t status = NOTMUCH_STATUS_SUCCESS;
 
+    status = _notmuch_database_ensure_writable (message->notmuch);
+    if (status)
+	return status;
+
     _get_maildir_flag_actions (message, &to_set, &to_clear);
 
     for (filenames = notmuch_message_get_filenames (message);
diff --git a/test/T566-lib-message.sh b/test/T566-lib-message.sh
index 3a5e9607..551898c6 100755
--- a/test/T566-lib-message.sh
+++ b/test/T566-lib-message.sh
@@ -373,7 +373,6 @@ EOF
 test_expect_equal_file EXPECTED OUTPUT
 
 test_begin_subtest "Handle converting tags to maildir flags with closed db"
-test_subtest_known_broken
 cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR}
     {
 	notmuch_status_t status;
-- 
2.35.2

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

* Re: notmuch_message* error handling cleanup part II
  2022-05-25 10:51 notmuch_message* error handling cleanup part II David Bremner
                   ` (5 preceding siblings ...)
  2022-05-25 10:51 ` [PATCH 6/6] lib: check for writable db in n_m_tags_maildir_flags David Bremner
@ 2022-06-25 19:38 ` David Bremner
  6 siblings, 0 replies; 8+ messages in thread
From: David Bremner @ 2022-06-25 19:38 UTC (permalink / raw)
  To: notmuch

David Bremner <david@tethera.net> writes:

> This is a continuation of the series [1]. It should in principle be
> applicable to master, but I have not tried resolving the resulting
> conflicts.
>
> The big picture change here is having
> _notmuch_database_ensure_writable check for the database being
> open. The "open" flag was added for some other reason (config related
> iirc), but now that we have it, we may as well use it to make certain
> failures more atomic.
>
>
> [1]: id:20220523233901.3506880-1-david@tethera.net

series applied to master.

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

end of thread, other threads:[~2022-06-25 19:38 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-25 10:51 notmuch_message* error handling cleanup part II David Bremner
2022-05-25 10:51 ` [PATCH 1/6] test: error handling for n_m_tags_to_maildir_flags David Bremner
2022-05-25 10:51 ` [PATCH 2/6] test: add known broken test for notmuch_tags_valid (NULL) David Bremner
2022-05-25 10:51 ` [PATCH 3/6] lib/tag: handle NULL argument to notmuch_tags_valid David Bremner
2022-05-25 10:51 ` [PATCH 4/6] lib: Add missing private status values David Bremner
2022-05-25 10:51 ` [PATCH 5/6] lib: add NOTMUCH_STATUS_CLOSED_DATABASE, use in _n_d_ensure_writable David Bremner
2022-05-25 10:51 ` [PATCH 6/6] lib: check for writable db in n_m_tags_maildir_flags David Bremner
2022-06-25 19:38 ` notmuch_message* error handling cleanup part II 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).