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] Re: [PATCH]: xl: introduce xlu_cfg_get_(string|long)_default

To: Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>
Subject: [Xen-devel] Re: [PATCH]: xl: introduce xlu_cfg_get_(string|long)_default
From: Gianni Tedesco <gianni.tedesco@xxxxxxxxxx>
Date: Tue, 31 Aug 2010 18:04:26 +0100
Cc: Ian Campbell <Ian.Campbell@xxxxxxxxxxxxx>, Xen Devel <xen-devel@xxxxxxxxxxxxxxxxxxx>, Stefano Stabellini <Stefano.Stabellini@xxxxxxxxxxxxx>
Delivery-date: Tue, 31 Aug 2010 10:08:09 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <19581.13001.354762.600142@xxxxxxxxxxxxxxxxxxxxxxxx>
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>
References: <1283269353.25936.9.camel@xxxxxxxxxxxxxxxxxxxxxx> <19581.13001.354762.600142@xxxxxxxxxxxxxxxxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
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