Re: [PATCH]: xl: don't free string literals

[Gianni Tedesco <gianni.tedesco@citrix.com>]

On Tue, 2010-09-07 at 19:02 +0100, Ian Jackson wrote:
> Gianni Tedesco writes ("[PATCH]: xl: don't free string literals"):
> > 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 the initialisers in init_dm_info() and unconditionally free them
> > before assigning non-default values to prevent their leakage.
> 
> Can't we replace the dozen copies of this
> 
> > -        if (!xlu_cfg_get_string (config, "device_model", &buf))
> > +        if (!xlu_cfg_get_string (config, "device_model", &buf)) {
> > +            free(dm_info->device_model);
> >              dm_info->device_model = strdup(buf);
> > +        }
> 
> with something like
> 
> > -        if (!xlu_cfg_get_string (config, "device_model", &buf))
> > +        if (!xlu_cfg_get_string_mallocd (config, "device_model", &buf)) {
> 
> or whatever you want to call the function ?  Or just change the
> definition of xlu_cfg_get_string and declare that it always frees its
> argument if it's not 0 to start with.
> 
> Ian.

Absolutely, I think I'll just change xlu_cfg_get_string() everywhere.
Don't see a need to have two versions of a function and a non-obvious
rule about which to use where. Will hack that up and re-send later.

Gianni