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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list