Re: [libvirt] [PATCH 2/7] security: Use virDomainSeclabelDefClear

12 Jan 2011

On 01/12/2011 10:22 AM, Cole Robinson wrote:
...
Renamed from virSecurityLabelDefFree.
I like the rename, but...
...
Signed-off-by: Cole Robinson <crobinso@redhat.com>
---
 cfg.mk                           |    1 +
 src/conf/domain_conf.c           |   17 +++++++++--------
 src/conf/domain_conf.h           |    1 +
 src/libvirt_private.syms         |    2 +-
 src/qemu/qemu_driver.c           |    4 +---
 src/security/security_apparmor.c |   11 ++---------
 src/security/security_selinux.c  |    9 ++-------
 7 files changed, 17 insertions(+), 28 deletions(-)

diff --git a/cfg.mk b/cfg.mk
index 03186b3..2c6f595 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -98,6 +98,7 @@ useless_free_options =				\
   --name=virDomainInputDefFree			\
   --name=virDomainNetDefFree			\
   --name=virDomainObjFree			\
+  --name=virDomainSeclabelDefClear		\
This is wrong.  It is _not_ a free-like function, because it does not
free the passed in pointer.  You probably want to have two functions:

/* clear guts only, must be given valid pointer */
void virDomainSeclabelDefClear(ptr) ATTRIBUTE_NONNULL(1);

/* clear entire allocated pointer, free-like */
void virDomainSeclabelDefFree(ptr) {
  if (!ptr) return;
  virDomainSeclabelDefClear(ptr);
  VIR_FREE(ptr);
}

and list only virDomainSeclabelDefFree in cfg.mk.
...
-void virSecurityLabelDefFree(virDomainDefPtr def)
+void virDomainSeclabelDefClear(virSecurityLabelDefPtr seclabel)
 {
-    VIR_FREE(def->seclabel.model);
-    VIR_FREE(def->seclabel.label);
-    VIR_FREE(def->seclabel.imagelabel);
+    if (!seclabel)
+        return;
which means this early exit should only be part of the Free variant, not
the Clear variant.
...
+
+    VIR_FREE(seclabel->model);
+    VIR_FREE(seclabel->label);
+    VIR_FREE(seclabel->imagelabel);
 }
static void
@@ -855,7 +856,7 @@ void virDomainDefFree(virDomainDefPtr def)
virDomainMemballoonDefFree(def->memballoon);
-    virSecurityLabelDefFree(def);
+    virDomainSeclabelDefClear(&def->seclabel);
But hunks like this are okay (when a SeclabelDef occurs as an inline
struct member, rather than an indirect SeclabelDefPtr pointer).
...
+++ b/src/libvirt_private.syms
@@ -279,6 +279,7 @@ virDomainRemoveInactive;
 virDomainSaveConfig;
 virDomainSaveStatus;
 virDomainSaveXML;
+virDomainSeclabelDefClear;
 virDomainSnapshotAssignDef;
 virDomainSnapshotDefFormat;
 virDomainSnapshotDefFree;
@@ -313,7 +314,6 @@ virDomainWatchdogActionTypeToString;
 virDomainWatchdogModelTypeFromString;
 virDomainWatchdogModelTypeToString;
-
 # domain_event.h
Why the whitespace change?

-- 
Eric Blake   eblake@redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

    