After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 651433 - Possible memory leak in e_book_backend_file_get_contact_list()
Possible memory leak in e_book_backend_file_get_contact_list()
Status: RESOLVED FIXED
Product: evolution-data-server
Classification: Platform
Component: Contacts
2.32.x (obsolete)
Other Linux
: Normal normal
: ---
Assigned To: evolution-addressbook-maintainers
Evolution QA team
Depends on:
Blocks:
 
 
Reported: 2011-05-30 07:19 UTC by Christophe Dumez
Modified: 2011-06-07 08:59 UTC
See Also:
GNOME target: ---
GNOME version: ---

Attachments
Proposed patch (1.12 KB, patch)
2011-05-30 07:19 UTC, Christophe Dumez 		needs-work Details | Review
Proposed patch (using free) (1.12 KB, patch)
2011-05-31 13:34 UTC, Christophe Dumez 		accepted-commit_now Details | Review
Patch for master (1.12 KB, patch)
2011-06-06 13:52 UTC, Christophe Dumez 		committed Details | Review

Description Christophe Dumez 2011-05-30 07:19:09 UTC 
Created attachment 188858 [details] [review]
Proposed patch

I believe I found a possible memory leak in e_book_backend_file_get_contact_list().

db->c_get() allocates memory in vcard_dbt.data but this memory is never freed in the case where it is not appended to contact_list.

Please see attached patch.
Comment 1 Matthew Barnes 2011-05-31 13:30:50 UTC 
It does appear to be leaking memory, but I think you want to use free() instead of g_free() since the memory was allocated by the DB, not GLib.  free() and g_free() are not necessarily symonymous.
Comment 2 Christophe Dumez 2011-05-31 13:34:03 UTC 
Created attachment 188937 [details] [review]
Proposed patch (using free)

Use free instead of g_free.
Comment 3 Matthew Barnes 2011-05-31 14:14:56 UTC 
Looks good.  Commit to master and backport as far back as you need.


As an aside, it occurs to me these DB vs GLib memory allocation mismatches are likely widespread in the file backend.  For example, I bet the DB-allocated data getting added to the contact_list GList is getting freed elsewhere with g_free().  Everything is fine as long as g_mem_is_system_malloc() is TRUE, meaning DB and GLib's allocations are compatible.  But if something calls g_mem_set_vtable() with a different vtable, we're in for trouble.

Obviously this is a separate issue and exceeds the scope of this bug, but since you're already hunting for memory issues I thought I'd mention it.  Something to look out for.
Comment 4 Christophe Dumez 2011-06-06 13:52:23 UTC 
Created attachment 189325 [details] [review]
Patch for master

Here is version of the patch that applies on master. The other one was for 2.32 and did not apply on master directly.
Comment 5 Patrick Ohly 2011-06-07 08:59:09 UTC 
Chris backported the fixes and I committed them to 2.32 and 3.0 branches:

715725caa2d72d38072aa7aefdc161a132229ff4 e_book_backend_file_get_contact_list: Fix memory leak
246fb4ca8d6836f97468c4a61e7a535ab771b805 e_book_backend_file_get_contact_list: Fix memory leak