[evolution-patches] Freeing a xmlGetProp() return value with g_free()



evo/e-util/e-menu.c/emph_construct_menu() assigns tmp =
xmlGetProp("file"), then  assigns ui->filename = tmp.

In emph_free_ui() it then calls g_free(ui->filename).

This is wrong, isn't it? The value returned by xmlGetProp() should be
freed with xmlFree(). It just happens to work most of the time to use
g_free(), as xmlGetProp() normally returns a malloc()ed result, and
g_free() normally just calls free(). But this is not necessarily always
the case.

(To catch things like this, it would be so much better if for instance
the libxml2 functions didn't return the exact pointers that malloc() has
returned, but purposedly allocated, say, four extra bytes and returned a
correspondingly offset pointer. (xmlFree() would then of course have to
correspondingly de-offset what it passes to free().) Then mixups like
this would be more likely to cause serious problems and be noticed
earlier.)

I guess it's cleanest here to use g_strdup(tmp) in the assignment to
ui->filename, and call xmlFree(tmp) immediately afterwards, and leave
the g_free() in emph_free_ui() as is.

I noticed this while I was adding Win32 installation relocatability for
the UI file names.

--tml


Index: e-util/e-menu.c
===================================================================
RCS file: /cvs/gnome/evolution/e-util/e-menu.c,v
retrieving revision 1.9
diff -p -u -2 -r1.9 e-menu.c
--- e-util/e-menu.c	26 Nov 2005 03:51:16 -0000	1.9
+++ e-util/e-menu.c	27 Dec 2005 10:34:43 -0000
@@ -30,11 +30,12 @@
 #include <glib.h>
 
-#include "e-menu.h"
-
-#include <e-util/e-icon-factory.h>
-
 #include <libgnome/gnome-i18n.h>
 #include <bonobo/bonobo-ui-util.h>
 
+#include <libedataserver/e-util.h>
+
+#include "e-menu.h"
+#include "e-icon-factory.h"
+
 #define d(x)
 
@@ -785,5 +786,14 @@ emph_construct_menu(EPluginHook *eph, xm
 				EMenuUIFile *ui = g_malloc0(sizeof(*ui));
 
-				ui->filename = tmp;
+				ui->filename = g_strdup(tmp);
+				xmlFree(tmp);
+#ifdef G_OS_WIN32
+				{
+					char *mapped_location = e_util_replace_prefix (e_util_get_prefix (),
+										       ui->filename);
+					g_free (ui->filename);
+					ui->filename = mapped_location;
+				}
+#endif
 				ui->appdir = g_strdup(g_get_tmp_dir());
 				ui->appname = g_strdup("Evolution");


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]