[libvirt RFC PATCH 1/5] conf: rename and narrow scope of virDomainHostdevDefClear()

Laine Stump laine at redhat.com
Fri Feb 12 05:54:03 UTC 2021


This function had the name "Clear", but some of its fields were being
cleared, and others were just being freed, but the pointers not
cleared. In fact, all the uses of virDomainHostdevDefClear() ended up
freeing the memory containing the HosdevDef immediately after clearing
it.

So it is acceptable to change the VIR_FREEs in this function to
g_frees, but that would be confusing to future me, who will have
forgotten that a "cleared" hostdevdef is never re-used. In order to
eliminate the confusion, we can rename the function to
virDomainHostdevDefFreeContents(), which is more accurate (doesn't
make false claims).

In the meantime, this function was only ever called from other
functions within the same file, so there is no need for it to be
declared in domain_conf.h, much less be listed among the exported
functions in libvirt_private.syms, so lets just make this renamed
function static for efficiency's sake.

Signed-off-by: Laine Stump <laine at redhat.com>
---
 src/conf/domain_conf.c   | 19 +++++++++++--------
 src/conf/domain_conf.h   |  1 -
 src/libvirt_private.syms |  1 -
 3 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 17e0d1ed31..3b9d0da232 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -1339,6 +1339,7 @@ static virClassPtr virDomainXMLOptionClass;
 static void virDomainObjDispose(void *obj);
 static void virDomainXMLOptionDispose(void *obj);
 
+static void virDomainHostdevDefFreeContents(virDomainHostdevDefPtr def);
 
 static void
 virDomainChrSourceDefFormat(virBufferPtr buf,
@@ -2430,7 +2431,7 @@ virDomainActualNetDefFree(virDomainActualNetDefPtr def)
         g_free(def->data.direct.linkdev);
         break;
     case VIR_DOMAIN_NET_TYPE_HOSTDEV:
-        virDomainHostdevDefClear(&def->data.hostdev.def);
+        virDomainHostdevDefFree(&def->data.hostdev.def);
         break;
     default:
         break;
@@ -2531,7 +2532,7 @@ virDomainNetDefFree(virDomainNetDefPtr def)
         break;
 
     case VIR_DOMAIN_NET_TYPE_HOSTDEV:
-        virDomainHostdevDefClear(&def->data.hostdev.def);
+        virDomainHostdevDefFree(&def->data.hostdev.def);
         break;
 
     case VIR_DOMAIN_NET_TYPE_ETHERNET:
@@ -3009,7 +3010,8 @@ virDomainHostdevSubsysSCSIClear(virDomainHostdevSubsysSCSIPtr scsisrc)
 }
 
 
-void virDomainHostdevDefClear(virDomainHostdevDefPtr def)
+static void
+virDomainHostdevDefFreeContents(virDomainHostdevDefPtr def)
 {
     if (!def)
         return;
@@ -3030,13 +3032,13 @@ void virDomainHostdevDefClear(virDomainHostdevDefPtr def)
     case VIR_DOMAIN_HOSTDEV_MODE_CAPABILITIES:
         switch ((virDomainHostdevCapsType) def->source.caps.type) {
         case VIR_DOMAIN_HOSTDEV_CAPS_TYPE_STORAGE:
-            VIR_FREE(def->source.caps.u.storage.block);
+            g_free(def->source.caps.u.storage.block);
             break;
         case VIR_DOMAIN_HOSTDEV_CAPS_TYPE_MISC:
-            VIR_FREE(def->source.caps.u.misc.chardev);
+            g_free(def->source.caps.u.misc.chardev);
             break;
         case VIR_DOMAIN_HOSTDEV_CAPS_TYPE_NET:
-            VIR_FREE(def->source.caps.u.net.ifname);
+            g_free(def->source.caps.u.net.ifname);
             virNetDevIPInfoClear(&def->source.caps.u.net.ip);
             break;
         case VIR_DOMAIN_HOSTDEV_CAPS_TYPE_LAST:
@@ -3049,7 +3051,7 @@ void virDomainHostdevDefClear(virDomainHostdevDefPtr def)
             virDomainHostdevSubsysSCSIClear(&def->source.subsys.u.scsi);
             break;
         case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI_HOST:
-            VIR_FREE(def->source.subsys.u.scsi_host.wwpn);
+            g_free(def->source.subsys.u.scsi_host.wwpn);
             break;
         case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB:
         case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI:
@@ -3061,6 +3063,7 @@ void virDomainHostdevDefClear(virDomainHostdevDefPtr def)
     }
 }
 
+
 void virDomainTPMDefFree(virDomainTPMDefPtr def)
 {
     if (!def)
@@ -3089,7 +3092,7 @@ void virDomainHostdevDefFree(virDomainHostdevDefPtr def)
         return;
 
     /* free all subordinate objects */
-    virDomainHostdevDefClear(def);
+    virDomainHostdevDefFreeContents(def);
 
     /* If there is a parentnet device object, it will handle freeing
      * the memory.
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index e263e6098b..415826134d 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -3126,7 +3126,6 @@ void virDomainVideoDefFree(virDomainVideoDefPtr def);
 G_DEFINE_AUTOPTR_CLEANUP_FUNC(virDomainVideoDef, virDomainVideoDefFree);
 void virDomainVideoDefClear(virDomainVideoDefPtr def);
 virDomainHostdevDefPtr virDomainHostdevDefNew(void);
-void virDomainHostdevDefClear(virDomainHostdevDefPtr def);
 void virDomainHostdevDefFree(virDomainHostdevDefPtr def);
 void virDomainHubDefFree(virDomainHubDefPtr def);
 void virDomainRedirdevDefFree(virDomainRedirdevDefPtr def);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 49e665a5f0..e586263a61 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -445,7 +445,6 @@ virDomainGraphicsVNCSharePolicyTypeFromString;
 virDomainGraphicsVNCSharePolicyTypeToString;
 virDomainHasNet;
 virDomainHostdevCapsTypeToString;
-virDomainHostdevDefClear;
 virDomainHostdevDefFree;
 virDomainHostdevDefNew;
 virDomainHostdevFind;
-- 
2.29.2




More information about the libvir-list mailing list