add_message: Propagate error status from notmuch_message_create_for_message_id
authorCarl Worth <cworth@cworth.org>
Sun, 25 Oct 2009 16:47:21 +0000 (09:47 -0700)
committerCarl Worth <cworth@cworth.org>
Sun, 25 Oct 2009 17:54:43 +0000 (10:54 -0700)
What a great feeling to remove an XXX comment.

database.cc
message.cc
notmuch-private.h
notmuch.h
query.cc

index b392914190bb2f6247d89af371e0a86d56301938..3f8ea903c7f7cdf33038a3f0c9b43ffc50942be5 100644 (file)
@@ -284,7 +284,7 @@ notmuch_database_find_message (notmuch_database_t *notmuch,
     if (status == NOTMUCH_PRIVATE_STATUS_NO_DOCUMENT_FOUND)
        return NULL;
 
-    return _notmuch_message_create (notmuch, notmuch, doc_id);
+    return _notmuch_message_create (notmuch, notmuch, doc_id, NULL);
 }
 
 /* Return one or more thread_ids, (as a GPtrArray of strings), for the
@@ -762,17 +762,12 @@ notmuch_database_add_message (notmuch_database_t *notmuch,
         * database). */
 
        /* Use NULL for owner since we want to free this locally. */
-
-       /* XXX: This call can fail by either out-of-memory or an
-        * "impossible" Xapian exception. We should rewrite it to
-        * allow us to propagate the error status. */
-       message = _notmuch_message_create_for_message_id (NULL, notmuch,
-                                                         message_id);
-       if (message == NULL) {
-           fprintf (stderr, "Internal error. This shouldn't happen.\n\n");
-           fprintf (stderr, "I mean, it's possible you ran out of memory, but then this code path is still an internal error since it should have detected that and propagated the status value up the stack.\n");
-           exit (1);
-       }
+       message = _notmuch_message_create_for_message_id (NULL,
+                                                         notmuch,
+                                                         message_id,
+                                                         &ret);
+       if (message == NULL)
+           goto DONE;
 
        /* Has a message previously been added with the same ID? */
        old_filename = notmuch_message_get_filename (message);
index 80832cae6a8c9f5ba1061b60f6991020c7a859bd..b66ca7e2dc3cd7fdeb8a317084f360bd963dc34d 100644 (file)
@@ -80,23 +80,43 @@ _notmuch_message_destructor (notmuch_message_t *message)
  * caller *is* responsible for calling notmuch_message_destroy.
  *
  * If no document exists in the database with document ID of 'doc_id'
- * then this function returns NULL.
+ * then this function returns NULL and sets *status to
+ * NOTMUCH_PRIVATE_STATUS_NO_DOCUMENT_FOUND.
+ *
+ * This function can also fail to due lack of available memory,
+ * returning NULL and optionally setting *status to
+ * NOTMUCH_PRIVATE_STATUS_OUT_OF_MEMORY. Caller can pass NULL
+ * for status if uninterested in distinguishing these two cases.
  */
 notmuch_message_t *
 _notmuch_message_create (const void *talloc_owner,
                         notmuch_database_t *notmuch,
-                        unsigned int doc_id)
+                        unsigned int doc_id,
+                        notmuch_private_status_t *status)
 {
     notmuch_message_t *message;
 
+    if (status)
+       *status = NOTMUCH_PRIVATE_STATUS_SUCCESS;
+
     message = talloc (talloc_owner, notmuch_message_t);
-    if (unlikely (message == NULL))
+    if (unlikely (message == NULL)) {
+       if (status)
+           *status = NOTMUCH_PRIVATE_STATUS_OUT_OF_MEMORY;
        return NULL;
+    }
 
     message->notmuch = notmuch;
     message->doc_id = doc_id;
     message->message_id = NULL; /* lazily created */
     message->filename = NULL; /* lazily created */
+
+    /* This is C++'s creepy "placement new", which is really just an
+     * ugly way to call a constructor for a pre-allocated object. So
+     * it's really not an error to not be checking for OUT_OF_MEMORY
+     * here, since this "new" isn't actually allocating memory. This
+     * is language-design comedy of the wrong kind. */
+
     new (&message->doc) Xapian::Document;
 
     talloc_set_destructor (message, _notmuch_message_destructor);
@@ -105,6 +125,8 @@ _notmuch_message_create (const void *talloc_owner,
        message->doc = notmuch->xapian_db->get_document (doc_id);
     } catch (const Xapian::DocNotFoundError &error) {
        talloc_free (message);
+       if (status)
+           *status = NOTMUCH_PRIVATE_STATUS_NO_DOCUMENT_FOUND;
        return NULL;
     }
 
@@ -127,32 +149,61 @@ _notmuch_message_create (const void *talloc_owner,
  * If there is already a document with message ID 'message_id' in the
  * database, then the returned message can be used to query/modify the
  * document. Otherwise, a new document will be inserted into the
- * database before this function returns;
+ * database before this function returns.
+ *
+ * If an error occurs, this function will return NULL and *status
+ * will be set as appropriate. (The status pointer argument must
+ * not be NULL.)
  */
 notmuch_message_t *
 _notmuch_message_create_for_message_id (const void *talloc_owner,
                                        notmuch_database_t *notmuch,
-                                       const char *message_id)
+                                       const char *message_id,
+                                       notmuch_status_t *status)
 {
+    notmuch_private_status_t private_status;
     notmuch_message_t *message;
     Xapian::Document doc;
     unsigned int doc_id;
     char *term;
 
+    *status = NOTMUCH_STATUS_SUCCESS;
+
     message = notmuch_database_find_message (notmuch, message_id);
     if (message)
        return talloc_steal (talloc_owner, message);
 
     term = talloc_asprintf (NULL, "%s%s",
                            _find_prefix ("id"), message_id);
-    doc.add_term (term);
-    talloc_free (term);
+    if (term == NULL) {
+       *status = NOTMUCH_STATUS_OUT_OF_MEMORY;
+       return NULL;
+    }
 
-    doc.add_value (NOTMUCH_VALUE_MESSAGE_ID, message_id);
+    try {
+       doc.add_term (term);
+       talloc_free (term);
+
+       doc.add_value (NOTMUCH_VALUE_MESSAGE_ID, message_id);
+
+       doc_id = notmuch->xapian_db->add_document (doc);
+    } catch (const Xapian::Error &error) {
+       *status = NOTMUCH_STATUS_XAPIAN_EXCEPTION;
+       return NULL;
+    }
 
-    doc_id = notmuch->xapian_db->add_document (doc);
+    message = _notmuch_message_create (talloc_owner, notmuch,
+                                      doc_id, &private_status);
 
-    return _notmuch_message_create (talloc_owner, notmuch, doc_id);
+    if (private_status >= (notmuch_private_status_t) NOTMUCH_STATUS_LAST_STATUS)
+    {
+       fprintf (stderr, "Internal error: Failed to find document immediately after adding it.\n");
+       exit (1);
+    }
+
+    *status = (notmuch_status_t) private_status;
+
+    return message;
 }
 
 const char *
index 1302cd30b7eea0480bc2aae11fa9ff0a276b703d..53ea75fa4d1c7bb09f2f08af50afd821b3ab0c5d 100644 (file)
@@ -83,6 +83,7 @@ typedef enum {
 typedef enum _notmuch_private_status {
     /* First, copy all the public status values. */
     NOTMUCH_PRIVATE_STATUS_SUCCESS = NOTMUCH_STATUS_SUCCESS,
+    NOTMUCH_PRIVATE_STATUS_OUT_OF_MEMORY = NOTMUCH_STATUS_OUT_OF_MEMORY,
     NOTMUCH_PRIVATE_STATUS_XAPIAN_EXCEPTION = NOTMUCH_STATUS_XAPIAN_EXCEPTION,
     NOTMUCH_PRIVATE_STATUS_FILE_NOT_EMAIL = NOTMUCH_STATUS_FILE_NOT_EMAIL,
     NOTMUCH_PRIVATE_STATUS_NULL_POINTER = NOTMUCH_STATUS_NULL_POINTER,
@@ -100,12 +101,14 @@ typedef enum _notmuch_private_status {
 notmuch_message_t *
 _notmuch_message_create (const void *talloc_owner,
                         notmuch_database_t *notmuch,
-                        unsigned int doc_id);
+                        unsigned int doc_id,
+                        notmuch_private_status_t *status);
 
 notmuch_message_t *
 _notmuch_message_create_for_message_id (const void *talloc_owner,
                                        notmuch_database_t *notmuch,
-                                       const char *message_id);
+                                       const char *message_id,
+                                       notmuch_status_t *status);
 
 /* Lookup a prefix value by name.
  *
index 559ac3ab5b4ca90d260f21e8f7157b678ff74592..ef29fb57507ed6e6caae96468a761a0aef7dc650 100644 (file)
--- a/notmuch.h
+++ b/notmuch.h
@@ -51,6 +51,8 @@ typedef int notmuch_bool_t;
  *
  * NOTMUCH_STATUS_SUCCESS: No error occurred.
  *
+ * NOTMUCH_STATUS_OUT_OF_MEMORY: Out of memory
+ *
  * XXX: We don't really want to expose this lame XAPIAN_EXCEPTION
  * value. Instead we should map to things like DATABASE_LOCKED or
  * whatever.
@@ -78,6 +80,7 @@ typedef int notmuch_bool_t;
  */
 typedef enum _notmuch_status {
     NOTMUCH_STATUS_SUCCESS = 0,
+    NOTMUCH_STATUS_OUT_OF_MEMORY,
     NOTMUCH_STATUS_XAPIAN_EXCEPTION,
     NOTMUCH_STATUS_FILE_ERROR,
     NOTMUCH_STATUS_FILE_NOT_EMAIL,
@@ -267,8 +270,8 @@ notmuch_database_add_message (notmuch_database_t *database,
  * a new notmuch_message_t object is returned. The caller should call
  * notmuch_message_destroy when done with the message.
  *
- * If no message is found with the given message_id, this function
- * returns NULL.
+ * If no message is found with the given message_id or if an
+ * out-of-memory situation occurs, this function returns NULL.
  */
 notmuch_message_t *
 notmuch_database_find_message (notmuch_database_t *database,
@@ -376,6 +379,9 @@ notmuch_results_has_more (notmuch_results_t *results);
  *
  * See the documentation of notmuch_query_search for example code
  * showing how to iterate over a notmuch_results_t object.
+ *
+ * If an out-of-memory situation occurs, this function will return
+ * NULL.
  */
 notmuch_message_t *
 notmuch_results_get (notmuch_results_t *results);
index c68bd37ad7fdb4bab5cc6f35cffcda8792b70919..f5ee7aadc5ee7185037fce302c805ba1592c1f86 100644 (file)
--- a/query.cc
+++ b/query.cc
@@ -165,12 +165,24 @@ notmuch_results_has_more (notmuch_results_t *results)
 notmuch_message_t *
 notmuch_results_get (notmuch_results_t *results)
 {
+    notmuch_message_t *message;
     Xapian::docid doc_id;
+    notmuch_private_status_t status;
 
     doc_id = *results->iterator;
 
-    return _notmuch_message_create (results,
-                                   results->notmuch, doc_id);
+    message = _notmuch_message_create (results,
+                                      results->notmuch, doc_id,
+                                      &status);
+
+    if (message == NULL &&
+       status == NOTMUCH_PRIVATE_STATUS_NO_DOCUMENT_FOUND)
+    {
+       fprintf (stderr, "Internal error: a results iterator contains a non-existent document ID.\n");
+       exit (1);
+    }
+
+    return message;
 }
 
 void