On 2013年03月21日 17:01, Han Cheng wrote:
> On 03/06/2013 02:24 PM, Osier Yang wrote:
>> On 2013年03月04日 14:01, Han Cheng wrote:
>>> @@ -2928,6 +2929,96 @@ virDomainParseLegacyDeviceAddress(char *devaddr,
>>> }
>>>
>>> static int
>>> +virDomainHostdevSubsysScsiDefParseXML(const xmlNodePtr node,
>>> + virDomainHostdevDefPtr def)
>>> +{
>>> + int ret = -1;
>>> + xmlNodePtr cur;
>>
>> If you define those variables here:
>>
>> char *bus, *target, *unit;
>>
>>> +
>>> + cur = node->children;
>>> + while (cur != NULL) {
>>> + if (cur->type == XML_ELEMENT_NODE) {
>>> + if (xmlStrEqual(cur->name, BAD_CAST "address")) {
>>> + char *bus, *target, *unit;
>>> +
>>> + bus=virXMLPropString(cur, "bus");
>>> + if (bus) {
>>
>> These codes can be simplified as:
>>
>> if ((bus = virXMLPropString(cur, "bus"))< 0) {
>> virReportError(...);
>> goto out;
>> }
>>
>> if (virStrToLong_ui(bus, NULL, 0,&def->source.subsys.u.scsi.bus)< 0) {
>> virReportError(...);
>> goto out;
>> }
>>
>> With freeing the strings in "out". [1]
>>
>>> + ret = virStrToLong_ui(bus, NULL, 0,
>>> +&def->source.subsys.u.scsi.bus);
>>> + VIR_FREE(bus);
>>> + if (ret< 0) {
>>> + virReportError(VIR_ERR_INTERNAL_ERROR,
>>> + _("cannot parse bus %s"), bus);
>>> + goto out;
> ...
>>> + }
>>> + }
>>> + cur = cur->next;
>>> + }
>>> +
>>> + ret = 0;
>>> +out:
>>
>> [1]
>>
>> VIR_FREE(bus);
>> VIR_FREE(target);
>> VIR_FREE(unit);
>>
>>> + return ret;
>>> +}
>>> +
> This may cause memory leak if someone add more than one<address> by
> mistake.
No, as they are free'ed anyway after 1 round parsing (assuming there
are multiple address specified indeed), regardless of whether the
parsing succeeded or not.
And the things we store in the memory for these attributes are numbers.
So no memory leak.
Osier
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list