GNOME Bugzilla – Bug 172406
gconvert.c, function open_converter
Last modified: 2011-02-18 16:14:20 UTC
This function allocates strings that might very well be under Dr. Evil's control on the stack. That doesn't feel safe. (And if it is for speed, then using an sprintf type function on it is plain silly.) This function should probably use malloc right away.
Or use a hash table whose keys are pairs of strings
This is a performance problem also. Doing a lot of g_utf8_collate calls shows that about 40% of the time for g_utf8_collate is spent in open_converter, the vast majority in the _g_sprintf call. (_g_sprintf ends up doing lots of memory allocation due to the way it is implemented; it is simply not the right tool for string concatenation.)
Created attachment 49896 [details] [review] Use malloc directly and do not use sprintf to concatenate strings This cuts around 30% of the time for gnumeric to load the test case in bug 311816. That load time is dominated by g_str_collate. At the same time it resolves the security issue with the stack management.
Created attachment 49955 [details] [review] Updated patch Updated patch that avoids malloc for small keys that are in the cache.
2006-12-14 Matthias Clasen <mclasen@redhat.com> * glib/gconvert.c (open_converter): Don't use alloca and avoid allocating memory for small keys that are already cached. (#172406, Morten Welinder)