[Pidgin] #9115: purple_account_option_destroy() doesn't free list key/value

[Pidgin]

#9115: purple_account_option_destroy() doesn't free list key/value
-----------------------------------------+----------------------------------
 Reporter:  stefan.becker at nokia.com      |        Owner:           
     Type:  patch                        |       Status:  new      
Milestone:  Patches Needing Review       |    Component:  libpurple
  Version:  2.5.5                        |   Resolution:           
 Keywords:  memory leak account options  |  
-----------------------------------------+----------------------------------
Changes (by darkrain42):

  * keywords:  memory leak, account options => memory leak account options
  * type:  defect => patch
  * milestone:  => Patches Needing Review


Old description:

> purple_account_option_add_list_item() has this code:
>
> kvp = g_new0(PurpleKeyValuePair, 1);
> kvp->key = g_strdup(key);
> kvp->value = g_strdup(value);
> option->default_value.list =
> g_list_append(option->default_value.list,kvp);
>

> purple_account_option_destroy() only frees "kvp" and the list:
>
> else if (option->type == PURPLE_PREF_STRING_LIST)
> {
>         if (option->default_value.list != NULL)
>         {
>                 g_list_foreach(option->default_value.list, (GFunc)g_free,
> NULL);
>                 g_list_free(option->default_value.list);
>         }
> }
>
> As a result libpurple leaks memory for the key & value srings.
>

> Of course purple_account_option_destroy() must not touch the entries
> provided by the caller to purple_account_option_list_new().

New description:

 purple_account_option_add_list_item() has this code:

 {{{
 kvp = g_new0(PurpleKeyValuePair, 1);
 kvp->key = g_strdup(key);
 kvp->value = g_strdup(value);
 option->default_value.list =
 g_list_append(option->default_value.list,kvp);
 }}}

 purple_account_option_destroy() only frees "kvp" and the list:
 {{{
 else if (option->type == PURPLE_PREF_STRING_LIST)
 {
         if (option->default_value.list != NULL)
         {
                 g_list_foreach(option->default_value.list, (GFunc)g_free,
 NULL);
                 g_list_free(option->default_value.list);
         }
 }
 }}}

 As a result libpurple leaks memory for the key & value srings.


 Of course purple_account_option_destroy() must not touch the entries
 provided by the caller to purple_account_option_list_new().

--

Comment:

 This is mostly a "patch" (although an actual diff would be great).

 Also, as I look at this, the API specifically specifies that the entries
 provided to `purple_account_option_list_new` need to be allocated such
 that the account option API will free them.

 {{{
  * The list passed will be owned by the account option, and the
  * strings inside will be freed automatically.
 }}}

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