Re: [Evolution-hackers] Memory leak in camel-imap-message-cache.c
On Mon, 2008-01-14 at 11:45 +0100, Philip Van Hoof wrote: > On Mon, 2008-01-14 at 16:02 +0530, Srinivasa Ragavan wrote: > > Look at the free_part function being called at > > g_hash_table_foreach (cache->parts, free_part, cache); > > > > It frees the key. > > Aha. Okay thanks for the clarification. > > I wonder, wouldn't it be better to use the standard infrastructure of > GHashTable to clearup keys? The full version of the GHashTable's > constructor has a callback function pointer parameter that'll be used > for freeing up keys like this. I think it needs to be done that way. But just a clean up work IMO. > I of course also wonder howcome my memory analysis tool pointed me to > this memory leak. Although it might be explained by one CamelMessage > reference still being open when the exit(0) took place. > I use totalview to analyze memory. I dont see that from it. May be at some specific scenarios. -Srini. ___ Evolution-hackers mailing list Evolution-hackers@gnome.org http://mail.gnome.org/mailman/listinfo/evolution-hackers
Re: [Evolution-hackers] Memory leak in camel-imap-message-cache.c
On Mon, 2008-01-14 at 16:02 +0530, Srinivasa Ragavan wrote: > Look at the free_part function being called at > g_hash_table_foreach (cache->parts, free_part, cache); > > It frees the key. Aha. Okay thanks for the clarification. I wonder, wouldn't it be better to use the standard infrastructure of GHashTable to clearup keys? The full version of the GHashTable's constructor has a callback function pointer parameter that'll be used for freeing up keys like this. I of course also wonder howcome my memory analysis tool pointed me to this memory leak. Although it might be explained by one CamelMessage reference still being open when the exit(0) took place. > On Mon, 2008-01-14 at 02:58 +0100, Philip Van Hoof wrote: > > Hi there, > > > > The cache->parts = g_hash_table_new (g_str_hash, g_str_equal); of > > camel-imap-message-cache.c does not free its keys (it's the default way > > of creating a hashtable, so there's no freeup function provided for the > > keys). > > > > Yet at (or around) line 118 we see this: > > > > g_hash_table_insert (cache->parts, g_strdup (uid), subparts); > > > > That uid is string-copied. So either the hashtable needs a freeup > > function or the string should not be copied or .. this is wrong. > > > > Because I don't know how important the string copying is, what do you > > guys think we should do here? > > > > > -- Philip Van Hoof, freelance software developer home: me at pvanhoof dot be gnome: pvanhoof at gnome dot org http://pvanhoof.be/blog http://codeminded.be ___ Evolution-hackers mailing list Evolution-hackers@gnome.org http://mail.gnome.org/mailman/listinfo/evolution-hackers
Re: [Evolution-hackers] Memory leak in camel-imap-message-cache.c
Look at the free_part function being called at g_hash_table_foreach (cache->parts, free_part, cache); It frees the key. -Srini. On Mon, 2008-01-14 at 02:58 +0100, Philip Van Hoof wrote: > Hi there, > > The cache->parts = g_hash_table_new (g_str_hash, g_str_equal); of > camel-imap-message-cache.c does not free its keys (it's the default way > of creating a hashtable, so there's no freeup function provided for the > keys). > > Yet at (or around) line 118 we see this: > > g_hash_table_insert (cache->parts, g_strdup (uid), subparts); > > That uid is string-copied. So either the hashtable needs a freeup > function or the string should not be copied or .. this is wrong. > > Because I don't know how important the string copying is, what do you > guys think we should do here? > > ___ Evolution-hackers mailing list Evolution-hackers@gnome.org http://mail.gnome.org/mailman/listinfo/evolution-hackers
