Re: [Evolution-hackers] Memory leak in camel-imap-message-cache.c

2008-01-14 Thread Philip Van Hoof

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

2008-01-14 Thread Srinivasa Ragavan
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


Re: [Evolution-hackers] Memory leak in camel-imap-message-cache.c

2008-01-14 Thread Srinivasa Ragavan

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