One thing I noticed while browsing around Inkscape source, was the pattern "allocate temp gtk string, call some api, deallocate temp string".
I think it makes sense to make a small "TempGString" class that captures this (RAII) pattern, and deallocates the string on going out of scope. This would have some advantages compared to current approach:
1) string would be freed regardless of how the code exits the block (exceptions e.g.)
2) 2 lines instead of 3
3) easier on the eyes
Sample before-after code snippet:
Before:
if (!qname_prefix(code).id()) {
gchar *svg_name = g_strconcat(prefix, ":", g_quark_to_string(code), NULL);
repr->setCodeUnsafe(g_quark_from_string(svg_name));
g_free(svg_name);
}
After:
if (!qname_prefix(code).id()) {
TempGString svg_name(g_strconcat(prefix, ":", g_quark_to_string(code), NULL));
repr->setCodeUnsafe(g_quark_from_string(svg_name));
}
The constructor takes the freshly allocated string, and the destructor calls g_free on it. There is an operator overload so that contexts that expect ordinary pointers to gchar* gets what it want. This automagic casting is a point I'm a bit ambivalent about myself; I wouldn't mind having an accessor function "get_gchar_ptr()" or similar just to be super explicit, thoughts on that?
I don't think this would break anything, however it might be that I'm ignorant to some weird behaviour of ghar* or glib/gtk?
On a convention/code style notion, I looked through the code for RAII (worst abbreviation for something so clear/simple semantically) and noticed it's used in a couple other places, however I've still been wary of introducing this refactoring to the code base as I don't know if you guys would like it.
The pattern occurs on at least 140 places in the code base.
Cheers,