On Tue, 2010-08-31 at 17:50 +0100, Ian Jackson wrote:
> Gianni Tedesco writes ("[PATCH]: xl: introduce
> xlu_cfg_get_(string|long)_default"):
> > The function init_dm_info() in xl_cmdimpl.c is used to initialise a
> > libxl_device_model_info structure with some default values. After being
> > called, some of those values are overridden. For the string values which
> > are not overwritten we either end up with a double free or attempt to
> > free a string literal resulting in a segfault. This type of usage model
> > would be complex to fix by adding strdup()'s in the init function and
> > then checking and freeing when over-writing.
> >
> > My proposed solution is to add default versions of xlu_cfg_get_string
> > and xlu_cfg_get_long. This way both double-free's and leaks are avoided
> > and arguably the intention of the code is clearer.
>
> I think in general the approach of trying to do the defaults one
> config setting at a time like this is insufficiently powerful.
>
> If we're going to change it, it would be better to do all the default
> setting at the end, after the config file has been completely read.
> That means that the default for one config setting can depend on the
> actual values for another.
>
> It also gets rid of the memory management problem, because defaults
> never need to be applied to anything that is a non-null pointer.
I had considered that approach first but then discarded it because it
was hard to tell which values had been set or not, eg. if a boolean
option had been set to 0 by an explicit config entry or due to initial
memset() - meaning that you wouldn't know whether to override it with a
default or not. IOW we would need a bitmap or something to keep track.
Perhaps a higher level API would be better:
struct {
...
}configs[] = {
{.type = STR, .u.str = {&dm_info.vnclisten, "127.0.0.1"}, },
{.type = INT, .u.num = {&dm_info.vnc, 1}, },
...
};
Gianni
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
Home