On 02/06/2013 09:06 PM, Osier Yang wrote:
On 2013年02月07日 05:35, John Ferlan wrote:
> Fix various resource leaks discovered while parsing through Valgrind
> output
> ---
> src/conf/domain_conf.c | 12 +++++++++++-
> 1 file changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 27f5b5e..62a604f 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -7604,6 +7604,7 @@ virDomainVideoDefParseXML(const xmlNodePtr node,
> VIR_FREE(ram);
> VIR_FREE(vram);
> VIR_FREE(heads);
> + VIR_FREE(primary);
>
> return def;
>
> @@ -9587,6 +9588,7 @@ static virDomainDefPtr
> virDomainDefParseXML(virCapsPtr caps,
> }
>
> if ((value =
> virDomainFeatureStateTypeFromString(tmp))< 0) {
> + VIR_FREE(tmp);
> virReportError(VIR_ERR_XML_ERROR,
> _("invalid value of state
> argument "
> "for HyperV Enlightenment
> feature '%s'"),
> @@ -9594,6 +9596,7 @@ static virDomainDefPtr
> virDomainDefParseXML(virCapsPtr caps,
> goto error;
> }
>
> + VIR_FREE(tmp);
> def->hyperv_features[feature] = value;
> break;
Good to fix the leak in the similar hunk as well:
tmp = virXPathString("string(./features/apic/@eoi)", ctxt);
if (tmp) {
int eoi;
if ((eoi = virDomainFeatureStateTypeFromString(tmp))
<= 0) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("unknown value for attribute
eoi: %s"),
tmp);
goto error;
}
def->apic_eoi = eoi;
VIR_FREE(tmp);
}
>
> @@ -9922,6 +9925,7 @@ static virDomainDefPtr
> virDomainDefParseXML(virCapsPtr caps,
> if (controller->type == VIR_DOMAIN_CONTROLLER_TYPE_USB) {
> if (controller->model ==
> VIR_DOMAIN_CONTROLLER_MODEL_USB_NONE) {
> if (usb_other || usb_none) {
> + virDomainControllerDefFree(controller);
> virReportError(VIR_ERR_XML_DETAIL, "%s",
> _("Can't add another USB
> controller: "
> "USB is disabled for this
> domain"));
> @@ -9930,6 +9934,7 @@ static virDomainDefPtr
> virDomainDefParseXML(virCapsPtr caps,
> usb_none = true;
> } else {
> if (usb_none) {
> + virDomainControllerDefFree(controller);
> virReportError(VIR_ERR_XML_DETAIL, "%s",
> _("Can't add another USB
> controller: "
> "USB is disabled for this
> domain"));
> @@ -10227,6 +10232,7 @@ static virDomainDefPtr
> virDomainDefParseXML(virCapsPtr caps,
>
> /* Check if USB bus is required */
> if (input->bus == VIR_DOMAIN_INPUT_BUS_USB&& usb_none) {
> + virDomainInputDefFree(input);
> virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> _("Can't add USB input device. "
> "USB bus is disabled"));
> @@ -10324,6 +10330,7 @@ static virDomainDefPtr
> virDomainDefParseXML(virCapsPtr caps,
>
> if (video->primary) {
> if (primaryVideo) {
> + virDomainVideoDefFree(video);
> virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> _("Only one primary video device is
> supported"));
> goto error;
> @@ -10335,8 +10342,10 @@ static virDomainDefPtr
> virDomainDefParseXML(virCapsPtr caps,
> if (VIR_INSERT_ELEMENT_INPLACE(def->videos,
> ii,
> def->nvideos,
> - video)< 0)
> + video)< 0) {
> + virDomainVideoDefFree(video);
> goto error;
> + }
> }
> VIR_FREE(nodes);
>
> @@ -10452,6 +10461,7 @@ static virDomainDefPtr
> virDomainDefParseXML(virCapsPtr caps,
> goto error;
>
> if (hub->type == VIR_DOMAIN_HUB_TYPE_USB&& usb_none) {
> + virDomainHubDefFree(hub);
> virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> _("Can't add USB hub: "
> "USB is disabled for this domain"));
ACK with fixing the similar hunk.
In retrospect, the virDomainDefParseXML() will VIR_FREE(tmp) in the
error: path, so rather than add within the error path as I did at line
9592 and the error path for "./features/apic/@eoi", I removed that one
line.
I probably added that one as a result of adding the one in the non error
path and hyper-focusing on the code on the screen around the change.
John