From mboxrd@z Thu Jan  1 00:00:00 1970
From: Ian Jackson <Ian.Jackson@eu.citrix.com>
Subject: Re: [PATCH]: xl: don't free string literals
Date: Tue, 7 Sep 2010 19:02:08 +0100
Message-ID: <19590.32288.472147.509673@mariner.uk.xensource.com>
References: <1283775473.20276.135.camel@qabil.uk.xensource.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xensource.com>
In-Reply-To: <1283775473.20276.135.camel@qabil.uk.xensource.com>
List-Unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xensource.com>
List-Help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-Subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
Sender: xen-devel-bounces@lists.xensource.com
Errors-To: xen-devel-bounces@lists.xensource.com
To: Gianni Tedesco <gianni.tedesco@citrix.com>
Cc: Xen Devel <xen-devel@lists.xensource.com>, Stefano Stabellini <Stefano.Stabellini@eu.citrix.com>
List-Id: xen-devel@lists.xenproject.org

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.