[libvirt] [PATCH 2/2] build: enforce reference count checking
Laine Stump
laine at laine.org
Thu Mar 24 20:43:39 UTC 2011
On 03/18/2011 01:37 PM, Eric Blake wrote:
> 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.
> ---
> src/conf/domain_conf.c | 6 ++++--
> src/conf/domain_conf.h | 2 +-
> src/openvz/openvz_conf.c | 6 ++++--
> src/qemu/qemu_domain.c | 11 ++++++++---
> src/qemu/qemu_monitor.c | 2 +-
> src/qemu/qemu_monitor.h | 2 +-
> src/qemu/qemu_process.c | 3 ++-
> src/vmware/vmware_conf.c | 7 +++++--
> 8 files changed, 26 insertions(+), 13 deletions(-)
ACK.
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 1b02c25..b681dc3 100644
> --- 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 9f595d6..1e8223f 100644
> --- 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 0eb5ab3..b9e1e7e 100644
> --- 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 cc137d2..c2a1f9a 100644
> --- 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 fd02ca0..0ce1075 100644
> --- a/src/qemu/qemu_monitor.c
> +++ b/src/qemu/qemu_monitor.c
> @@ -762,7 +762,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))
>
> int qemuMonitorGetDiskSecret(qemuMonitorPtr mon,
> virConnectPtr conn,
> diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
> index 7bea083..f9d3233 100644
> --- a/src/qemu/qemu_monitor.h
> +++ b/src/qemu/qemu_monitor.h
> @@ -131,7 +131,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 740684a..ac504f5 100644
> --- 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 c3f53ea..6339248 100644
> --- 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;
> }
>
More information about the libvir-list
mailing list