On Wed, 2010-09-08 at 17:31 +0100, Gianni Tedesco wrote:
> 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>
About to bail for the day, but I guess this (untested) patch makes sense
on top of this?
Avoids c_info->name being a string literal as well. Although I wonder if
maybe domain configurations with no name could just be rejected as
invalid? Having a whole bunch of domains called "test" doesn't seem
likely to be what anyone wants...
Ian.
Subject: xl: use xlu_cfg_replace_string in a few more places.
Signed-off-by: Ian Campbell <ian.campbell@xxxxxxxxxx>
diff -r d6026f6dcebf tools/libxl/xl_cmdimpl.c
--- a/tools/libxl/xl_cmdimpl.c Wed Sep 08 17:37:47 2010 +0100
+++ b/tools/libxl/xl_cmdimpl.c Wed Sep 08 17:41:56 2010 +0100
@@ -606,10 +606,8 @@ static void parse_config_data(const char
if (!xlu_cfg_get_long (config, "hap", &l))
c_info->hap = l;
- if (!xlu_cfg_get_string (config, "name", &buf))
- c_info->name = strdup(buf);
- else
- c_info->name = "test";
+ if (xlu_cfg_replace_string (config, "name", &c_info->name))
+ c_info->name = strdup("test");
if (!xlu_cfg_get_string (config, "uuid", &buf) ) {
if ( libxl_uuid_from_string(&c_info->uuid, buf) ) {
@@ -695,8 +693,7 @@ static void parse_config_data(const char
if (!xlu_cfg_get_long (config, "videoram", &l))
b_info->video_memkb = l * 1024;
- if (!xlu_cfg_get_string (config, "kernel", &buf))
- b_info->kernel.path = strdup(buf);
+ xlu_cfg_replace_string (config, "kernel", &b_info->kernel.path);
if (c_info->hvm == 1) {
if (!xlu_cfg_get_long (config, "pae", &l))
@@ -734,10 +731,8 @@ static void parse_config_data(const char
exit(1);
}
- if (!xlu_cfg_get_string (config, "bootloader", &buf))
- b_info->u.pv.bootloader = strdup(buf);
- if (!xlu_cfg_get_string (config, "bootloader_args", &buf))
- b_info->u.pv.bootloader_args = strdup(buf);
+ xlu_cfg_replace_string (config, "bootloader",
&b_info->u.pv.bootloader);
+ xlu_cfg_replace_string (config, "bootloader_args",
&b_info->u.pv.bootloader_args);
if (!b_info->u.pv.bootloader && !b_info->kernel.path) {
fprintf(stderr, "Neither kernel nor bootloader specified\n");
@@ -745,8 +740,7 @@ static void parse_config_data(const char
}
b_info->u.pv.cmdline = cmdline;
- if (!xlu_cfg_get_string (config, "ramdisk", &buf))
- b_info->u.pv.ramdisk.path = strdup(buf);
+ xlu_cfg_replace_string (config, "ramdisk", &b_info->u.pv.ramdisk.path);
}
if (!xlu_cfg_get_list (config, "disk", &vbds, 0)) {
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
Home