From: Michal Sojka Date: Thu, 30 Aug 2012 15:38:51 +0000 (+0200) Subject: Re: [PATCH 01/11] lib: new thread addresses structure X-Git-Url: http://git.tremily.us/gitweb.cgi?a=commitdiff_plain;h=71345a58548872c9e1c1c6cb074da84e87cc6a75;p=notmuch-archives.git Re: [PATCH 01/11] lib: new thread addresses structure --- diff --git a/83/1a650e839e34b62d399147e4cf086760725cce b/83/1a650e839e34b62d399147e4cf086760725cce new file mode 100644 index 000000000..550dbf31e --- /dev/null +++ b/83/1a650e839e34b62d399147e4cf086760725cce @@ -0,0 +1,263 @@ +Return-Path: +X-Original-To: notmuch@notmuchmail.org +Delivered-To: notmuch@notmuchmail.org +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