]> git.cworth.org Git - notmuch/commitdiff
Merge branch from fixing up bugs after bisecting.
authorCarl Worth <cworth@cworth.org>
Thu, 22 Oct 2009 06:23:32 +0000 (23:23 -0700)
committerCarl Worth <cworth@cworth.org>
Thu, 22 Oct 2009 06:23:44 +0000 (23:23 -0700)
I'm glad that when I implemented "notmuch restore" I went through the
extra effort to take the code I had written in one sitting into over a
dozen commits. Sure enough, I hadn't tested well enough and had
totally broken "notmuch setup", (segfaults and bogus thread_id
values).

With the little commits I had made, git bisect saved the day, and I
went back to make the fixes right on top of the commits that
introduced the bugs. So now we octopus merge those in.

1  2  3 
database.cc
message.cc

diff --combined database.cc
index 5049b47e9ef593d44b586bc67d37064f79dc0a84,00b03731206289b171736dd8f9c4044a67f8f0a8,77b2eff22973073aa511c2d0c0baa31d20190e31..6d62109e73c11e758987c5203839e2ef91aa8291
   
   using namespace std;
   
 ++const char *
 ++notmuch_status_to_string (notmuch_status_t status)
 ++{
 ++    switch (status) {
 ++    case NOTMUCH_STATUS_SUCCESS:
 ++     return "No error occurred";
 ++    case NOTMUCH_STATUS_XAPIAN_EXCEPTION:
 ++     return "A Xapian exception occurred";
 ++    case NOTMUCH_STATUS_FILE_NOT_EMAIL:
 ++     return "File is not an email";
 ++    case NOTMUCH_STATUS_NULL_POINTER:
 ++     return "Erroneous NULL pointer";
 ++    case NOTMUCH_STATUS_TAG_TOO_LONG:
 ++     return "Tag value is too long";
 ++    default:
 ++    case NOTMUCH_STATUS_LAST_STATUS:
 ++     return "Unknown error status value";
 ++    }
 ++}
 ++
   /* "128 bits of thread-id ought to be enough for anybody" */
   #define NOTMUCH_THREAD_ID_BITS        128
   #define NOTMUCH_THREAD_ID_DIGITS (NOTMUCH_THREAD_ID_BITS / 4)
@@@@ -84,8 -64,6 -64,6 +84,8 @@@@ thread_id_generate (thread_id_t *thread
       }
   }
   
 ++/* XXX: We should drop this function and convert all callers to call
 ++ * _notmuch_message_add_term instead. */
   static void
   add_term (Xapian::Document doc,
          const char *prefix_name,
@@@@ -133,21 -111,42 -111,44 +133,44 @@@@ find_message_by_docid (Xapian::Databas
       return db->get_document (docid);
   }
   
 - Xapian::Document
 - find_message_by_message_id (Xapian::Database *db, const char *message_id)
 - {
 -     Xapian::PostingIterator i, end;
 - 
 -     find_messages_by_term (db, "msgid", message_id, &i, &end);
 - 
 -     if (i != end)
 -      return find_message_by_docid (db, *i);
 -     else
 -      return Xapian::Document ();
 - }
 - 
+  static void
+  insert_thread_id (GHashTable *thread_ids, Xapian::Document doc)
+  {
+      string value_string;
+      const char *value, *id, *comma;
+  
+      value_string = doc.get_value (NOTMUCH_VALUE_THREAD);
+      value = value_string.c_str();
+      if (strlen (value)) {
+       id = value;
+       while (*id) {
+           comma = strchr (id, ',');
+           if (comma == NULL)
+               comma = id + strlen (id);
+           g_hash_table_insert (thread_ids,
+                                strndup (id, comma - id), NULL);
+           id = comma;
+           if (*id)
+               id++;
+       }
+      }
+  }
+  
 + notmuch_message_t *
 + notmuch_database_find_message (notmuch_database_t *notmuch,
 +                             const char *message_id)
 + {
 +     Xapian::PostingIterator i, end;
 + 
 +     find_messages_by_term (notmuch->xapian_db,
 +                         "msgid", message_id, &i, &end);
 + 
 +     if (i == end)
 +      return NULL;
 + 
 +     return _notmuch_message_create (notmuch, notmuch, *i);
 + }
 + 
   /* Return one or more thread_ids, (as a GPtrArray of strings), for the
    * given message based on looking into the database for any messages
    * referenced in parents, and also for any messages in the database
    * Caller should free all strings in the array and the array itself,
    * (g_ptr_array_free) when done. */
   static GPtrArray *
 - find_thread_ids (Xapian::Database *db,
 + find_thread_ids (notmuch_database_t *notmuch,
                 GPtrArray *parents,
                 const char *message_id)
   {
 +     Xapian::WritableDatabase *db = notmuch->xapian_db;
       Xapian::PostingIterator child, children_end;
       Xapian::Document doc;
       GHashTable *thread_ids;
   
       find_messages_by_term (db, "ref", message_id, &child, &children_end);
       for ( ; child != children_end; child++) {
-       const char *thread_id;
        doc = find_message_by_docid (db, *child);
-  
-       thread_id = doc.get_value (NOTMUCH_VALUE_THREAD).c_str ();
-       if (strlen (thread_id) == 0) {
-           fprintf (stderr, "Database error: Message with doc_id %u has empty thread-id value (value index %d)\n",
-                    *child, NOTMUCH_VALUE_THREAD);
-       } else {
-           g_hash_table_insert (thread_ids, strdup (thread_id), NULL);
-       }
+       insert_thread_id (thread_ids, doc);
       }
   
       for (i = 0; i < parents->len; i++) {
 +      notmuch_message_t *parent;
 +      notmuch_thread_ids_t *ids;
 + 
        parent_message_id = (char *) g_ptr_array_index (parents, i);
 -      doc = find_message_by_message_id (db, parent_message_id);
 -      insert_thread_id (thread_ids, doc);
 +      parent = notmuch_database_find_message (notmuch, parent_message_id);
 +      if (parent == NULL)
 +          continue;
 + 
 +      for (ids = notmuch_message_get_thread_ids (parent);
 +           notmuch_thread_ids_has_more (ids);
 +           notmuch_thread_ids_advance (ids))
 +      {
 +          const char *id;
 + 
 +          id = notmuch_thread_ids_get (ids);
 +          g_hash_table_insert (thread_ids, strdup (id), NULL);
 +      }
 + 
 +      notmuch_message_destroy (parent);
       }
   
       result = g_ptr_array_new ();
@@@@ -523,7 -497,7 -516,7 +538,7 @@@@ notmuch_database_add_message (notmuch_d
            message_id = NULL;
        }
   
 -      thread_ids = find_thread_ids (db, parents, message_id);
 +      thread_ids = find_thread_ids (notmuch, parents, message_id);
   
        for (i = 0; i < parents->len; i++)
            g_free (g_ptr_array_index (parents, i));
diff --combined message.cc
index 338d953f2dfb744f8b98c677ab1cf5c711a1f823,24dbae91b17c962640d59c400de05cbe8122d48a,0efa470adcf149071ad8943f4619f9d28d4236d3..dd73d13c97fe06332850798f24de7d2012bf80cb
   #include <xapian.h>
   
   struct _notmuch_message {
 ++    notmuch_database_t *notmuch;
 ++    Xapian::docid doc_id;
 ++    char *message_id;
       Xapian::Document doc;
   };
   
@@@@ -69,7 -66,6 -66,6 +69,7 @@@@ prefix_t BOOLEAN_PREFIX[] = 
       { "email", "E" },
       { "date", "D" },
       { "label", "L" },
 ++    { "tag", "L" },
       { "source_id", "I" },
       { "attachment_extension", "O" },
       { "msgid", "Q" },
@@@@ -108,19 -104,16 -104,16 +108,19 @@@@ _notmuch_message_destructor (notmuch_me
   }
   
   notmuch_message_t *
 - _notmuch_message_create (notmuch_results_t *owner,
 + _notmuch_message_create (const void *talloc_owner,
                         notmuch_database_t *notmuch,
 -                       Xapian::docid doc_id)
 +                       unsigned int doc_id)
   {
       notmuch_message_t *message;
   
 -     message = talloc (owner, notmuch_message_t);
 +     message = talloc (talloc_owner, notmuch_message_t);
       if (unlikely (message == NULL))
        return NULL;
   
 ++    message->notmuch = notmuch;
 ++    message->doc_id = doc_id;
 ++    message->message_id = NULL; /* lazily created */
       new (&message->doc) Xapian::Document;
   
       talloc_set_destructor (message, _notmuch_message_destructor);
@@@@ -135,19 -128,12 -128,12 +135,19 @@@@ notmuch_message_get_message_id (notmuch
   {
       Xapian::TermIterator i;
   
 ++    if (message->message_id)
 ++     return message->message_id;
 ++
       i = message->doc.termlist_begin ();
 --    i.skip_to ("Q");
 --    if (i != message->doc.termlist_end ())
 --     return talloc_strdup (message, (*i).c_str () + 1);
 --    else
 ++    i.skip_to (_find_prefix ("msgid"));
 ++
 ++    /* XXX: This should really be an internal error, but we'll need to
 ++     * fix the add_message side of things first. */
 ++    if (i == message->doc.termlist_end ())
        return NULL;
 ++
 ++    message->message_id = talloc_strdup (message, (*i).c_str () + 1);
 ++    return message->message_id;
   }
   
   /* We end up having to call the destructors explicitly because we had
@@@@ -180,7 -166,7 -166,7 +180,7 @@@@ notmuch_message_get_tags (notmuch_messa
       talloc_set_destructor (tags, _notmuch_tags_destructor);
   
       tags->iterator = message->doc.termlist_begin ();
 --    tags->iterator.skip_to ("L");
 ++    tags->iterator.skip_to (_find_prefix ("tag"));
       tags->iterator_end = message->doc.termlist_end ();
   
       return tags;
@@@@ -190,14 -176,14 -176,14 +190,14 @@@@ notmuch_thread_ids_t 
   notmuch_message_get_thread_ids (notmuch_message_t *message)
   {
       notmuch_thread_ids_t *thread_ids;
- -    const char *id_str;
+ +    std::string id_str;
   
       thread_ids = talloc (message, notmuch_thread_ids_t);
       if (unlikely (thread_ids == NULL))
        return NULL;
   
- -    id_str = message->doc.get_value (NOTMUCH_VALUE_THREAD).c_str ();
- -    thread_ids->next = talloc_strdup (message, id_str);
+ +    id_str = message->doc.get_value (NOTMUCH_VALUE_THREAD);
+ +    thread_ids->next = talloc_strdup (message, id_str.c_str ());
   
       /* Initialize thread_ids->current and terminate first ID. */
       notmuch_thread_ids_advance (thread_ids);
       return thread_ids;
   }
   
 ++/* Synchronize changes made to message->doc into the database. */
 ++static void
 ++_notmuch_message_sync (notmuch_message_t *message)
 ++{
 ++    Xapian::WritableDatabase *db = message->notmuch->xapian_db;
 ++
 ++    db->replace_document (message->doc_id, message->doc);
 ++}
 ++
 ++notmuch_private_status_t
 ++_notmuch_message_add_term (notmuch_message_t *message,
 ++                        const char *prefix_name,
 ++                        const char *value)
 ++{
 ++
 ++    char *term;
 ++
 ++    if (value == NULL)
 ++     return NOTMUCH_PRIVATE_STATUS_NULL_POINTER;
 ++
 ++    term = talloc_asprintf (message, "%s%s",
 ++                         _find_prefix (prefix_name), value);
 ++
 ++    if (strlen (term) > NOTMUCH_TERM_MAX)
 ++     return NOTMUCH_PRIVATE_STATUS_TERM_TOO_LONG;
 ++
 ++    message->doc.add_term (term);
 ++    _notmuch_message_sync (message);
 ++
 ++    talloc_free (term);
 ++
 ++    return NOTMUCH_PRIVATE_STATUS_SUCCESS;
 ++}
 ++
 ++notmuch_private_status_t
 ++_notmuch_message_remove_term (notmuch_message_t *message,
 ++                           const char *prefix_name,
 ++                           const char *value)
 ++{
 ++    char *term;
 ++
 ++    if (value == NULL)
 ++     return NOTMUCH_PRIVATE_STATUS_NULL_POINTER;
 ++
 ++    term = talloc_asprintf (message, "%s%s",
 ++                         _find_prefix (prefix_name), value);
 ++
 ++    if (strlen (term) > NOTMUCH_TERM_MAX)
 ++     return NOTMUCH_PRIVATE_STATUS_TERM_TOO_LONG;
 ++
 ++    message->doc.remove_term (term);
 ++    _notmuch_message_sync (message);
 ++
 ++    talloc_free (term);
 ++
 ++    return NOTMUCH_PRIVATE_STATUS_SUCCESS;
 ++}
 ++
 ++notmuch_status_t
 ++notmuch_message_add_tag (notmuch_message_t *message, const char *tag)
 ++{
 ++    notmuch_private_status_t status;
 ++
 ++    if (tag == NULL)
 ++     return NOTMUCH_STATUS_NULL_POINTER;
 ++
 ++    if (strlen (tag) > NOTMUCH_TAG_MAX)
 ++     return NOTMUCH_STATUS_TAG_TOO_LONG;
 ++
 ++    status = _notmuch_message_add_term (message, "tag", tag);
 ++    if (status) {
 ++     fprintf (stderr, "Internal error: _notmuch_message_add_term return unexpected value: %d\n",
 ++              status);
 ++     exit (1);
 ++    }
 ++
 ++    return NOTMUCH_STATUS_SUCCESS;
 ++}
 ++
 ++notmuch_status_t
 ++notmuch_message_remove_tag (notmuch_message_t *message, const char *tag)
 ++{
 ++    notmuch_private_status_t status;
 ++
 ++    if (tag == NULL)
 ++     return NOTMUCH_STATUS_NULL_POINTER;
 ++
 ++    if (strlen (tag) > NOTMUCH_TAG_MAX)
 ++     return NOTMUCH_STATUS_TAG_TOO_LONG;
 ++
 ++    status = _notmuch_message_remove_term (message, "tag", tag);
 ++    if (status) {
 ++     fprintf (stderr, "Internal error: _notmuch_message_remove_term return unexpected value: %d\n",
 ++              status);
 ++     exit (1);
 ++    }
 ++
 ++    return NOTMUCH_STATUS_SUCCESS;
 ++}
 ++
   void
   notmuch_message_destroy (notmuch_message_t *message)
   {