On 07/23/2015 08:35 AM, Ján Tomko wrote: > [reducing the cc-list] > > On Fri, Jul 17, 2015 at 02:43:31PM -0400, Laine Stump wrote: >> There are some non-0 default values in virDomainControllerDef (and >> will soon be more) that are easier to not forget if the remembering is >> done by a single initializer function (rather than inline code after >> allocating the obejct with generic VIR_ALLOC(). >> --- >> new in V2 >> >> src/conf/domain_conf.c | 64 +++++++++++++++++++++++++++++++++----------------- >> 1 file changed, 43 insertions(+), 21 deletions(-) >> >> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c >> index 5a9a88d..8dd4bf0 100644 >> --- a/src/conf/domain_conf.c >> +++ b/src/conf/domain_conf.c >> @@ -1527,6 +1527,37 @@ virDomainDiskSetFormat(virDomainDiskDefPtr def, int format) >> } >> >> >> +static virDomainControllerDefPtr >> +virDomainControllerDefNew(virDomainControllerType type) >> +{ >> + virDomainControllerDefPtr def; >> + >> + if (VIR_ALLOC(def) < 0) >> + return NULL; >> + >> + def->type = type; >> + >> + /* initialize anything that has a non-0 default */ >> + switch ((virDomainControllerType) def->type) { >> + case VIR_DOMAIN_CONTROLLER_TYPE_VIRTIO_SERIAL: >> + def->opts.vioserial.ports = -1; >> + def->opts.vioserial.vectors = -1; >> + break; >> + case VIR_DOMAIN_CONTROLLER_TYPE_PCI: >> + case VIR_DOMAIN_CONTROLLER_TYPE_IDE: >> + case VIR_DOMAIN_CONTROLLER_TYPE_FDC: >> + case VIR_DOMAIN_CONTROLLER_TYPE_SCSI: >> + case VIR_DOMAIN_CONTROLLER_TYPE_SATA: >> + case VIR_DOMAIN_CONTROLLER_TYPE_CCID: >> + case VIR_DOMAIN_CONTROLLER_TYPE_USB: >> + case VIR_DOMAIN_CONTROLLER_TYPE_LAST: >> + break; >> + } >> + > The model could be initialized to -1 here as well. > >> + return def; >> +} >> + >> + >> void virDomainControllerDefFree(virDomainControllerDefPtr def) >> { >> if (!def) >> @@ -7610,18 +7642,18 @@ virDomainControllerDefParseXML(xmlNodePtr node, >> >> ctxt->node = node; >> >> - if (VIR_ALLOC(def) < 0) >> - return NULL; >> - >> - type = virXMLPropString(node, "type"); >> - if (type) { >> - if ((def->type = virDomainControllerTypeFromString(type)) < 0) { >> + typeStr = virXMLPropString(node, "type"); >> + if (typeStr) { >> + if ((type = virDomainControllerTypeFromString(typeStr)) < 0) { >> virReportError(VIR_ERR_CONFIG_UNSUPPORTED, >> - _("Unknown controller type '%s'"), type); >> + _("Unknown controller type '%s'"), typeStr); >> goto error; >> } >> } >> >> + if (!(def = virDomainControllerDefNew(type))) >> + return NULL; > goto error; > Otherwise typeStr gets leaked. Thanks! I've made both of those changes. -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list