Re: [libvirt] [PATCHv2 04/17] conf: add virDomainControllerDefNew()

[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.