build: enforce reference count checking

Add the compiler attribute to ensure we don't introduce any more
ref bugs like were just patched in commit 9741f34, then explicitly
mark the remaining places in code that are safe.

* src/qemu/qemu_monitor.h (qemuMonitorUnref): Mark
ATTRIBUTE_RETURN_CHECK.
* src/conf/domain_conf.h (virDomainObjUnref): Likewise.
* src/conf/domain_conf.c (virDomainObjParseXML)
(virDomainLoadStatus): Fix offenders.
* src/openvz/openvz_conf.c (openvzLoadDomains): Likewise.
* src/vmware/vmware_conf.c (vmwareLoadDomains): Likewise.
* src/qemu/qemu_domain.c (qemuDomainObjBeginJob)
(qemuDomainObjBeginJobWithDriver)
(qemuDomainObjExitRemoteWithDriver): Likewise.
* src/qemu/qemu_monitor.c (QEMU_MONITOR_CALLBACK): Likewise.
Suggested by Daniel P. Berrange.

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 1b02c25bb278526604786cbbcf403926781b2483..b681dc3464d948f2ecea41a605864e0d0518ec0b 100644 (file)
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -6015,7 +6015,8 @@ static virDomainObjPtr virDomainObjParseXML(virCapsPtr caps,
     return obj;
 
 error:
-    virDomainObjUnref(obj);
+    /* obj was never shared, so unref should return 0 */
+    ignore_value(virDomainObjUnref(obj));
     return NULL;
 }
 
@@ -8220,8 +8221,9 @@ static virDomainObjPtr virDomainLoadStatus(virCapsPtr caps,
     return obj;
 
 error:
+    /* obj was never shared, so unref should return 0 */
     if (obj)
-        virDomainObjUnref(obj);
+        ignore_value(virDomainObjUnref(obj));
     VIR_FREE(statusFile);
     return NULL;
 }

diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 9f595d6d9c7b69f9cc9e14210cf648925657e704..1e8223f5d966cf2a3ac7c5f6fb585d6de5801471 100644 (file)
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1206,7 +1206,7 @@ int virDomainDeviceInfoIterate(virDomainDefPtr def,
 void virDomainDefFree(virDomainDefPtr vm);
 void virDomainObjRef(virDomainObjPtr vm);
 /* Returns 1 if the object was freed, 0 if more refs exist */
-int virDomainObjUnref(virDomainObjPtr vm);
+int virDomainObjUnref(virDomainObjPtr vm) ATTRIBUTE_RETURN_CHECK;
 
 /* live == true means def describes an active domain (being migrated or
  * restored) as opposed to a new persistent configuration of the domain */

diff --git a/src/openvz/openvz_conf.c b/src/openvz/openvz_conf.c
index 0eb5ab30755b869e52cc0a546b2bb92a3d8ea579..2fcca68caee431e9f0a0f83988cb2a357c77ea35 100644 (file)
--- a/src/openvz/openvz_conf.c
+++ b/src/openvz/openvz_conf.c
@@ -1,7 +1,7 @@
 /*
  * openvz_conf.c: config functions for managing OpenVZ VEs
  *
- * Copyright (C) 2010 Red Hat, Inc.
+ * Copyright (C) 2010-2011 Red Hat, Inc.
  * Copyright (C) 2006, 2007 Binary Karma
  * Copyright (C) 2006 Shuveb Hussain
  * Copyright (C) 2007 Anoop Joe Cyriac
@@ -52,6 +52,7 @@
 #include "nodeinfo.h"
 #include "files.h"
 #include "command.h"
+#include "ignore-value.h"
 
 #define VIR_FROM_THIS VIR_FROM_OPENVZ
 
@@ -543,8 +544,9 @@ int openvzLoadDomains(struct openvz_driver *driver) {
  cleanup:
     virCommandFree(cmd);
     VIR_FREE(outbuf);
+    /* dom hasn't been shared yet, so unref should return 0 */
     if (dom)
-        virDomainObjUnref(dom);
+        ignore_value(virDomainObjUnref(dom));
     return -1;
 }
 

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index cc137d2b5816caeba1f0abe45ba80772e1fd2280..c2a1f9a0ad115909f94e95fa1e19491d658d87c4 100644 (file)
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -31,6 +31,7 @@
 #include "c-ctype.h"
 #include "event.h"
 #include "cpu/cpu.h"
+#include "ignore-value.h"
 
 #include <sys/time.h>
 
@@ -460,7 +461,8 @@ int qemuDomainObjBeginJob(virDomainObjPtr obj)
 
     while (priv->jobActive) {
         if (virCondWaitUntil(&priv->jobCond, &obj->lock, then) < 0) {
-            virDomainObjUnref(obj);
+            /* Safe to ignore value since ref count was incremented above */
+            ignore_value(virDomainObjUnref(obj));
             if (errno == ETIMEDOUT)
                 qemuReportError(VIR_ERR_OPERATION_TIMEOUT,
                                 "%s", _("cannot acquire state change lock"));
@@ -504,7 +506,8 @@ int qemuDomainObjBeginJobWithDriver(struct qemud_driver *driver,
 
     while (priv->jobActive) {
         if (virCondWaitUntil(&priv->jobCond, &obj->lock, then) < 0) {
-            virDomainObjUnref(obj);
+            /* Safe to ignore value since ref count was incremented above */
+            ignore_value(virDomainObjUnref(obj));
             if (errno == ETIMEDOUT)
                 qemuReportError(VIR_ERR_OPERATION_TIMEOUT,
                                 "%s", _("cannot acquire state change lock"));
@@ -650,7 +653,9 @@ void qemuDomainObjExitRemoteWithDriver(struct qemud_driver *driver,
 {
     qemuDriverLock(driver);
     virDomainObjLock(obj);
-    virDomainObjUnref(obj);
+    /* Safe to ignore value, since we incremented ref in
+     * qemuDomainObjEnterRemoteWithDriver */
+    ignore_value(virDomainObjUnref(obj));
 }
 
 

diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 4ccadcef2931c48dd7850a30bc644d39d63446a1..800f7444bfab5be095d74c751619eb563d6f189c 100644 (file)
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -763,7 +763,7 @@ int qemuMonitorHMPCommandWithFd(qemuMonitorPtr mon,
         if ((mon)->cb && (mon)->cb->callback)                   \
             (ret) = ((mon)->cb->callback)(mon, __VA_ARGS__);    \
         qemuMonitorLock(mon);                                   \
-        qemuMonitorUnref(mon);                                  \
+        ignore_value(qemuMonitorUnref(mon));                    \
     } while (0)
 
 int qemuMonitorGetDiskSecret(qemuMonitorPtr mon,

diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index 1566a6d268b34ac76ce441f81ae8ead5d9c442d2..c90219b675f9a1dc9baf493d65477d97f4572c82 100644 (file)
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -133,7 +133,7 @@ void qemuMonitorLock(qemuMonitorPtr mon);
 void qemuMonitorUnlock(qemuMonitorPtr mon);
 
 int qemuMonitorRef(qemuMonitorPtr mon);
-int qemuMonitorUnref(qemuMonitorPtr mon);
+int qemuMonitorUnref(qemuMonitorPtr mon) ATTRIBUTE_RETURN_CHECK;
 
 /* These APIs are for use by the internal Text/JSON monitor impl code only */
 int qemuMonitorSend(qemuMonitorPtr mon,

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 7831c3b3bfb66483e0d67c1e03fcae4a6db3e1fb..76cceadc015eeb88b82cbd3a1ac7a1b0df13d7b0 100644 (file)
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -643,8 +643,9 @@ qemuConnectMonitor(struct qemud_driver *driver, virDomainObjPtr vm)
                                 priv->monJSON,
                                 &monitorCallbacks);
 
+    /* Safe to ignore value since ref count was incremented above */
     if (priv->mon == NULL)
-        virDomainObjUnref(vm);
+        ignore_value(virDomainObjUnref(vm));
 
     if (virSecurityManagerClearSocketLabel(driver->securityManager, vm) < 0) {
         VIR_ERROR(_("Failed to clear security context for monitor for %s"),

diff --git a/src/vmware/vmware_conf.c b/src/vmware/vmware_conf.c
index c3f53ea9768a289c7234286f040054f25bda8689..6339248a9ed709da625730d3473e8a4b94a535ad 100644 (file)
--- a/src/vmware/vmware_conf.c
+++ b/src/vmware/vmware_conf.c
@@ -1,5 +1,7 @@
 /*---------------------------------------------------------------------------*/
-/* Copyright 2010, diateam (www.diateam.net)
+/*
+ * Copyright (C) 2011 Red Hat, Inc.
+ * Copyright 2010, diateam (www.diateam.net)
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Lesser General Public
@@ -201,8 +203,9 @@ cleanup:
     VIR_FREE(directoryName);
     VIR_FREE(fileName);
     VIR_FREE(vmx);
+    /* any non-NULL vm here has not been shared, so unref will return 0 */
     if (vm)
-        virDomainObjUnref(vm);
+        ignore_value(virDomainObjUnref(vm));
     return ret;
 }