[Xfce4-commits] r30225 - in xfce4-appfinder/trunk: . src

[Brian J. Tarricone]

On 2009/07/08 23:46, Colin Leroy wrote:
> On Thu, 9 Jul 2009 08:25:01 +0200, Nick Schermer wrote:
>
> Hi,
>
>>> +      categories_array = g_new0 (const gchar *, g_list_length
>>> (categories) + 1); +
>>> +      for (lp = categories, n = 0; lp != NULL; lp = lp->next, ++n)
>>> +        categories_array[n] = lp->data;
>
> gchar *tmp = NULL, *categories_string = NULL;
>
> for (lp = categories; lp != NULL; lp = lp->next)
>    {
>      tmp = g_strdup_printf("%s%s%s",
>               categories_string ? categories_string : "",
>               categories_string ? ", " : "",
>               lp->data);
>      g_free(categories_string);
>      categories_string = tmp;
>    }

An extra allocation for each list item?  Ick.  Better to just iterate 
over the list twice; figure out the total strlen in the first pass, 
allocate a buffer a single time, and then do the second pass and copy 
the chars to the new buffer.  (Hell, the original code iterates over the 
list twice [g_list_length() is O(n)] already anyway.)

Or just mostly use the original code and do g_string_append_printf() for 
each list item.  Yah, you get more allocs that way, but GString does a 
better job than just doing it manually, usually.

The original code, isn't actually that bad, depending on how 
g_strjoinv() is implemented.  But a single malloc() for categories_array 
is likely to be slower than a pass over the entire list with a strlen() 
on each element.

Eh, I dunno.  Performance could go either way depending on the number of 
list entries (which I'd figure would usually be 4-6 in the average case, 
maybe worst case around 15).

	-b