lib: make find_message{,by_filename) report errors

Previously, the functions notmuch_database_find_message() and
notmuch_database_find_message_by_filename() functions did not properly
report error condition to the library user.

For more information, read the thread on the notmuch mailing list
starting with my mail "id:871uv2unfd.fsf@gmail.com"

Make these functions accept a pointer to 'notmuch_message_t' as argument
and return notmuch_status_t which may be used to check for any error
condition.

restore: Modify for the new notmuch_database_find_message()
new: Modify for the new notmuch_database_find_message_by_filename()

diff --git a/lib/database.cc b/lib/database.cc
index d43e1143b76ae73a0e061625661ab5b72d3ef753..e77fd53674bb6b3dfbf1f1a6f555ef488b0ddf71 100644 (file)
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -360,13 +360,17 @@ _message_id_compressed (void *ctx, const char *message_id)
     return compressed;
 }
 
-notmuch_message_t *
+notmuch_status_t
 notmuch_database_find_message (notmuch_database_t *notmuch,
-                              const char *message_id)
+                              const char *message_id,
+                              notmuch_message_t **message_ret)
 {
     notmuch_private_status_t status;
     unsigned int doc_id;
 
+    if (message_ret == NULL)
+       return NOTMUCH_STATUS_NULL_POINTER;
+
     if (strlen (message_id) > NOTMUCH_MESSAGE_ID_MAX)
        message_id = _message_id_compressed (notmuch, message_id);
 
@@ -375,14 +379,21 @@ notmuch_database_find_message (notmuch_database_t *notmuch,
                                                       message_id, &doc_id);
 
        if (status == NOTMUCH_PRIVATE_STATUS_NO_DOCUMENT_FOUND)
-           return NULL;
+           *message_ret = NULL;
+       else {
+           *message_ret = _notmuch_message_create (notmuch, notmuch, doc_id,
+                                                   NULL);
+           if (*message_ret == NULL)
+               return NOTMUCH_STATUS_OUT_OF_MEMORY;
+       }
 
-       return _notmuch_message_create (notmuch, notmuch, doc_id, NULL);
+       return NOTMUCH_STATUS_SUCCESS;
     } catch (const Xapian::Error &error) {
        fprintf (stderr, "A Xapian exception occurred finding message: %s.\n",
                 error.get_msg().c_str());
        notmuch->exception_reported = TRUE;
-       return NULL;
+       *message_ret = NULL;
+       return NOTMUCH_STATUS_XAPIAN_EXCEPTION;
     }
 }
 
@@ -1311,7 +1322,9 @@ _get_metadata_thread_id_key (void *ctx, const char *message_id)
 
 /* Find the thread ID to which the message with 'message_id' belongs.
  *
- * Always returns a newly talloced string belonging to 'ctx'.
+ * Note: 'thread_id_ret' must not be NULL!
+ * On success '*thread_id_ret' is set to a newly talloced string belonging to
+ * 'ctx'.
  *
  * Note: If there is no message in the database with the given
  * 'message_id' then a new thread_id will be allocated for this
@@ -1319,25 +1332,30 @@ _get_metadata_thread_id_key (void *ctx, const char *message_id)
  * thread ID can be looked up if the message is added to the database
  * later).
  */
-static const char *
+static notmuch_status_t
 _resolve_message_id_to_thread_id (notmuch_database_t *notmuch,
                                  void *ctx,
-                                 const char *message_id)
+                                 const char *message_id,
+                                 const char **thread_id_ret)
 {
+    notmuch_status_t status;
     notmuch_message_t *message;
     string thread_id_string;
-    const char *thread_id;
     char *metadata_key;
     Xapian::WritableDatabase *db;
 
-    message = notmuch_database_find_message (notmuch, message_id);
+    status = notmuch_database_find_message (notmuch, message_id, &message);
+
+    if (status)
+       return status;
 
     if (message) {
-       thread_id = talloc_steal (ctx, notmuch_message_get_thread_id (message));
+       *thread_id_ret = talloc_steal (ctx,
+                                      notmuch_message_get_thread_id (message));
 
        notmuch_message_destroy (message);
 
-       return thread_id;
+       return NOTMUCH_STATUS_SUCCESS;
     }
 
     /* Message has not been seen yet.
@@ -1351,15 +1369,16 @@ _resolve_message_id_to_thread_id (notmuch_database_t *notmuch,
     thread_id_string = notmuch->xapian_db->get_metadata (metadata_key);
 
     if (thread_id_string.empty()) {
-       thread_id = _notmuch_database_generate_thread_id (notmuch);
-       db->set_metadata (metadata_key, thread_id);
+       *thread_id_ret = talloc_strdup (ctx,
+                                       _notmuch_database_generate_thread_id (notmuch));
+       db->set_metadata (metadata_key, *thread_id_ret);
     } else {
-       thread_id = thread_id_string.c_str();
+       *thread_id_ret = talloc_strdup (ctx, thread_id_string.c_str());
     }
 
     talloc_free (metadata_key);
 
-    return talloc_strdup (ctx, thread_id);
+    return NOTMUCH_STATUS_SUCCESS;
 }
 
 static notmuch_status_t
@@ -1446,9 +1465,12 @@ _notmuch_database_link_message_to_parents (notmuch_database_t *notmuch,
        _notmuch_message_add_term (message, "reference",
                                   parent_message_id);
 
-       parent_thread_id = _resolve_message_id_to_thread_id (notmuch,
-                                                            message,
-                                                            parent_message_id);
+       ret = _resolve_message_id_to_thread_id (notmuch,
+                                               message,
+                                               parent_message_id,
+                                               &parent_thread_id);
+       if (ret)
+           goto DONE;
 
        if (*thread_id == NULL) {
            *thread_id = talloc_strdup (message, parent_thread_id);
@@ -1759,11 +1781,13 @@ notmuch_status_t
 notmuch_database_remove_message (notmuch_database_t *notmuch,
                                 const char *filename)
 {
-    notmuch_message_t *message =
-       notmuch_database_find_message_by_filename (notmuch, filename);
-    notmuch_status_t status = NOTMUCH_STATUS_SUCCESS;
+    notmuch_status_t status;
+    notmuch_message_t *message;
 
-    if (message) {
+    status = notmuch_database_find_message_by_filename (notmuch, filename,
+                                                       &message);
+
+    if (status == NOTMUCH_STATUS_SUCCESS && message) {
            status = _notmuch_message_remove_filename (message, filename);
            if (status == NOTMUCH_STATUS_SUCCESS)
                _notmuch_message_delete (message);
@@ -1776,24 +1800,27 @@ notmuch_database_remove_message (notmuch_database_t *notmuch,
     return status;
 }
 
-notmuch_message_t *
+notmuch_status_t
 notmuch_database_find_message_by_filename (notmuch_database_t *notmuch,
-                                          const char *filename)
+                                          const char *filename,
+                                          notmuch_message_t **message_ret)
 {
     void *local;
     const char *prefix = _find_prefix ("file-direntry");
     char *direntry, *term;
     Xapian::PostingIterator i, end;
-    notmuch_message_t *message = NULL;
     notmuch_status_t status;
 
+    if (message_ret == NULL)
+       return NOTMUCH_STATUS_NULL_POINTER;
+
     local = talloc_new (notmuch);
 
     try {
        status = _notmuch_database_filename_to_direntry (local, notmuch,
                                                         filename, &direntry);
        if (status)
-           return NULL;
+           goto DONE;
 
        term = talloc_asprintf (local, "%s%s", prefix, direntry);
 
@@ -1802,19 +1829,26 @@ notmuch_database_find_message_by_filename (notmuch_database_t *notmuch,
        if (i != end) {
            notmuch_private_status_t private_status;
 
-           message = _notmuch_message_create (notmuch, notmuch,
-                                              *i, &private_status);
+           *message_ret = _notmuch_message_create (notmuch, notmuch, *i,
+                                                   &private_status);
+           if (*message_ret == NULL)
+               status = NOTMUCH_STATUS_OUT_OF_MEMORY;
        }
     } catch (const Xapian::Error &error) {
        fprintf (stderr, "Error: A Xapian exception occurred finding message by filename: %s\n",
                 error.get_msg().c_str());
        notmuch->exception_reported = TRUE;
-       message = NULL;
+       status = NOTMUCH_STATUS_XAPIAN_EXCEPTION;
     }
 
+  DONE:
     talloc_free (local);
 
-    return message;
+    if (status && *message_ret) {
+       notmuch_message_destroy (*message_ret);
+       *message_ret = NULL;
+    }
+    return status;
 }
 
 notmuch_string_list_t *

diff --git a/lib/message.cc b/lib/message.cc
index 531d304339a62e828a298f707e3517a9ead09519..8f22e02a5cde2d37a614febec9b7ad950dc08631 100644 (file)
--- a/lib/message.cc
+++ b/lib/message.cc
@@ -216,11 +216,13 @@ _notmuch_message_create_for_message_id (notmuch_database_t *notmuch,
     unsigned int doc_id;
     char *term;
 
-    *status_ret = NOTMUCH_PRIVATE_STATUS_SUCCESS;
-
-    message = notmuch_database_find_message (notmuch, message_id);
+    *status_ret = (notmuch_private_status_t) notmuch_database_find_message (notmuch,
+                                                                           message_id,
+                                                                           &message);
     if (message)
        return talloc_steal (notmuch, message);
+    else if (*status_ret)
+       return NULL;
 
     term = talloc_asprintf (NULL, "%s%s",
                            _find_prefix ("id"), message_id);

diff --git a/lib/notmuch.h b/lib/notmuch.h
index 6d7a99f2fec61f585090c8f8cf79e313e6199975..c4330e4b3e620119b22790ba9a24163aac7bdf4f 100644 (file)
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -347,35 +347,57 @@ notmuch_database_remove_message (notmuch_database_t *database,
 
 /* Find a message with the given message_id.
  *
- * If the database contains a message with the given message_id, then
- * a new notmuch_message_t object is returned. The caller should call
- * notmuch_message_destroy when done with the message.
+ * If a message with the given message_id is found then, on successful return
+ * (NOTMUCH_STATUS_SUCCESS) '*message' will be initialized to a message
+ * object.  The caller should call notmuch_message_destroy when done with the
+ * message.
  *
- * This function returns NULL in the following situations:
+ * On any failure or when the message is not found, this function initializes
+ * '*message' to NULL. This means, when NOTMUCH_STATUS_SUCCESS is returned, the
+ * caller is supposed to check '*message' for NULL to find out whether the
+ * message with the given message_id was found.
  *
- *     * No message is found with the given message_id
- *     * An out-of-memory situation occurs
- *     * A Xapian exception occurs
+ * Return value:
+ *
+ * NOTMUCH_STATUS_SUCCESS: Successful return, check '*message'.
+ *
+ * NOTMUCH_STATUS_NULL_POINTER: The given 'message' argument is NULL
+ *
+ * NOTMUCH_STATUS_OUT_OF_MEMORY: Out of memory, creating message object
+ *
+ * NOTMUCH_STATUS_XAPIAN_EXCEPTION: A Xapian exception occurred
  */
-notmuch_message_t *
+notmuch_status_t
 notmuch_database_find_message (notmuch_database_t *database,
-                              const char *message_id);
+                              const char *message_id,
+                              notmuch_message_t **message);
 
 /* Find a message with the given filename.
  *
- * If the database contains a message with the given filename, then a
- * new notmuch_message_t object is returned.  The caller should call 
- * notmuch_message_destroy when done with the message.
+ * If the database contains a message with the given filename then, on
+ * successful return (NOTMUCH_STATUS_SUCCESS) '*message' will be initialized to
+ * a message object. The caller should call notmuch_message_destroy when done
+ * with the message.
  *
- * This function returns NULL in the following situations:
+ * On any failure or when the message is not found, this function initializes
+ * '*message' to NULL. This means, when NOTMUCH_STATUS_SUCCESS is returned, the
+ * caller is supposed to check '*message' for NULL to find out whether the
+ * message with the given filename is found.
  *
- *     * No message is found with the given filename
- *     * An out-of-memory situation occurs
- *     * A Xapian exception occurs
+ * Return value:
+ *
+ * NOTMUCH_STATUS_SUCCESS: Successful return, check '*message'
+ *
+ * NOTMUCH_STATUS_NULL_POINTER: The given 'message' argument is NULL
+ *
+ * NOTMUCH_STATUS_OUT_OF_MEMORY: Out of memory, creating the message object
+ *
+ * NOTMUCH_STATUS_XAPIAN_EXCEPTION: A Xapian exception occurred
  */
-notmuch_message_t *
+notmuch_status_t
 notmuch_database_find_message_by_filename (notmuch_database_t *notmuch,
-                                          const char *filename);
+                                          const char *filename,
+                                          notmuch_message_t **message);
 
 /* Return a list of all tags found in the database.
  *

diff --git a/notmuch-new.c b/notmuch-new.c
index e79593cd092dd3345a8bac182234833ecfc92333..96a1e31e732da0b4d16cbdc374848d810904eb96 100644 (file)
--- a/notmuch-new.c
+++ b/notmuch-new.c
@@ -743,7 +743,9 @@ remove_filename (notmuch_database_t *notmuch,
     status = notmuch_database_begin_atomic (notmuch);
     if (status)
        return status;
-    message = notmuch_database_find_message_by_filename (notmuch, path);
+    status = notmuch_database_find_message_by_filename (notmuch, path, &message);
+    if (status || message == NULL)
+       return status;
     status = notmuch_database_remove_message (notmuch, path);
     if (status == NOTMUCH_STATUS_DUPLICATE_MESSAGE_ID) {
        add_files_state->renamed_messages++;

diff --git a/notmuch-restore.c b/notmuch-restore.c
index f095f64a76b7054dbda62b1fd8f9f4efa975117b..e4a5355beb4f2e5fd9e1eb12ef7dd9816d292951 100644 (file)
--- a/notmuch-restore.c
+++ b/notmuch-restore.c
@@ -87,10 +87,13 @@ notmuch_restore_command (unused (void *ctx), int argc, char *argv[])
        file_tags = xstrndup (line + match[2].rm_so,
                              match[2].rm_eo - match[2].rm_so);
 
-       message = notmuch_database_find_message (notmuch, message_id);
-       if (message == NULL) {
-           fprintf (stderr, "Warning: Cannot apply tags to missing message: %s\n",
-                    message_id);
+       status = notmuch_database_find_message (notmuch, message_id, &message);
+       if (status || message == NULL) {
+           fprintf (stderr, "Warning: Cannot apply tags to %smessage: %s\n",
+                    message ? "" : "missing ", message_id);
+           if (status)
+               fprintf (stderr, "%s\n",
+                        notmuch_status_to_string(status));
            goto NEXT_LINE;
        }