David Bremner <david@tethera.net> writes: > Tomi Ollila <tomi.ollila@iki.fi> writes: > >> To me it looks like replacing g_hash_table_insert() with >> g_hash_table_replace() would do the trick. >> >> (or even g_hash_table_add()!) >> >> One has to read the documentation a bit (and compare the docstrings of >> these 2 functions to guess the missing pieces) to get some understanding to >> this... >> > > Hi Tomi; > > Thanks for the suggestion. Unfortunately in my experiments it just > shifts the invalid memory access to a different piece of memory. I think > the problem is that a pointer to the previous copy of that key also > leaked a reference via last_ref, so when we kill that via > g_hash_table_replace it causes the same problem. > Thinking a bit more about it, at least in this case the pointer that was just inserted remains valid after insertion, and can be talloc_strdup'ed, i.e. 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 I'll let valgrind chew on that for a bit and see what it says.