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] Possible memory leak

2008-01-14 Thread Philip Van Hoof

I think I've found the leak ...

g_slist_remove_link:

Removes an element from a GSList, without freeing the element. The
removed element's next link is set to NULL, so that it becomes a
self-contained list with one element.

Notice the without freeing the element

Yet 

void
camel_operation_end (CamelOperation *cc)
{
...

g_free(s);
cc-status_stack = g_slist_remove_link(cc-status_stack,
 cc-status_stack);
UNLOCK();

if (msg) {
cc-status(cc, msg, sofar, oftotal, cc-status_data);
g_free(msg);
}
}

I think this needs to be something like this:

GSList *item = cc-status_stack;
cc-status_stack = g_slist_remove_link(cc-status_stack, item);
g_slist_free (item);


Can somebody with GSList know-how acknowledge this?



On Mon, 2008-01-14 at 02:28 +0100, Philip Van Hoof wrote:
 On Mon, 2008-01-14 at 01:47 +0100, Philip Van Hoof wrote:
 
  I have this memory analysis tool that I'm willing to believe that tells
  me this line in camel-folder.c causes 381 times a leaked allocation of
  in total 5.95 kb. (I opened one folder of 1000 items). It seems to be
  the g_slice_alloc (which does a malloc since I disabled gslice) of the
  g_hash_node_new.
 
 I found it, this was my own mistake in camel-lite's adaptations.
 
 I have found another leak in camel-operation.c:
 
 The cc-status_stack = g_slist_prepend(cc-status_stack, s); in
 camel_operation_start seems to be leaked each time it's called.
 
 I have 23 calls leaking 184 bytes. Various callers of
 camel_operation_start (all of them, it seems) seem to create this leak.
 
 My version of camel-operation.c adds two fields (a total and a current
 position in stead of a percentage, because my requirements where to give
 accurate to-the-byte progress info, not rounded percentages). Those
 changes are closely reviewed once more and don't seem to be causing this
 leak.
 
 ps. I attached the differences that I have. I basically replaced pc
 with a sofar and a oftotal.
 
 
 ___
 Evolution-hackers mailing list
 Evolution-hackers@gnome.org
 http://mail.gnome.org/mailman/listinfo/evolution-hackers
-- 
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] Possible memory leak

2008-01-14 Thread Matthew Barnes
On Mon, 2008-01-14 at 14:33 +0100, Philip Van Hoof wrote:
 I think I've found the leak ...
 
 g_slist_remove_link:
 
 Removes an element from a GSList, without freeing the element. The
 removed element's next link is set to NULL, so that it becomes a
 self-contained list with one element.
 
 Notice the without freeing the element
 
 Yet 
 
 void
 camel_operation_end (CamelOperation *cc)
 {
   ...
 
   g_free(s);
   cc-status_stack = g_slist_remove_link(cc-status_stack,
cc-status_stack);
   UNLOCK();
 
   if (msg) {
   cc-status(cc, msg, sofar, oftotal, cc-status_data);
   g_free(msg);
   }
 }


g_slist_delete_link() frees the GSList node, whereas
g_slist_remove_link() just disconnects the node from the list.

If I'm reading this right, the preceding logic frees the list node
contents, so I think we just need to use g_slist_delete_link() in place
of g_slist_remove_link().  One line change.

void
camel_operation_end (CamelOperation *cc)
{
...
s = cc-status_stack-data;
...
g_free (s);
cc-status_stack = g_slist_delete_link (
cc-status_stack, cc-status_stack);
...
}

___
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


[Evolution-hackers] Evolution 2.21.5 , Evolution-Data-Server 2.21.5 , GtkHTML3.17.5 and Evolution-Exchange 2.21.5 released

2008-01-14 Thread Srinivasa Ragavan
Hi All,

The Evolution Team is pleased to announce the release of Evolution
2.21.5. 

You can download the following :

http://ftp.acc.umu.se/pub/gnome/sources/gtkhtml/3.17/gtkhtml-3.17.5.tar.bz2
http://ftp.acc.umu.se/pub/gnome/sources/evolution-data-server/2.21/evolution-data-server-2.21.5.tar.bz2
http://ftp.acc.umu.se/pub/gnome/sources/evolution/2.21/evolution-2.21.5.tar.bz2
http://ftp.acc.umu.se/pub/gnome/sources/evolution-exchange/2.21/evolution-exchange-2.21.5.tar.bz2

Upgrade Notes :
Evolution 2.21.x is the unstable series of 2.22 development.
(Note: Evolution/EDS/Exchange versions are synced with the GNOME
versions)

RELNOTE: If you have created labels/tags using Evolution 2.21.4. You
need to delete them and recreate using 2.21.5. We have changes the way
labels are stored now. It should be final now.

What is New ?
=

Evolution:
==
New in 2.21.5:
Mail errors are now non-intrusive (Srinivasa Ragavan)
RTL fixes - Mail and Addressbook (Djihed Afifi)
Message Tagging (aka custom labels) Improvements (Milan Crha)

Bug fixes:
#211353: Allow categories to be assigned to emails (Milan Crha)
#219197: Quit application when session dies (Milan Crha)
#270605: Skip disabled accounts. (Suman Manjunath)
#300336: Added checkbox for Enable Search Folders option (Milan Crha)
#300336: Ensure vfolder is running. (Milan Crha)
#309432: Fix message headers for RTL languages. (Djihed Afifi)
#317823: Bump GtkHTML requirement to 3.17.5 (Matthew Barnes)
#317823: Save inline pictures embedded into HTML Mails (Milan Crha)
#327965: Fixed multiple password prompts in an exchange account (Sushma 
Rai)
#329692: Get the content size of the MIME part (Jean-Christophe BEGUE)
#333695: Print attenddes/roles in the print view (Milan Crha)
#339813: Setting new option 'e_date_edit_set_twodigit_year_can_future' 
to FALSE (Milan Crha)
#348638: Remove pre-edit buffer cleanly in day view (Mayank Jain)
#350932: Enable the use of scrollable tabs in the mail-preferences 
dialog. (Gert Kulyk)
#362638: Overhaul the message passing API (Matthew Barnes)
#364642: New option in Composer tab to preset Request Read Receipt in 
composer (Milan Crha)
#375580: Use ISO-8859-1 encoding to store contacts in iPod (João Vale)
#448441: Disable OK and Edit Full buttons if no source is selected. 
Also set always book from combo, do not use the new created (Milan Crha)
#457842: Do not call edit/start editing of the event when double 
clicked on the same component as is actually editing (Milan Crha)
#474118: Check for the right type of store and invoke appropriate 
functions (Bharath Acharya)
#476264: Add mnemonic_widget for default junk plugin (Andre Klapper)
#488213: Fix locks when displaying e-mail with large tiff drawing 
attached (Milan Crha)
#492188: Use the new Tangoized icons instead of deprecated icons from 
gnome-icon-theme. (Michael Monreal)
#492702: Just disable the dbus message part of mail notification if 
dbus isn't there. Also remove new-mail-notify plugin (Srinivasa Ragavan)
#496301: Clean up the schema (Martin Meyer)
#496402: Do not synchronize blocked bodies from pidgin (Milan Crha)
#497914: backport changes from the copy/pasted code in imap-headers 
plugin (Gilles Dartiguelongue)
#498095: Fix mnemonic issues (David Turner)
#499145: Follow RFC 3798 to send return receipts. (Colin Leroy)
#502303: Plugins configure widgets are not freed correctly in 
plugin-mamager (Milan Crha)
#502783: Restore message states (read receipt/priority) when opening 
from a draft ([EMAIL PROTECTED])
#502914: Do not write NULL into gconf (Milan Crha)
#503954: Accept custom domain names while setting up account (Nyall 
Dawson)
#504480: Increases the height of the ETaskBar to eliminate the constant 
resizing (Milan Crha)
#504541: Add --with[out]-help option to make it possible to skip 
building and installing user documentation (Matthew Barnes)
#506772: Not-NULL check for a string array before finding its length 
(Christian Krause)
#506814: Add the signal only if the view is present (Srinivasa Ragavan)
#507067: Can't save pictures embedded in HTML Mail when picture = link 
(Milan Crha)
#507311: Submit bugs to the BugBuddyBugs Bugzilla component (Matthew 
Barnes)
#507359: 
#507363: Also check if toplevel widget has non-NULL window property 
(Milan Crha)
#508282: Mark the window title for translation. (Changwoo Ryu)
#508678: Included missing header glib/gi18n.h (Bharath Acharya)
#508731: Have a safe default, if the values from gconf isn't so nice. 
(Srinivasa Ragavan)

Updated Translations:
Amitakhya Phukan (as)
Espen Stefansen (nb)
Jorge Gonzalez (es)