[Pidgin] #3281: msimppl leaks memory in statstring, username, key_context, storing user info, user, MsimMessage dictionary parsing

[Pidgin]

#3281: msimppl leaks memory in statstring, username, key_context, storing user
info, user, MsimMessage dictionary parsing
----------------------+-----------------------------------------------------
  Reporter:  jeff     |       Owner:  jeff    
      Type:  defect   |      Status:  new     
  Priority:  minor    |   Milestone:          
 Component:  MySpace  |     Version:  2.2.0   
Resolution:           |    Keywords:  msimprpl
   Pending:  0        |  
----------------------+-----------------------------------------------------
Comment (by jeff):

 On 9/23/07, Oliver Hardt <ohardt at gmail.com> wrote:
 > Hey,
 >
 > i profiled/memcheck'ed my application yesterday (which uses msimprpl)
 > and i found a few memory leaks (or places that i think leak memory).
 > I'm not familiar with the msimprpl code so i can't always tell if it's
 > a valid problem or not but some of these are real leaks.
 > Let me know if you want the whole valgrind trace/output or if i can
 > help you in any other way.
 >
 > Cheers,
 >
 >   Oliver
 >
 >
 >
 > myspace.c:1073     leaks memory - statstring is a copy,
 > purple_markup_strip_html() returns a           copy
 Good catch. Removed g_strdup().

 > myspace.c:636        probably need g_free( username);
 Already have this in the current revision.

 > myspace.c:452        key_context is never free'd ?
 Added purple_cipher_context_destroy().

 > user.c: 170 - 182     all the g_strdup'ing is suspicious, comes up in
 > the memchecker way too often
 After msim_store_user_info_each() is called, I free value_str, so the
 function needs to g_strdup() it if it wants to keep a copy around. But I
 changed the caller to now not free the string, leaving the responsibility
 to msim_store_user_info_each(): if it doesn't need it, the function will
 free g_free(), if it does it won't. This will avoid copying and should be
 more efficient.

 Another problem was that the user structures are overwritten without
 freeing their previous values, if any. I now g_free() each string member
 before replacing it.

 > user.c:265              user is leaked later, probably in
 Added g_free() in non-error cases.

 > myspace.c:1862      user isn't free'd anywhere - memory from user.c:55
 is lost
 Not sure what to do about this one.

 > message.c:1197      careless g_strdup'ing
 g_strdup(key) is in fact leaked in msim_msg_dictionary_parse(), but the
 strings have to be copied because key and value point within the 'raw'
 parameter, which isn't guaranteed to be around. 'value' is dynamic, so it
 will be freed, but 'key' is a static string so you can pass string
 literals to msim_msg_new() and msim_msg_append(). This is currently
 unfixed. Any ideas on the best way to do it?


 Thanks for finding these bugs.

-- 
Ticket URL: <http://developer.pidgin.im/ticket/3281#comment:1>
Pidgin <http://pidgin.im>
Pidgin