[libvirt] [PATCH 09/10] conf: Provide missing virDomain*DefNew() functions

Andrea Bolognani posted 10 patches 7 years, 10 months ago
[libvirt] [PATCH 09/10] conf: Provide missing virDomain*DefNew() functions
Posted by Andrea Bolognani 7 years, 10 months ago
Same as virDomainDeviceInfo itself, any struct that
embeds it needs to be initialized properly before use;
however, none of the structs in question even had a
proper allocation function defined.

Implement an allocation function for all structs
embedding a virDomainDeviceInfo and use them instead
of plain VIR_ALLOC() everywhere.

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
---
 src/bhyve/bhyve_parse_command.c |   4 +-
 src/conf/domain_conf.c          | 280 +++++++++++++++++++++++++++++++---------
 src/conf/domain_conf.h          |  15 +++
 src/libvirt_private.syms        |  11 ++
 src/openvz/openvz_conf.c        |   2 +-
 src/qemu/qemu_command.c         |  12 +-
 src/qemu/qemu_domain.c          |  11 +-
 src/qemu/qemu_domain_address.c  |   2 +-
 src/qemu/qemu_hotplug.c         |   5 +-
 src/qemu/qemu_parse_command.c   |  27 ++--
 src/vbox/vbox_common.c          |  12 +-
 src/vmx/vmx.c                   |   2 +-
 src/vz/vz_sdk.c                 |   6 +-
 src/xen/xen_driver.c            |   2 +-
 src/xenapi/xenapi_driver.c      |   2 +-
 src/xenconfig/xen_common.c      |   2 +-
 src/xenconfig/xen_sxpr.c        |   8 +-
 src/xenconfig/xen_xl.c          |   2 +-
 src/xenconfig/xen_xm.c          |   2 +-
 19 files changed, 303 insertions(+), 104 deletions(-)

diff --git a/src/bhyve/bhyve_parse_command.c b/src/bhyve/bhyve_parse_command.c
index fcaaed2..b9e8bc6 100644
--- a/src/bhyve/bhyve_parse_command.c
+++ b/src/bhyve/bhyve_parse_command.c
@@ -432,7 +432,7 @@ bhyveParsePCIDisk(virDomainDefPtr def,
     int idx = -1;
     virDomainDiskDefPtr disk = NULL;
 
-    if (VIR_ALLOC(disk) < 0)
+    if (!(disk = virDomainDiskDefNew(NULL)))
         goto cleanup;
     if (VIR_ALLOC(disk->src) < 0)
         goto error;
@@ -505,7 +505,7 @@ bhyveParsePCINet(virDomainDefPtr def,
     const char *separator = NULL;
     const char *mac = NULL;
 
-    if (VIR_ALLOC(net) < 0)
+    if (!(net = virDomainNetDefNew()))
         goto cleanup;
 
     /* As we only support interface type='bridge' and cannot
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 11c4627..055fde9 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -1389,6 +1389,17 @@ void virDomainGraphicsDefFree(virDomainGraphicsDefPtr def)
     VIR_FREE(def);
 }
 
+virDomainInputDefPtr
+virDomainInputDefNew(void)
+{
+    virDomainInputDefPtr def;
+
+    if (VIR_ALLOC(def) < 0)
+        return NULL;
+
+    return def;
+}
+
 void virDomainInputDefFree(virDomainInputDefPtr def)
 {
     if (!def)
@@ -2021,6 +2032,17 @@ virDomainNetDefClear(virDomainNetDefPtr def)
     virNetDevVlanClear(&def->vlan);
 }
 
+virDomainNetDefPtr
+virDomainNetDefNew(void)
+{
+    virDomainNetDefPtr def;
+
+    if (VIR_ALLOC(def) < 0)
+        return NULL;
+
+    return def;
+}
+
 void
 virDomainNetDefFree(virDomainNetDefPtr def)
 {
@@ -2248,6 +2270,17 @@ void virDomainChrDefFree(virDomainChrDefPtr def)
     VIR_FREE(def);
 }
 
+virDomainSmartcardDefPtr
+virDomainSmartcardDefNew(void)
+{
+    virDomainSmartcardDefPtr def;
+
+    if (VIR_ALLOC(def) < 0)
+        return NULL;
+
+    return def;
+}
+
 void virDomainSmartcardDefFree(virDomainSmartcardDefPtr def)
 {
     size_t i;
@@ -2285,6 +2318,17 @@ void virDomainSoundCodecDefFree(virDomainSoundCodecDefPtr def)
     VIR_FREE(def);
 }
 
+virDomainSoundDefPtr
+virDomainSoundDefNew(void)
+{
+    virDomainSoundDefPtr def;
+
+    if (VIR_ALLOC(def) < 0)
+        return NULL;
+
+    return def;
+}
+
 void virDomainSoundDefFree(virDomainSoundDefPtr def)
 {
     if (!def)
@@ -2300,6 +2344,17 @@ void virDomainSoundDefFree(virDomainSoundDefPtr def)
     VIR_FREE(def);
 }
 
+virDomainMemballoonDefPtr
+virDomainMemballoonDefNew(void)
+{
+    virDomainMemballoonDefPtr def;
+
+    if (VIR_ALLOC(def) < 0)
+        return NULL;
+
+    return def;
+}
+
 void virDomainMemballoonDefFree(virDomainMemballoonDefPtr def)
 {
     if (!def)
@@ -2311,6 +2366,17 @@ void virDomainMemballoonDefFree(virDomainMemballoonDefPtr def)
     VIR_FREE(def);
 }
 
+virDomainNVRAMDefPtr
+virDomainNVRAMDefNew(void)
+{
+    virDomainNVRAMDefPtr def;
+
+    if (VIR_ALLOC(def) < 0)
+        return NULL;
+
+    return def;
+}
+
 void virDomainNVRAMDefFree(virDomainNVRAMDefPtr def)
 {
     if (!def)
@@ -2321,6 +2387,17 @@ void virDomainNVRAMDefFree(virDomainNVRAMDefPtr def)
     VIR_FREE(def);
 }
 
+virDomainWatchdogDefPtr
+virDomainWatchdogDefNew(void)
+{
+    virDomainWatchdogDefPtr def;
+
+    if (VIR_ALLOC(def) < 0)
+        return NULL;
+
+    return def;
+}
+
 void virDomainWatchdogDefFree(virDomainWatchdogDefPtr def)
 {
     if (!def)
@@ -2331,6 +2408,50 @@ void virDomainWatchdogDefFree(virDomainWatchdogDefPtr def)
     VIR_FREE(def);
 }
 
+virDomainRNGDefPtr
+virDomainRNGDefNew(void)
+{
+    virDomainRNGDefPtr def;
+
+    if (VIR_ALLOC(def) < 0)
+        return NULL;
+
+    return def;
+}
+
+void
+virDomainRNGDefFree(virDomainRNGDefPtr def)
+{
+    if (!def)
+        return;
+
+    switch ((virDomainRNGBackend) def->backend) {
+    case VIR_DOMAIN_RNG_BACKEND_RANDOM:
+        VIR_FREE(def->source.file);
+        break;
+    case VIR_DOMAIN_RNG_BACKEND_EGD:
+        virDomainChrSourceDefFree(def->source.chardev);
+        break;
+    case VIR_DOMAIN_RNG_BACKEND_LAST:
+        break;
+    }
+
+    virDomainDeviceInfoClear(&def->info);
+    VIR_FREE(def->virtio);
+    VIR_FREE(def);
+}
+
+virDomainShmemDefPtr
+virDomainShmemDefNew(void)
+{
+    virDomainShmemDefPtr def;
+
+    if (VIR_ALLOC(def) < 0)
+        return NULL;
+
+    return def;
+}
+
 void virDomainShmemDefFree(virDomainShmemDefPtr def)
 {
     if (!def)
@@ -2342,6 +2463,17 @@ void virDomainShmemDefFree(virDomainShmemDefPtr def)
     VIR_FREE(def);
 }
 
+virDomainVideoDefPtr
+virDomainVideoDefNew(void)
+{
+    virDomainVideoDefPtr def;
+
+    if (VIR_ALLOC(def) < 0)
+        return NULL;
+
+    return def;
+}
+
 void virDomainVideoDefFree(virDomainVideoDefPtr def)
 {
     if (!def)
@@ -2457,6 +2589,17 @@ void virDomainHostdevDefClear(virDomainHostdevDefPtr def)
     def->privateData = NULL;
 }
 
+virDomainTPMDefPtr
+virDomainTPMDefNew(void)
+{
+    virDomainTPMDefPtr def;
+
+    if (VIR_ALLOC(def) < 0)
+        return NULL;
+
+    return def;
+}
+
 void virDomainTPMDefFree(virDomainTPMDefPtr def)
 {
     if (!def)
@@ -2489,6 +2632,17 @@ void virDomainHostdevDefFree(virDomainHostdevDefPtr def)
         VIR_FREE(def);
 }
 
+virDomainHubDefPtr
+virDomainHubDefNew(void)
+{
+    virDomainHubDefPtr def;
+
+    if (VIR_ALLOC(def) < 0)
+        return NULL;
+
+    return def;
+}
+
 void virDomainHubDefFree(virDomainHubDefPtr def)
 {
     if (!def)
@@ -2498,6 +2652,17 @@ void virDomainHubDefFree(virDomainHubDefPtr def)
     VIR_FREE(def);
 }
 
+virDomainRedirdevDefPtr
+virDomainRedirdevDefNew(void)
+{
+    virDomainRedirdevDefPtr def;
+
+    if (VIR_ALLOC(def) < 0)
+        return NULL;
+
+    return def;
+}
+
 void virDomainRedirdevDefFree(virDomainRedirdevDefPtr def)
 {
     if (!def)
@@ -2523,6 +2688,17 @@ void virDomainRedirFilterDefFree(virDomainRedirFilterDefPtr def)
     VIR_FREE(def);
 }
 
+virDomainMemoryDefPtr
+virDomainMemoryDefNew(void)
+{
+    virDomainMemoryDefPtr def;
+
+    if (VIR_ALLOC(def) < 0)
+        return NULL;
+
+    return def;
+}
+
 void virDomainMemoryDefFree(virDomainMemoryDefPtr def)
 {
     if (!def)
@@ -2733,6 +2909,17 @@ virDomainResourceDefFree(virDomainResourceDefPtr resource)
     VIR_FREE(resource);
 }
 
+virDomainPanicDefPtr
+virDomainPanicDefNew(void)
+{
+    virDomainPanicDefPtr def;
+
+    if (VIR_ALLOC(def) < 0)
+        return NULL;
+
+    return def;
+}
+
 void
 virDomainPanicDefFree(virDomainPanicDefPtr panic)
 {
@@ -9732,8 +9919,8 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
     xmlNodePtr oldnode = ctxt->node;
     int rv, val;
 
-    if (VIR_ALLOC(def) < 0)
-        return NULL;
+    if (!(def = virDomainNetDefNew()))
+        goto error;
 
     ctxt->node = node;
 
@@ -11198,8 +11385,8 @@ virDomainSmartcardDefParseXML(virDomainXMLOptionPtr xmlopt,
     virDomainSmartcardDefPtr def;
     size_t i;
 
-    if (VIR_ALLOC(def) < 0)
-        return NULL;
+    if (!(def = virDomainSmartcardDefNew()))
+        goto error;
 
     mode = virXMLPropString(node, "mode");
     if (mode == NULL) {
@@ -11344,8 +11531,8 @@ virDomainTPMDefParseXML(xmlNodePtr node,
     xmlNodePtr *backends = NULL;
     int nbackends;
 
-    if (VIR_ALLOC(def) < 0)
-        return NULL;
+    if (!(def = virDomainTPMDefNew()))
+        goto error;
 
     model = virXMLPropString(node, "model");
     if (model != NULL &&
@@ -11425,8 +11612,8 @@ virDomainPanicDefParseXML(xmlNodePtr node,
     virDomainPanicDefPtr panic;
     char *model = NULL;
 
-    if (VIR_ALLOC(panic) < 0)
-        return NULL;
+    if (!(panic = virDomainPanicDefNew()))
+        goto error;
 
     if (virDomainDeviceInfoParseXML(node, NULL, &panic->info, flags) < 0)
         goto error;
@@ -11462,8 +11649,8 @@ virDomainInputDefParseXML(const virDomainDef *dom,
     char *type = NULL;
     char *bus = NULL;
 
-    if (VIR_ALLOC(def) < 0)
-        return NULL;
+    if (!(def = virDomainInputDefNew()))
+        goto error;
 
     ctxt->node = node;
 
@@ -11606,8 +11793,8 @@ virDomainHubDefParseXML(xmlNodePtr node, unsigned int flags)
     virDomainHubDefPtr def;
     char *type = NULL;
 
-    if (VIR_ALLOC(def) < 0)
-        return NULL;
+    if (!(def = virDomainHubDefNew()))
+        goto error;
 
     type = virXMLPropString(node, "type");
 
@@ -12708,8 +12895,8 @@ virDomainSoundDefParseXML(xmlNodePtr node,
     virDomainSoundDefPtr def;
     xmlNodePtr save = ctxt->node;
 
-    if (VIR_ALLOC(def) < 0)
-        return NULL;
+    if (!(def = virDomainSoundDefNew()))
+        goto error;
 
     ctxt->node = node;
 
@@ -12777,8 +12964,8 @@ virDomainWatchdogDefParseXML(xmlNodePtr node,
     char *action = NULL;
     virDomainWatchdogDefPtr def;
 
-    if (VIR_ALLOC(def) < 0)
-        return NULL;
+    if (!(def = virDomainWatchdogDefNew()))
+        goto error;
 
     model = virXMLPropString(node, "model");
     if (model == NULL) {
@@ -12835,8 +13022,8 @@ virDomainRNGDefParseXML(virDomainXMLOptionPtr xmlopt,
     xmlNodePtr *backends = NULL;
     int nbackends;
 
-    if (VIR_ALLOC(def) < 0)
-        return NULL;
+    if (!(def = virDomainRNGDefNew()))
+        goto error;
 
     if (!(model = virXMLPropString(node, "model"))) {
         virReportError(VIR_ERR_XML_ERROR, "%s", _("missing RNG device model"));
@@ -12949,8 +13136,8 @@ virDomainMemballoonDefParseXML(xmlNodePtr node,
     xmlNodePtr save = ctxt->node;
     unsigned int period = 0;
 
-    if (VIR_ALLOC(def) < 0)
-        return NULL;
+    if (!(def = virDomainMemballoonDefNew()))
+        goto error;
 
     model = virXMLPropString(node, "model");
     if (model == NULL) {
@@ -13010,8 +13197,8 @@ virDomainNVRAMDefParseXML(xmlNodePtr node,
 {
    virDomainNVRAMDefPtr def;
 
-    if (VIR_ALLOC(def) < 0)
-        return NULL;
+    if (!(def = virDomainNVRAMDefNew()))
+        goto error;
 
     if (virDomainDeviceInfoParseXML(node, NULL, &def->info, flags) < 0)
         goto error;
@@ -13034,9 +13221,8 @@ virDomainShmemDefParseXML(xmlNodePtr node,
     xmlNodePtr save = ctxt->node;
     xmlNodePtr server = NULL;
 
-
-    if (VIR_ALLOC(def) < 0)
-        return NULL;
+    if (!(def = virDomainShmemDefNew()))
+        goto error;
 
     ctxt->node = node;
 
@@ -13550,8 +13736,8 @@ virDomainVideoDefParseXML(xmlNodePtr node,
 
     ctxt->node = node;
 
-    if (VIR_ALLOC(def) < 0)
-        return NULL;
+    if (!(def = virDomainVideoDefNew()))
+        goto error;
 
     cur = node->children;
     while (cur != NULL) {
@@ -13789,8 +13975,8 @@ virDomainRedirdevDefParseXML(virDomainXMLOptionPtr xmlopt,
     char *bus = NULL, *type = NULL;
     int remaining;
 
-    if (VIR_ALLOC(def) < 0)
-        return NULL;
+    if (!(def = virDomainRedirdevDefNew()))
+        goto error;
 
     if (!(def->source = virDomainChrSourceDefNew(xmlopt)))
         goto error;
@@ -14255,8 +14441,8 @@ virDomainMemoryDefParseXML(xmlNodePtr memdevNode,
 
     ctxt->node = memdevNode;
 
-    if (VIR_ALLOC(def) < 0)
-        return NULL;
+    if (!(def = virDomainMemoryDefNew()))
+        goto error;
 
     if (!(tmp = virXMLPropString(memdevNode, "model"))) {
         virReportError(VIR_ERR_XML_ERROR, "%s",
@@ -16383,7 +16569,7 @@ virDomainDefAddController(virDomainDefPtr def, int type, int idx, int model)
     cont->model = model;
 
     if (VIR_APPEND_ELEMENT_COPY(def->controllers, def->ncontrollers, cont) < 0) {
-        VIR_FREE(cont);
+        virDomainControllerDefFree(cont);
         return NULL;
     }
 
@@ -16477,14 +16663,14 @@ virDomainDefMaybeAddInput(virDomainDefPtr def,
             return 0;
     }
 
-    if (VIR_ALLOC(input) < 0)
+    if (!(input = virDomainInputDefNew()))
         return -1;
 
     input->type = type;
     input->bus = bus;
 
     if (VIR_APPEND_ELEMENT(def->inputs, def->ninputs, input) < 0) {
-        VIR_FREE(input);
+        virDomainInputDefFree(input);
         return -1;
     }
 
@@ -20827,14 +21013,14 @@ static int
 virDomainDefAddImplicitVideo(virDomainDefPtr def)
 {
     int ret = -1;
-    virDomainVideoDefPtr video = NULL;
+    virDomainVideoDefPtr video;
 
     /* For backwards compatibility, if no <video> tag is set but there
      * is a <graphics> tag, then we add a single video tag */
     if (def->ngraphics == 0 || def->nvideos > 0)
         return 0;
 
-    if (VIR_ALLOC(video) < 0)
+    if (!(video = virDomainVideoDefNew()))
         goto cleanup;
     video->type = virDomainVideoDefaultType(def);
     if (video->type < 0) {
@@ -23253,28 +23439,6 @@ virDomainRNGDefFormat(virBufferPtr buf,
     return 0;
 }
 
-void
-virDomainRNGDefFree(virDomainRNGDefPtr def)
-{
-    if (!def)
-        return;
-
-    switch ((virDomainRNGBackend) def->backend) {
-    case VIR_DOMAIN_RNG_BACKEND_RANDOM:
-        VIR_FREE(def->source.file);
-        break;
-    case VIR_DOMAIN_RNG_BACKEND_EGD:
-        virDomainChrSourceDefFree(def->source.chardev);
-        break;
-    case VIR_DOMAIN_RNG_BACKEND_LAST:
-        break;
-    }
-
-    virDomainDeviceInfoClear(&def->info);
-    VIR_FREE(def->virtio);
-    VIR_FREE(def);
-}
-
 
 static int
 virDomainMemorySourceDefFormat(virBufferPtr buf,
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 00d0d65..a09669a 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2067,6 +2067,7 @@ struct _virDomainMemoryDef {
     virDomainDeviceInfo info;
 };
 
+virDomainMemoryDefPtr virDomainMemoryDefNew(void);
 void virDomainMemoryDefFree(virDomainMemoryDefPtr def);
 
 struct _virDomainIdMapEntry {
@@ -2641,9 +2642,11 @@ int virDomainObjWait(virDomainObjPtr vm);
 int virDomainObjWaitUntil(virDomainObjPtr vm,
                           unsigned long long whenms);
 
+virDomainPanicDefPtr virDomainPanicDefNew(void);
 void virDomainPanicDefFree(virDomainPanicDefPtr panic);
 void virDomainResourceDefFree(virDomainResourceDefPtr resource);
 void virDomainGraphicsDefFree(virDomainGraphicsDefPtr def);
+virDomainInputDefPtr virDomainInputDefNew(void);
 void virDomainInputDefFree(virDomainInputDefPtr def);
 virDomainDiskDefPtr virDomainDiskDefNew(virDomainXMLOptionPtr xmlopt);
 void virDomainDiskDefFree(virDomainDiskDefPtr def);
@@ -2672,24 +2675,34 @@ virDomainControllerDefNew(virDomainControllerType type);
 void virDomainFSDefFree(virDomainFSDefPtr def);
 void virDomainActualNetDefFree(virDomainActualNetDefPtr def);
 void virDomainNetDefClear(virDomainNetDefPtr def);
+virDomainNetDefPtr virDomainNetDefNew(void);
 void virDomainNetDefFree(virDomainNetDefPtr def);
+virDomainSmartcardDefPtr virDomainSmartcardDefNew(void);
 void virDomainSmartcardDefFree(virDomainSmartcardDefPtr def);
 void virDomainChrDefFree(virDomainChrDefPtr def);
 void virDomainChrSourceDefFree(virDomainChrSourceDefPtr def);
 int virDomainChrSourceDefCopy(virDomainChrSourceDefPtr src,
                               virDomainChrSourceDefPtr dest);
 void virDomainSoundCodecDefFree(virDomainSoundCodecDefPtr def);
+virDomainSoundDefPtr virDomainSoundDefNew(void);
 void virDomainSoundDefFree(virDomainSoundDefPtr def);
+virDomainMemballoonDefPtr virDomainMemballoonDefNew(void);
 void virDomainMemballoonDefFree(virDomainMemballoonDefPtr def);
+virDomainNVRAMDefPtr virDomainNVRAMDefNew(void);
 void virDomainNVRAMDefFree(virDomainNVRAMDefPtr def);
+virDomainWatchdogDefPtr virDomainWatchdogDefNew(void);
 void virDomainWatchdogDefFree(virDomainWatchdogDefPtr def);
+virDomainVideoDefPtr virDomainVideoDefNew(void);
 void virDomainVideoDefFree(virDomainVideoDefPtr def);
 virDomainHostdevDefPtr virDomainHostdevDefNew(virDomainXMLOptionPtr xmlopt);
 void virDomainHostdevDefClear(virDomainHostdevDefPtr def);
 void virDomainHostdevDefFree(virDomainHostdevDefPtr def);
+virDomainHubDefPtr virDomainHubDefNew(void);
 void virDomainHubDefFree(virDomainHubDefPtr def);
+virDomainRedirdevDefPtr virDomainRedirdevDefNew(void);
 void virDomainRedirdevDefFree(virDomainRedirdevDefPtr def);
 void virDomainRedirFilterDefFree(virDomainRedirFilterDefPtr def);
+virDomainShmemDefPtr virDomainShmemDefNew(void);
 void virDomainShmemDefFree(virDomainShmemDefPtr def);
 void virDomainDeviceDefFree(virDomainDeviceDefPtr def);
 virDomainDeviceDefPtr virDomainDeviceDefCopy(virDomainDeviceDefPtr src,
@@ -2699,6 +2712,7 @@ virDomainDeviceDefPtr virDomainDeviceDefCopy(virDomainDeviceDefPtr src,
 int virDomainDeviceAddressIsValid(virDomainDeviceInfoPtr info,
                                   int type);
 virDomainDeviceInfoPtr virDomainDeviceGetInfo(virDomainDeviceDefPtr device);
+virDomainTPMDefPtr virDomainTPMDefNew(void);
 void virDomainTPMDefFree(virDomainTPMDefPtr def);
 
 typedef int (*virDomainDeviceInfoCallback)(virDomainDefPtr def,
@@ -2903,6 +2917,7 @@ int virDomainDefCompatibleDevice(virDomainDefPtr def,
                                  virDomainDeviceDefPtr dev,
                                  virDomainDeviceAction action);
 
+virDomainRNGDefPtr virDomainRNGDefNew(void);
 void virDomainRNGDefFree(virDomainRNGDefPtr def);
 
 int virDomainDiskIndexByAddress(virDomainDefPtr def,
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 48e9e33..98f3969 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -377,11 +377,13 @@ virDomainHostdevModeTypeToString;
 virDomainHostdevRemove;
 virDomainHostdevSubsysPCIBackendTypeToString;
 virDomainHostdevSubsysTypeToString;
+virDomainHubDefNew;
 virDomainHubTypeFromString;
 virDomainHubTypeToString;
 virDomainHypervTypeFromString;
 virDomainHypervTypeToString;
 virDomainInputDefFree;
+virDomainInputDefNew;
 virDomainIOMMUModelTypeFromString;
 virDomainIOMMUModelTypeToString;
 virDomainIOThreadIDAdd;
@@ -406,9 +408,11 @@ virDomainLoaderTypeFromString;
 virDomainLoaderTypeToString;
 virDomainLockFailureTypeFromString;
 virDomainLockFailureTypeToString;
+virDomainMemballoonDefNew;
 virDomainMemballoonModelTypeFromString;
 virDomainMemballoonModelTypeToString;
 virDomainMemoryDefFree;
+virDomainMemoryDefNew;
 virDomainMemoryFindByDef;
 virDomainMemoryFindInactiveByDef;
 virDomainMemoryInsert;
@@ -419,6 +423,7 @@ virDomainNetAppendIPAddress;
 virDomainNetDefClear;
 virDomainNetDefFormat;
 virDomainNetDefFree;
+virDomainNetDefNew;
 virDomainNetFind;
 virDomainNetFindIdx;
 virDomainNetGenerateMAC;
@@ -439,6 +444,7 @@ virDomainNetTypeFromString;
 virDomainNetTypeToString;
 virDomainNostateReasonTypeFromString;
 virDomainNostateReasonTypeToString;
+virDomainNVRAMDefNew;
 virDomainObjAssignDef;
 virDomainObjBroadcast;
 virDomainObjCopyPersistentDef;
@@ -463,6 +469,8 @@ virDomainObjWait;
 virDomainObjWaitUntil;
 virDomainOSTypeFromString;
 virDomainOSTypeToString;
+virDomainPanicDefFree;
+virDomainPanicDefNew;
 virDomainParseMemory;
 virDomainPausedReasonTypeFromString;
 virDomainPausedReasonTypeToString;
@@ -503,6 +511,7 @@ virDomainSmartcardTypeToString;
 virDomainSmbiosModeTypeFromString;
 virDomainSmbiosModeTypeToString;
 virDomainSoundDefFree;
+virDomainSoundDefNew;
 virDomainSoundModelTypeFromString;
 virDomainSoundModelTypeToString;
 virDomainStartupPolicyTypeFromString;
@@ -530,6 +539,7 @@ virDomainUSBDeviceDefForeach;
 virDomainVideoDefaultRAM;
 virDomainVideoDefaultType;
 virDomainVideoDefFree;
+virDomainVideoDefNew;
 virDomainVideoTypeFromString;
 virDomainVideoTypeToString;
 virDomainVideoVGAConfTypeFromString;
@@ -538,6 +548,7 @@ virDomainVirtTypeFromString;
 virDomainVirtTypeToString;
 virDomainWatchdogActionTypeFromString;
 virDomainWatchdogActionTypeToString;
+virDomainWatchdogDefNew;
 virDomainWatchdogModelTypeFromString;
 virDomainWatchdogModelTypeToString;
 virDomainXMLOptionGetNamespace;
diff --git a/src/openvz/openvz_conf.c b/src/openvz/openvz_conf.c
index 23a02d7..a551229 100644
--- a/src/openvz/openvz_conf.c
+++ b/src/openvz/openvz_conf.c
@@ -219,7 +219,7 @@ openvzReadNetworkConf(virDomainDefPtr def,
     } else if (ret > 0) {
         token = strtok_r(temp, " ", &saveptr);
         while (token != NULL) {
-            if (VIR_ALLOC(net) < 0)
+            if (!(net = virDomainNetDefNew()))
                 goto error;
 
             net->type = VIR_DOMAIN_NET_TYPE_ETHERNET;
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index c53ab97..43a23d1 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3522,19 +3522,22 @@ qemuBuildMemoryCellBackendStr(virDomainDefPtr def,
     const char *backendType;
     int ret = -1;
     int rc;
-    virDomainMemoryDef mem = { 0 };
+    virDomainMemoryDefPtr mem;
     unsigned long long memsize = virDomainNumaGetNodeMemorySize(def->numa,
                                                                 cell);
 
+    if (!(mem = virDomainMemoryDefNew()))
+        goto cleanup;
+
     *backendStr = NULL;
-    mem.size = memsize;
-    mem.targetNode = cell;
+    mem->size = memsize;
+    mem->targetNode = cell;
 
     if (virAsprintf(&alias, "ram-node%zu", cell) < 0)
         goto cleanup;
 
     if ((rc = qemuBuildMemoryBackendStr(&props, &backendType, cfg, qemuCaps,
-                                        def, &mem, auto_nodeset, false)) < 0)
+                                        def, mem, auto_nodeset, false)) < 0)
         goto cleanup;
 
     if (!(*backendStr = virQEMUBuildObjectCommandlineFromJSON(backendType,
@@ -3547,6 +3550,7 @@ qemuBuildMemoryCellBackendStr(virDomainDefPtr def,
  cleanup:
     VIR_FREE(alias);
     virJSONValueFree(props);
+    virDomainMemoryDefFree(mem);
 
     return ret;
 }
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 8e7404d..451bbdf 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -2511,7 +2511,7 @@ qemuDomainDefAddDefaultDevices(virDomainDefPtr def,
 
     if (addDefaultMemballoon && !def->memballoon) {
         virDomainMemballoonDefPtr memballoon;
-        if (VIR_ALLOC(memballoon) < 0)
+        if (!(memballoon = virDomainMemballoonDefNew()))
             goto cleanup;
 
         memballoon->model = VIR_DOMAIN_MEMBALLOON_MODEL_VIRTIO;
@@ -2545,10 +2545,13 @@ qemuDomainDefAddDefaultDevices(virDomainDefPtr def,
 
         if (j == def->npanics) {
             virDomainPanicDefPtr panic;
-            if (VIR_ALLOC(panic) < 0 ||
-                VIR_APPEND_ELEMENT_COPY(def->panics,
+
+            if (!(panic = virDomainPanicDefNew()))
+                goto cleanup;
+
+            if (VIR_APPEND_ELEMENT_COPY(def->panics,
                                         def->npanics, panic) < 0) {
-                VIR_FREE(panic);
+                virDomainPanicDefFree(panic);
                 goto cleanup;
             }
         }
diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c
index 93ab304..c827a7e 100644
--- a/src/qemu/qemu_domain_address.c
+++ b/src/qemu/qemu_domain_address.c
@@ -2342,7 +2342,7 @@ qemuDomainUSBAddressAddHubs(virDomainDefPtr def)
               data.count, available_ports, hubs_needed);
 
     for (i = 0; i < hubs_needed; i++) {
-        if (VIR_ALLOC(hub) < 0)
+        if (!(hub = virDomainHubDefNew()))
             return -1;
         hub->type = VIR_DOMAIN_HUB_TYPE_USB;
 
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index b5b62df..735983a 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -572,16 +572,15 @@ qemuDomainFindOrCreateSCSIDiskController(virQEMUDriverPtr driver,
 
     /* No SCSI controller present, for backward compatibility we
      * now hotplug a controller */
-    if (VIR_ALLOC(cont) < 0)
+    if (!(cont = virDomainControllerDefNew(VIR_DOMAIN_CONTROLLER_TYPE_SCSI)))
         return NULL;
-    cont->type = VIR_DOMAIN_CONTROLLER_TYPE_SCSI;
     cont->idx = controller;
     cont->model = -1;
 
     VIR_INFO("No SCSI controller present, hotplugging one");
     if (qemuDomainAttachControllerDevice(driver,
                                          vm, cont) < 0) {
-        VIR_FREE(cont);
+        virDomainControllerDefFree(cont);
         return NULL;
     }
 
diff --git a/src/qemu/qemu_parse_command.c b/src/qemu/qemu_parse_command.c
index 90e8d09..3d1f058 100644
--- a/src/qemu/qemu_parse_command.c
+++ b/src/qemu/qemu_parse_command.c
@@ -649,7 +649,7 @@ qemuParseCommandLineDisk(virDomainXMLOptionPtr xmlopt,
                           0) < 0)
         return NULL;
 
-    if (VIR_ALLOC(def) < 0)
+    if (!(def = virDomainDiskDefNew(NULL)))
         goto cleanup;
     if (VIR_ALLOC(def->src) < 0)
         goto error;
@@ -1032,7 +1032,7 @@ qemuParseCommandLineNet(virDomainXMLOptionPtr xmlopt,
         nkeywords = 0;
     }
 
-    if (VIR_ALLOC(def) < 0)
+    if (!(def = virDomainNetDefNew()))
         goto cleanup;
 
     /* 'tap' could turn into libvirt type=ethernet, type=bridge or
@@ -1502,10 +1502,13 @@ qemuParseCommandLineCPU(virDomainDefPtr dom,
 
             if (j == dom->npanics) {
                 virDomainPanicDefPtr panic;
-                if (VIR_ALLOC(panic) < 0 ||
-                    VIR_APPEND_ELEMENT_COPY(dom->panics,
+
+                if (!(panic = virDomainPanicDefNew()))
+                    goto cleanup;
+
+                if (VIR_APPEND_ELEMENT_COPY(dom->panics,
                                             dom->npanics, panic) < 0) {
-                    VIR_FREE(panic);
+                    virDomainPanicDefFree(panic);
                     goto cleanup;
                 }
                 panic->model = VIR_DOMAIN_PANIC_MODEL_HYPERV;
@@ -2240,7 +2243,7 @@ qemuParseCommandLine(virCapsPtr caps,
                 STREQ(val, "mouse") ||
                 STREQ(val, "keyboard")) {
                 virDomainInputDefPtr input;
-                if (VIR_ALLOC(input) < 0)
+                if (!(input = virDomainInputDefNew()))
                     goto error;
                 input->bus = VIR_DOMAIN_INPUT_BUS_USB;
                 if (STREQ(val, "tablet"))
@@ -2330,7 +2333,7 @@ qemuParseCommandLine(virCapsPtr caps,
 
                 if (type != -1) {
                     virDomainSoundDefPtr snd;
-                    if (VIR_ALLOC(snd) < 0)
+                    if (!(snd = virDomainSoundDefNew()))
                         goto error;
                     snd->model = type;
                     if (VIR_APPEND_ELEMENT(def->sounds, def->nsounds, snd) < 0) {
@@ -2347,7 +2350,7 @@ qemuParseCommandLine(virCapsPtr caps,
 
             if (model != -1) {
                 virDomainWatchdogDefPtr wd;
-                if (VIR_ALLOC(wd) < 0)
+                if (!(wd = virDomainWatchdogDefNew()))
                     goto error;
                 wd->model = model;
                 wd->action = VIR_DOMAIN_WATCHDOG_ACTION_RESET;
@@ -2450,7 +2453,7 @@ qemuParseCommandLine(virCapsPtr caps,
                    STRPREFIX(progargv[i + 1], "spapr-nvram.reg=")) {
             WANT_VALUE();
 
-            if (VIR_ALLOC(def->nvram) < 0)
+            if (!(def->nvram = virDomainNVRAMDefNew()))
                 goto error;
 
             def->nvram->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO;
@@ -2477,7 +2480,7 @@ qemuParseCommandLine(virCapsPtr caps,
 
             if (STRPREFIX(opts, "virtio-balloon")) {
                 WANT_VALUE();
-                if (VIR_ALLOC(def->memballoon) < 0)
+                if (!(def->memballoon = virDomainMemballoonDefNew()))
                     goto error;
                 def->memballoon->model = VIR_DOMAIN_MEMBALLOON_MODEL_VIRTIO;
             } else {
@@ -2606,7 +2609,7 @@ qemuParseCommandLine(virCapsPtr caps,
 
     if (def->ngraphics) {
         virDomainVideoDefPtr vid;
-        if (VIR_ALLOC(vid) < 0)
+        if (!(vid = virDomainVideoDefNew()))
             goto error;
         if (def->virtType == VIR_DOMAIN_VIRT_XEN)
             vid->type = VIR_DOMAIN_VIDEO_TYPE_XEN;
@@ -2631,7 +2634,7 @@ qemuParseCommandLine(virCapsPtr caps,
      */
     if (!def->memballoon) {
         virDomainMemballoonDefPtr memballoon;
-        if (VIR_ALLOC(memballoon) < 0)
+        if (!(memballoon = virDomainMemballoonDefNew()))
             goto error;
         memballoon->model = VIR_DOMAIN_MEMBALLOON_MODEL_NONE;
 
diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c
index 92ee371..32f45c3 100644
--- a/src/vbox/vbox_common.c
+++ b/src/vbox/vbox_common.c
@@ -3223,7 +3223,7 @@ vboxDumpVideo(virDomainDefPtr def, vboxDriverPtr data ATTRIBUTE_UNUSED,
         return -1;
     def->nvideos = 1;
 
-    if (VIR_ALLOC(def->videos[0]) < 0)
+    if (!(def->videos[0] = virDomainVideoDefNew()))
         return -1;
 
     gVBoxAPI.UIMachine.GetVRAMSize(machine, &VRAMSize);
@@ -3390,7 +3390,7 @@ vboxDumpSharedFolders(virDomainDefPtr def, vboxDriverPtr data, IMachine *machine
         char *hostPath = NULL;
         PRBool writable = PR_FALSE;
 
-        if (VIR_ALLOC(def->fss[i]) < 0)
+        if (!(def->fss[i] = virDomainFSDefNew()))
             goto sharedFoldersCleanup;
 
         def->fss[i]->type = VIR_DOMAIN_FS_TYPE_MOUNT;
@@ -3451,7 +3451,7 @@ vboxDumpNetwork(virDomainDefPtr def, vboxDriverPtr data, IMachine *machine, PRUi
     /* Allocate memory for the networkcards which are enabled */
     if ((def->nnets > 0) && (VIR_ALLOC_N(def->nets, def->nnets) >= 0)) {
         for (i = 0; i < def->nnets; i++)
-            ignore_value(VIR_ALLOC(def->nets[i]));
+            def->nets[i] = virDomainNetDefNew();
     }
 
     /* Now get the details about the network cards here */
@@ -3585,7 +3585,7 @@ vboxDumpAudio(virDomainDefPtr def, vboxDriverPtr data ATTRIBUTE_UNUSED,
 
             def->nsounds = 1;
             if (VIR_ALLOC_N(def->sounds, def->nsounds) >= 0) {
-                if (VIR_ALLOC(def->sounds[0]) >= 0) {
+                if ((def->sounds[0] = virDomainSoundDefNew())) {
                     gVBoxAPI.UIAudioAdapter.GetAudioController(audioAdapter, &audioController);
                     if (audioController == AudioControllerType_SB16) {
                         def->sounds[0]->model = VIR_DOMAIN_SOUND_MODEL_SB16;
@@ -3630,7 +3630,7 @@ vboxDumpSerial(virDomainDefPtr def, vboxDriverPtr data, IMachine *machine, PRUin
     /* Allocate memory for the serial ports which are enabled */
     if ((def->nserials > 0) && (VIR_ALLOC_N(def->serials, def->nserials) >= 0)) {
         for (i = 0; i < def->nserials; i++)
-            ignore_value(VIR_ALLOC(def->serials[i]));
+            def->serials[i] = virDomainChrDefNew(NULL);
     }
 
     /* Now get the details about the serial ports here */
@@ -3718,7 +3718,7 @@ vboxDumpParallel(virDomainDefPtr def, vboxDriverPtr data, IMachine *machine, PRU
     /* Allocate memory for the parallel ports which are enabled */
     if ((def->nparallels > 0) && (VIR_ALLOC_N(def->parallels, def->nparallels) >= 0)) {
         for (i = 0; i < def->nparallels; i++)
-            ignore_value(VIR_ALLOC(def->parallels[i]));
+            def->parallels[i] = virDomainChrDefNew(NULL);
     }
 
     /* Now get the details about the parallel ports here */
diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c
index 96507f1..2203599 100644
--- a/src/vmx/vmx.c
+++ b/src/vmx/vmx.c
@@ -3037,7 +3037,7 @@ virVMXParseSVGA(virConfPtr conf, virDomainVideoDefPtr *def)
         return -1;
     }
 
-    if (VIR_ALLOC(*def) < 0)
+    if (!(*def = virDomainVideoDefNew()))
         return -1;
 
     (*def)->type = VIR_DOMAIN_VIDEO_TYPE_VMVGA;
diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
index 0aa1a30..161ce30 100644
--- a/src/vz/vz_sdk.c
+++ b/src/vz/vz_sdk.c
@@ -552,7 +552,7 @@ prlsdkAddDomainVideoInfoCt(virDomainDefPtr def)
     if (def->ngraphics == 0)
         return 0;
 
-    if (VIR_ALLOC(video) < 0)
+    if (!(virDomainVideoDefNew()))
         goto cleanup;
 
     video->type = VIR_DOMAIN_VIDEO_TYPE_PARALLELS;
@@ -581,7 +581,7 @@ prlsdkAddDomainVideoInfoVm(PRL_HANDLE sdkdom, virDomainDefPtr def)
     ret = PrlVmCfg_GetVideoRamSize(sdkdom, &videoRam);
     prlsdkCheckRetGoto(ret, error);
 
-    if (VIR_ALLOC(video) < 0)
+    if (!(video = virDomainVideoDefNew()))
         goto error;
 
     if (VIR_ALLOC(accel) < 0)
@@ -1157,7 +1157,7 @@ prlsdkAddDomainNetInfo(PRL_HANDLE sdkdom, virDomainDefPtr def)
         ret = PrlVmCfg_GetNetAdapter(sdkdom, i, &netAdapter);
         prlsdkCheckRetGoto(ret, error);
 
-        if (VIR_ALLOC(net) < 0)
+        if (!(net = virDomainNetDefNew()))
             goto error;
 
         if (prlsdkGetNetInfo(netAdapter, net, IS_CT(def)) < 0)
diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c
index b5cd47e..53e1ba8 100644
--- a/src/xen/xen_driver.c
+++ b/src/xen/xen_driver.c
@@ -375,7 +375,7 @@ xenDomainDefPostParse(virDomainDefPtr def,
 {
     if (!def->memballoon) {
         virDomainMemballoonDefPtr memballoon;
-        if (VIR_ALLOC(memballoon) < 0)
+        if (!(memballoon = virDomainMemballoonDefNew()))
             return -1;
 
         memballoon->model = VIR_DOMAIN_MEMBALLOON_MODEL_XEN;
diff --git a/src/xenapi/xenapi_driver.c b/src/xenapi/xenapi_driver.c
index fb462cd..09fbcff 100644
--- a/src/xenapi/xenapi_driver.c
+++ b/src/xenapi/xenapi_driver.c
@@ -1548,7 +1548,7 @@ xenapiDomainGetXMLDesc(virDomainPtr dom, unsigned int flags)
             goto error;
         }
         for (i = 0; i < vif_set->size; i++) {
-            if (VIR_ALLOC(defPtr->nets[i]) < 0) {
+            if (!(defPtr->nets[i] = virDomainNetDefNew()))
                 xen_vif_set_free(vif_set);
                 goto error;
             }
diff --git a/src/xenconfig/xen_common.c b/src/xenconfig/xen_common.c
index 6d7dc2c..146acc4 100644
--- a/src/xenconfig/xen_common.c
+++ b/src/xenconfig/xen_common.c
@@ -942,7 +942,7 @@ xenParseVif(virConfPtr conf, virDomainDefPtr def, const char *vif_typename)
                 key = nextkey;
             }
 
-            if (VIR_ALLOC(net) < 0)
+            if (!(net = virDomainNetDefNew()))
                 goto cleanup;
 
             if (mac[0]) {
diff --git a/src/xenconfig/xen_sxpr.c b/src/xenconfig/xen_sxpr.c
index fefa61a..f37a458 100644
--- a/src/xenconfig/xen_sxpr.c
+++ b/src/xenconfig/xen_sxpr.c
@@ -605,7 +605,7 @@ xenParseSxprNets(virDomainDefPtr def,
             model = sexpr_node(node, "device/vif/model");
             type = sexpr_node(node, "device/vif/type");
 
-            if (VIR_ALLOC(net) < 0)
+            if (!(net = virDomainNetDefNew()))
                 goto cleanup;
 
             if (tmp != NULL ||
@@ -728,7 +728,7 @@ xenParseSxprSound(virDomainDefPtr def,
 
         for (i = 0; i < (VIR_DOMAIN_SOUND_MODEL_ES1370 + 1); i++) {
             virDomainSoundDefPtr sound;
-            if (VIR_ALLOC(sound) < 0)
+            if (!(sound = virDomainSoundDefNew()))
                 goto error;
             sound->model = i;
             def->sounds[def->nsounds++] = sound;
@@ -752,7 +752,7 @@ xenParseSxprSound(virDomainDefPtr def,
                 goto error;
             }
 
-            if (VIR_ALLOC(sound) < 0)
+            if (!(sound = virDomainSoundDefNew()))
                 goto error;
 
             if ((sound->model = virDomainSoundModelTypeFromString(model)) < 0) {
@@ -801,7 +801,7 @@ xenParseSxprUSB(virDomainDefPtr def,
                     STREQ(tmp, "mouse") ||
                     STREQ(tmp, "keyboard")) {
                     virDomainInputDefPtr input;
-                    if (VIR_ALLOC(input) < 0)
+                    if (!(input = virDomainInputDefNew()))
                         goto error;
                     input->bus = VIR_DOMAIN_INPUT_BUS_USB;
                     if (STREQ(tmp, "tablet"))
diff --git a/src/xenconfig/xen_xl.c b/src/xenconfig/xen_xl.c
index fa3f1d0..86eacf1 100644
--- a/src/xenconfig/xen_xl.c
+++ b/src/xenconfig/xen_xl.c
@@ -550,7 +550,7 @@ xenParseXLInputDevs(virConfPtr conf, virDomainDefPtr def)
                      STREQ(str, "mouse") ||
                      STREQ(str, "keyboard"))) {
                 virDomainInputDefPtr input;
-                if (VIR_ALLOC(input) < 0)
+                if (!(input = virDomainInputDefNew()))
                     return -1;
 
                 input->bus = VIR_DOMAIN_INPUT_BUS_USB;
diff --git a/src/xenconfig/xen_xm.c b/src/xenconfig/xen_xm.c
index 8ef68bb..79c7f3b 100644
--- a/src/xenconfig/xen_xm.c
+++ b/src/xenconfig/xen_xm.c
@@ -412,7 +412,7 @@ xenParseXMInputDevs(virConfPtr conf, virDomainDefPtr def)
                  STREQ(str, "mouse") ||
                  STREQ(str, "keyboard"))) {
             virDomainInputDefPtr input;
-            if (VIR_ALLOC(input) < 0)
+            if (!(input = virDomainInputDefNew()))
                 return -1;
 
             input->bus = VIR_DOMAIN_INPUT_BUS_USB;
-- 
2.7.5

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 09/10] conf: Provide missing virDomain*DefNew() functions
Posted by Ján Tomko 7 years, 10 months ago
On Thu, Jun 29, 2017 at 08:04:02PM +0200, Andrea Bolognani wrote:
>Same as virDomainDeviceInfo itself, any struct that
>embeds it needs to be initialized properly before use;
>however, none of the structs in question even had a
>proper allocation function defined.
>
>Implement an allocation function for all structs
>embedding a virDomainDeviceInfo and use them instead
>of plain VIR_ALLOC() everywhere.
>

NACK

This is a pointless obfuscation

Jan

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 09/10] conf: Provide missing virDomain*DefNew() functions
Posted by Andrea Bolognani 7 years, 10 months ago
On Fri, 2017-06-30 at 10:18 +0200, Ján Tomko wrote:
> > Same as virDomainDeviceInfo itself, any struct that
> > embeds it needs to be initialized properly before use;
> > however, none of the structs in question even had a
> > proper allocation function defined.
> > 
> > Implement an allocation function for all structs
> > embedding a virDomainDeviceInfo and use them instead
> > of plain VIR_ALLOC() everywhere.
> 
> NACK
> 
> This is a pointless obfuscation

Would you mind spending a few words to explain why you feel
that's the case?

Having a function where you perform both allocation and
initialization of your data structure is a pretty common
pattern both outside and inside libvirt, and while it adds
one layer of indirection it also improves flexibility and
encapsulation, which makes it IMHO well worth it.

-- 
Andrea Bolognani / Red Hat / Virtualization

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 09/10] conf: Provide missing virDomain*DefNew() functions
Posted by Ján Tomko 7 years, 10 months ago
On Fri, Jun 30, 2017 at 12:34:15PM +0200, Andrea Bolognani wrote:
>On Fri, 2017-06-30 at 10:18 +0200, Ján Tomko wrote:
>> > Same as virDomainDeviceInfo itself, any struct that
>> > embeds it needs to be initialized properly before use;
>> > however, none of the structs in question even had a
>> > proper allocation function defined.
>> > 
>> > Implement an allocation function for all structs
>> > embedding a virDomainDeviceInfo and use them instead
>> > of plain VIR_ALLOC() everywhere.
>> 
>> NACK
>> 
>> This is a pointless obfuscation
>
>Would you mind spending a few words to explain why you feel
>that's the case?
>
>Having a function where you perform both allocation and
>initialization of your data structure is a pretty common

As of now, nothing special is initialized in the structure.
If you are going to need initialize something in the future,
please mention it also in the commit message, not just the
cover letter. This makes the obfuscation pointful.

>pattern both outside and inside libvirt, and while it adds
>one layer of indirection it also improves flexibility and
>encapsulation, which makes it IMHO well worth it.

I am not a fan of adding code just in case we might need it
in the future, which is what this patch looks like to a lazy
reviewer that does not read cover letters for cleanups/refactors.

Jan
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 09/10] conf: Provide missing virDomain*DefNew() functions
Posted by Peter Krempa 7 years, 10 months ago
On Thu, Jun 29, 2017 at 20:04:02 +0200, Andrea Bolognani wrote:
> Same as virDomainDeviceInfo itself, any struct that
> embeds it needs to be initialized properly before use;
> however, none of the structs in question even had a
> proper allocation function defined.
> 
> Implement an allocation function for all structs
> embedding a virDomainDeviceInfo and use them instead
> of plain VIR_ALLOC() everywhere.
> 
> Signed-off-by: Andrea Bolognani <abologna@redhat.com>
> ---
>  src/bhyve/bhyve_parse_command.c |   4 +-
>  src/conf/domain_conf.c          | 280 +++++++++++++++++++++++++++++++---------
>  src/conf/domain_conf.h          |  15 +++
>  src/libvirt_private.syms        |  11 ++
>  src/openvz/openvz_conf.c        |   2 +-
>  src/qemu/qemu_command.c         |  12 +-
>  src/qemu/qemu_domain.c          |  11 +-
>  src/qemu/qemu_domain_address.c  |   2 +-
>  src/qemu/qemu_hotplug.c         |   5 +-
>  src/qemu/qemu_parse_command.c   |  27 ++--
>  src/vbox/vbox_common.c          |  12 +-
>  src/vmx/vmx.c                   |   2 +-
>  src/vz/vz_sdk.c                 |   6 +-
>  src/xen/xen_driver.c            |   2 +-
>  src/xenapi/xenapi_driver.c      |   2 +-
>  src/xenconfig/xen_common.c      |   2 +-
>  src/xenconfig/xen_sxpr.c        |   8 +-
>  src/xenconfig/xen_xl.c          |   2 +-
>  src/xenconfig/xen_xm.c          |   2 +-
>  19 files changed, 303 insertions(+), 104 deletions(-)

While I agree that having allocation functions which initialize the
internals are necessary in some cases, this patch is a mess as you
squashed everything into one place.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list