WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

[Xen-devel] [PATCH,v2]: xl: don't free string literals

To: Xen Devel <xen-devel@xxxxxxxxxxxxxxxxxxx>
Subject: [Xen-devel] [PATCH,v2]: xl: don't free string literals
From: Gianni Tedesco <gianni.tedesco@xxxxxxxxxx>
Date: Wed, 8 Sep 2010 17:31:29 +0100
Cc: Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>
Delivery-date: Wed, 08 Sep 2010 09:34:19 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
The function init_dm_info() is initialising some strings from literals.
This is bad juju because when the destructor is called we cannot know if
the string literal was overridden with a strdup()'d value. Therefore
strdup values in the initialiser then introduce and use the function
libxlu_cfg_replace_string() which free's whatever is set before
strdupping the new value on top of it. The rule for the new call should
be clear due to const vs. non-const arguments - changing the behaviour
of libxlu_cfg_get_string() would cause more complexity than it saves.

Signed-off-by: Gianni Tedesco <gianni.tedesco@xxxxxxxxxx>

diff -r e14a3c281982 tools/libxl/libxlu_cfg.c
--- a/tools/libxl/libxlu_cfg.c  Wed Sep 08 16:54:16 2010 +0100
+++ b/tools/libxl/libxlu_cfg.c  Wed Sep 08 17:27:05 2010 +0100
@@ -151,7 +151,18 @@ int xlu_cfg_get_string(const XLU_Config 
     *value_r= set->values[0];
     return 0;
 }
-        
+ 
+int xlu_cfg_replace_string(const XLU_Config *cfg, const char *n,
+                           char **value_r) {
+    XLU_ConfigSetting *set;
+    int e;
+
+    e= find_atom(cfg,n,&set);  if (e) return e;
+    free(*value_r);
+    *value_r= strdup(set->values[0]);
+    return 0;
+}
+
 int xlu_cfg_get_long(const XLU_Config *cfg, const char *n,
                      long *value_r) {
     long l;
diff -r e14a3c281982 tools/libxl/libxlutil.h
--- a/tools/libxl/libxlutil.h   Wed Sep 08 16:54:16 2010 +0100
+++ b/tools/libxl/libxlutil.h   Wed Sep 08 17:27:05 2010 +0100
@@ -46,6 +46,7 @@ void xlu_cfg_destroy(XLU_Config*);
  */
 
 int xlu_cfg_get_string(const XLU_Config*, const char *n, const char **value_r);
+int xlu_cfg_replace_string(const XLU_Config *cfg, const char *n, char 
**value_r); /* free/strdup version */
 int xlu_cfg_get_long(const XLU_Config*, const char *n, long *value_r);
 
 int xlu_cfg_get_list(const XLU_Config*, const char *n,
diff -r e14a3c281982 tools/libxl/xl_cmdimpl.c
--- a/tools/libxl/xl_cmdimpl.c  Wed Sep 08 16:54:16 2010 +0100
+++ b/tools/libxl/xl_cmdimpl.c  Wed Sep 08 17:27:05 2010 +0100
@@ -297,7 +297,7 @@ static void init_dm_info(libxl_device_mo
     libxl_uuid_generate(&dm_info->uuid);
 
     dm_info->dom_name = c_info->name;
-    dm_info->device_model = "qemu-dm";
+    dm_info->device_model = strdup("qemu-dm");
     dm_info->videoram = b_info->video_memkb / 1024;
     dm_info->apic = b_info->u.hvm.apic;
     dm_info->vcpus = b_info->max_vcpus;
@@ -305,7 +305,7 @@ static void init_dm_info(libxl_device_mo
 
     dm_info->stdvga = 0;
     dm_info->vnc = 1;
-    dm_info->vnclisten = "127.0.0.1";
+    dm_info->vnclisten = strdup("127.0.0.1");
     dm_info->vncdisplay = 0;
     dm_info->vncunused = 1;
     dm_info->keymap = NULL;
@@ -313,7 +313,7 @@ static void init_dm_info(libxl_device_mo
     dm_info->opengl = 0;
     dm_info->nographic = 0;
     dm_info->serial = NULL;
-    dm_info->boot = "cda";
+    dm_info->boot = strdup("cda");
     dm_info->usb = 0;
     dm_info->usbdevice = NULL;
     dm_info->xen_platform_pci = 1;
@@ -1022,38 +1022,30 @@ skip_vfb:
         init_dm_info(dm_info, c_info, b_info);
 
         /* then process config related to dm */
-        if (!xlu_cfg_get_string (config, "device_model", &buf))
-            dm_info->device_model = strdup(buf);
+        !xlu_cfg_replace_string (config, "device_model", 
&dm_info->device_model);
         if (!xlu_cfg_get_long (config, "stdvga", &l))
             dm_info->stdvga = l;
         if (!xlu_cfg_get_long (config, "vnc", &l))
             dm_info->vnc = l;
-        if (!xlu_cfg_get_string (config, "vnclisten", &buf))
-            dm_info->vnclisten = strdup(buf);
-        if (!xlu_cfg_get_string (config, "vncpasswd", &buf))
-            dm_info->vncpasswd = strdup(buf);
+        xlu_cfg_replace_string (config, "vnclisten", &dm_info->vnclisten);
+        xlu_cfg_replace_string (config, "vncpasswd", &dm_info->vncpasswd);
         if (!xlu_cfg_get_long (config, "vncdisplay", &l))
             dm_info->vncdisplay = l;
         if (!xlu_cfg_get_long (config, "vncunused", &l))
             dm_info->vncunused = l;
-        if (!xlu_cfg_get_string (config, "keymap", &buf))
-            dm_info->keymap = strdup(buf);
+        xlu_cfg_replace_string (config, "keymap", &dm_info->keymap);
         if (!xlu_cfg_get_long (config, "sdl", &l))
             dm_info->sdl = l;
         if (!xlu_cfg_get_long (config, "opengl", &l))
             dm_info->opengl = l;
         if (!xlu_cfg_get_long (config, "nographic", &l))
             dm_info->nographic = l;
-        if (!xlu_cfg_get_string (config, "serial", &buf))
-            dm_info->serial = strdup(buf);
-        if (!xlu_cfg_get_string (config, "boot", &buf))
-            dm_info->boot = strdup(buf);
+        xlu_cfg_replace_string (config, "serial", &dm_info->serial);
+        xlu_cfg_replace_string (config, "boot", &dm_info->boot);
         if (!xlu_cfg_get_long (config, "usb", &l))
             dm_info->usb = l;
-        if (!xlu_cfg_get_string (config, "usbdevice", &buf))
-            dm_info->usbdevice = strdup(buf);
-        if (!xlu_cfg_get_string (config, "soundhw", &buf))
-            dm_info->soundhw = strdup(buf);
+        xlu_cfg_replace_string (config, "usbdevice", &dm_info->usbdevice);
+        xlu_cfg_replace_string (config, "soundhw", &dm_info->soundhw);
         if (!xlu_cfg_get_long (config, "xen_platform_pci", &l))
             dm_info->xen_platform_pci = l;
     }



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel