On Wed, Feb 22 2017, David Bremner <da...@tethera.net> wrote: > The two g_hash_table functions (insert, add) have different behaviour > with respect to existing keys. g_hash_table_insert frees the new key, > while g_hash_table_add (which is really g_hash_table_replace in > disguise) frees the existing key. With this change 'ref' is live until > the end of the function (assuming single-threaded access to > 'hash'). We can't guarantee it will continue to be live in the > future (i.e. there may be a future key duplication) so we copy it with > the allocation context passed to parse_references (in practice this is > the notmuch_message_t object whose parents we are finding). > > Thanks to Tomi for the simpler approach to the problem based on > reading the fine glib manual. > ---
Great work! LGTM. Tomi PS: tried to look whethet there were talloc_ref() (the glib way)... there is "refcounting" but a bit different (unsuitable) way... > > this at least passes the --medium memory test. I'll run the full one > but it probably needs a day or so to complete. > > lib/database.cc | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/lib/database.cc b/lib/database.cc > index f0bfe566..eddb780c 100644 > --- a/lib/database.cc > +++ b/lib/database.cc > @@ -652,7 +652,7 @@ parse_references (void *ctx, > ref = _parse_message_id (ctx, refs, &refs); > > if (ref && strcmp (ref, message_id)) { > - g_hash_table_insert (hash, ref, NULL); > + g_hash_table_add (hash, ref); > last_ref = ref; > } > } > @@ -661,7 +661,7 @@ parse_references (void *ctx, > * reference to the database. We should avoid making a message > * its own parent, thus the above check. > */ > - return last_ref; > + return talloc_strdup(ctx, last_ref); > } > > notmuch_status_t > -- > 2.11.0 _______________________________________________ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch