[libvirt] [PATCH RFC 2/5] conf: Introduce scsi hostdev

[Han Cheng]

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.

Regards,
Cheng