2011/3/18 Eric Blake <ebl...@redhat.com>: > The next patch will change reference counting idioms; consolidating > this pattern now makes the next patch smaller (touch only the new > macro rather than every caller). > > * src/qemu/qemu_monitor.c (QEMU_MONITOR_CALLBACK): New helper. > (qemuMonitorGetDiskSecret, qemuMonitorEmitShutdown) > (qemuMonitorEmitReset, qemuMonitorEmitPowerdown) > (qemuMonitorEmitStop, qemuMonitorEmitRTCChange) > (qemuMonitorEmitWatchdog, qemuMonitorEmitIOError) > (qemuMonitorEmitGraphics): Use it to reduce duplication. > --- > >> > We should probably annotate qemuMonitorUnref() (and similar functions) >> > with ATTRIBUTE_RETURN_CHECK too > >> I'm working on that now... > > And here we go; this exercise didn't find any more unsafe instances. > > src/qemu/qemu_monitor.c | 80 > +++++++++++++---------------------------------- > 1 files changed, 22 insertions(+), 58 deletions(-) > > diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c > index dc08594..fd02ca0 100644 > --- a/src/qemu/qemu_monitor.c > +++ b/src/qemu/qemu_monitor.c > @@ -754,6 +754,15 @@ int qemuMonitorHMPCommandWithFd(qemuMonitorPtr mon, > return qemuMonitorTextCommandWithFd(mon, cmd, scm_fd, reply); > } > > +/* Ensure proper locking around callbacks; assumes mon and ret are in > + * scope. */ > +#define QEMU_MONITOR_CALLBACK(callback, ...) \ > + qemuMonitorRef(mon); \ > + qemuMonitorUnlock(mon); \ > + if (mon->cb && mon->cb->callback) \ > + ret = (mon->cb->callback)(mon, __VA_ARGS__); \ > + qemuMonitorLock(mon); \ > + qemuMonitorUnref(mon) >
Shouldn't this be #define QEMU_MONITOR_CALLBACK(callback, ...) \ do { \ ... \ } while (0) just to ensure that doing things like if (...) QEMU_MONITOR_CALLBACK(domainWatchdog, mon->vm, action); is safe? Because this one will break with the current form of QEMU_MONITOR_CALLBACK. Another nit on readability. The macro hides the conditional assignment of ret, but turning it into ret = QEMU_MONITOR_CALLBACK(...) is not that simple. ACK, with that do {} while (0) problem addressed. Matthias -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list