[Bug 27400] review fix-segaults

[bugzilla-daemon at freedesktop.org]

http://bugs.freedesktop.org/show_bug.cgi?id=27400





--- Comment #2 from Cosimo Alfarano <cosimo.alfarano at collabora.co.uk>  2010-04-01 06:04:15 PST ---
Summary:

- tpl_channel_text_msg_token_exist_in_cache() removed
- side-effect removed from tpl_channel_text_clean_up_stale_tokens()

I rebased the branch to remove the old patches.

> In the first patch it seems you're changing the semantics of
> tpl_channel_text_clean_up_stale_tokens from "free the list and the contained
> strings" to "free the list, don't free the contained strings". Are you sure
> this isn't a memory leak?

Yes, introduced but not meant.
I removed the side-effect, removing this code as well.

> In the second patch I think the argument change is a necessary evil with the
> API as currently designed, but it surprises me that a function called

The intent was grouping repeated code for readability, the result is much more
readable when it isn't grouped, so I removed the method.

I'd like to express better why I'm removing the item from the list, which is
the key point.
I don't know if it's clear enough: lines 773 and 783, repeated at 889 and 899.


> tpl_channel_text_msg_token_exist_in_cache() has side-effects. (Also, it should
> have been called ..._exists_... anyway.)
> Please rename this function to tpl_channel_text_msg_token_remove_from_cache()

I removed the function.

> + * frees the list and its items (char *).
> We use gchar * for strings allocated by the g_malloc/g_free family, and char *

Comment actually removed because of the removal of the side-effect.

>> +  if (g_list_length (l) > 1)
>> +    CRITICAL ("found more than one log-id %s, it shouldn't be possible",
>> +        tpl_message_token);
> No. Please think about GList's underlying structure, and why this assertion
> will fail.

Removed with tpl_channel_text_msg_token_exist_in_cache()


-- 
Configure bugmail: http://bugs.freedesktop.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.