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

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.