From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by olra.theworths.org (Postfix) with ESMTP id 15344431FB6 for ; Thu, 30 Aug 2012 08:39:01 -0700 (PDT) X-Virus-Scanned: Debian amavisd-new at olra.theworths.org X-Spam-Flag: NO X-Spam-Score: -2.3 X-Spam-Level: X-Spam-Status: No, score=-2.3 tagged_above=-999 required=5 tests=[RCVD_IN_DNSWL_MED=-2.3] autolearn=disabled Received: from olra.theworths.org ([127.0.0.1]) by localhost (olra.theworths.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id ItPaG9fibirb for ; Thu, 30 Aug 2012 08:38:59 -0700 (PDT) Received: from max.feld.cvut.cz (max.feld.cvut.cz [147.32.192.36]) by olra.theworths.org (Postfix) with ESMTP id 331C6431FAE for ; Thu, 30 Aug 2012 08:38:59 -0700 (PDT) Received: from localhost (unknown [192.168.200.4]) by max.feld.cvut.cz (Postfix) with ESMTP id BB63B19F337A; Thu, 30 Aug 2012 17:38:54 +0200 (CEST) X-Virus-Scanned: IMAP AMAVIS Received: from max.feld.cvut.cz ([192.168.200.1]) by localhost (styx.feld.cvut.cz [192.168.200.4]) (amavisd-new, port 10044) with ESMTP id jCpFnc00AfAB; Thu, 30 Aug 2012 17:38:52 +0200 (CEST) Received: from imap.feld.cvut.cz (imap.feld.cvut.cz [147.32.192.34]) by max.feld.cvut.cz (Postfix) with ESMTP id 917DA19F32FF; Thu, 30 Aug 2012 17:38:52 +0200 (CEST) Received: from steelpick.2x.cz (note-sojka.felk.cvut.cz [147.32.86.30]) (Authenticated sender: sojkam1) by imap.feld.cvut.cz (Postfix) with ESMTPSA id 3B254660904; Thu, 30 Aug 2012 17:38:52 +0200 (CEST) Received: from wsh by steelpick.2x.cz with local (Exim 4.80) (envelope-from ) id 1T76pX-0004f8-Vq; Thu, 30 Aug 2012 17:38:51 +0200 From: Michal Sojka To: Jameson Graef Rollins , Notmuch Mail Subject: Re: [PATCH 01/11] lib: new thread addresses structure In-Reply-To: <1345427570-26518-2-git-send-email-jrollins@finestructure.net> References: <1345427570-26518-1-git-send-email-jrollins@finestructure.net> <1345427570-26518-2-git-send-email-jrollins@finestructure.net> User-Agent: Notmuch/0.14+3~g608c52f (http://notmuchmail.org) Emacs/24.1.1 (x86_64-pc-linux-gnu) Date: Thu, 30 Aug 2012 17:38:51 +0200 Message-ID: <87y5kw4cfo.fsf@steelpick.2x.cz> MIME-Version: 1.0 Content-Type: text/plain X-BeenThere: notmuch@notmuchmail.org X-Mailman-Version: 2.1.13 Precedence: list List-Id: "Use and development of the notmuch mail system." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 30 Aug 2012 15:39:01 -0000 Hi Jameson, some comments below. On Mon, Aug 20 2012, Jameson Graef Rollins wrote: > This new structure holds addresses associated with a thread, both > matched and unmatched. Initially this will be used to replace the > existing infrastructure for storing the addresses of thread authors. > Further patches will use it to store the addresses of threads > recipients. > > Init and destructor functions are included, as well as a function to > add addresses to a struct, either "matched" or not. > --- > lib/notmuch.h | 1 + > lib/thread.cc | 116 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 117 insertions(+) > > diff --git a/lib/notmuch.h b/lib/notmuch.h > index 3633bed..6acd38d 100644 > --- a/lib/notmuch.h > +++ b/lib/notmuch.h > @@ -118,6 +118,7 @@ typedef struct _notmuch_database notmuch_database_t; > typedef struct _notmuch_query notmuch_query_t; > typedef struct _notmuch_threads notmuch_threads_t; > typedef struct _notmuch_thread notmuch_thread_t; > +typedef struct _notmuch_thread_addresses notmuch_thread_addresses_t; > typedef struct _notmuch_messages notmuch_messages_t; > typedef struct _notmuch_message notmuch_message_t; > typedef struct _notmuch_tags notmuch_tags_t; > diff --git a/lib/thread.cc b/lib/thread.cc > index e976d64..7af9eeb 100644 > --- a/lib/thread.cc > +++ b/lib/thread.cc > @@ -24,6 +24,14 @@ > #include > #include /* GHashTable */ > > +struct visible _notmuch_thread_addresses { > + GHashTable *matched_hash; > + GPtrArray *matched_array; > + GHashTable *unmatched_hash; > + GPtrArray *unmatched_array; > + char *string; > +}; > + > struct visible _notmuch_thread { > notmuch_database_t *notmuch; > char *thread_id; > @@ -44,6 +52,18 @@ struct visible _notmuch_thread { > }; > > static int > +_notmuch_thread_addresses_destructor (notmuch_thread_addresses_t *addresses) > +{ > + g_hash_table_unref (addresses->matched_hash); > + g_hash_table_unref (addresses->unmatched_hash); > + g_ptr_array_free (addresses->matched_array, TRUE); > + g_ptr_array_free (addresses->unmatched_array, TRUE); > + addresses->matched_array = NULL; > + addresses->unmatched_array = NULL; > + return 0; > +} > + > +static int > _notmuch_thread_destructor (notmuch_thread_t *thread) > { > g_hash_table_unref (thread->authors_hash); > @@ -64,6 +84,81 @@ _notmuch_thread_destructor (notmuch_thread_t *thread) > return 0; > } > > +/* Add address to a thread addresses struct. If matched is TRUE, then > + * the address will be added to the matched list.*/ > +static void > +_thread_add_address (notmuch_thread_addresses_t *addresses, > + const char *address, > + notmuch_bool_t matched) > +{ > + char *address_copy; > + GHashTable *hash; > + GPtrArray *array; > + > + if (matched) { > + hash = addresses->matched_hash; > + array = addresses->matched_array; > + } else { > + hash = addresses->unmatched_hash; > + array = addresses->unmatched_array; > + } > + > + if (address == NULL) > + return; > + > + if (g_hash_table_lookup_extended (hash, address, NULL, NULL)) > + return; > + > + address_copy = talloc_strdup (addresses, address); > + > + g_hash_table_insert (hash, address_copy, NULL); > + > + g_ptr_array_add (array, address_copy); > +} > + > +/* Construct an addresses string from matched and unmatched addresses > + * in notmuch_thread_addresses_t. The string contains matched > + * addresses first, then non-matched addresses (with the two groups > + * separated by '|'). Within each group, addresses are listed in date > + * order. */ I'd say the the addresses are listed in the order in which they have been added, which might or might not be the date order. > +static void > +_resolve_thread_addresses_string (notmuch_thread_addresses_t *addresses) > +{ > + unsigned int i; > + char *address; > + int first_non_matched_address = 1; > + > + /* First, list all matched addressses in date order. */ > + for (i = 0; i < addresses->matched_array->len; i++) { > + address = (char *) g_ptr_array_index (addresses->matched_array, i); > + if (addresses->string) > + addresses->string = talloc_asprintf (addresses, "%s, %s", > + addresses->string, > + address); > + else > + addresses->string = address; > + } > + > + /* Next, append any non-matched addresses that haven't already appeared. */ > + for (i = 0; i < addresses->unmatched_array->len; i++) { > + address = (char *) g_ptr_array_index (addresses->unmatched_array, i); > + if (g_hash_table_lookup_extended (addresses->matched_hash, > + address, NULL, NULL)) > + continue; > + if (first_non_matched_address) { > + addresses->string = talloc_asprintf (addresses, "%s| %s", > + addresses->string, > + address); > + } else { > + addresses->string = talloc_asprintf (addresses, "%s, %s", > + addresses->string, > + address); > + } Simpler would be: addresses->string = talloc_asprintf (addresses, "%s%c %s", addresses->string, first_non_matched_address ? '|' : ',' address); Also, you might want to talloc_free the old address->string to not waste memory in the case of long lived notmuch_thread_addresses_t object. Or better use talloc_asprintf_append() function, which hopefully implements freeing internally. -Michal > + > + first_non_matched_address = 0; > + } > +} > + > /* Add each author of the thread to the thread's authors_hash and to > * the thread's authors_array. */ > static void > @@ -382,6 +477,27 @@ _resolve_thread_relationships (unused (notmuch_thread_t *thread)) > */ > } > > +/* Initialize a thread addresses struct. */ > +notmuch_thread_addresses_t * > +_thread_addresses_init (const void *ctx) > +{ > + notmuch_thread_addresses_t *addresses; > + > + addresses = talloc (ctx, notmuch_thread_addresses_t); > + if (unlikely (addresses == NULL)) > + return NULL; > + > + addresses->matched_hash = g_hash_table_new_full (g_str_hash, g_str_equal, > + NULL, NULL); > + addresses->matched_array = g_ptr_array_new (); > + addresses->unmatched_hash = g_hash_table_new_full (g_str_hash, g_str_equal, > + NULL, NULL); > + addresses->unmatched_array = g_ptr_array_new (); > + addresses->string = NULL; > + > + return addresses; > +} > + > /* Create a new notmuch_thread_t object by finding the thread > * containing the message with the given doc ID, treating any messages > * contained in match_set as "matched". Remove all messages in the > -- > 1.7.10.4