3:24 a.m.
On Tue, Apr 14, 2015 at 18:05:11 +0200, Peter Krempa wrote:
Among all the monitor APIs some where checking if mon is NULL and
some
were not. Since it's possible to have mon equal to NULL in case a second
call is attempted once entered the monitor. This requires that every
single API checks for the monitor.
This patch adds a macro that helps checking the state of the monitor and
either refactors existing checking code to use the macro or adds it in
case it was missing.
---
src/qemu/qemu_monitor.c | 918 +++++++++---------------------------------------
src/qemu/qemu_monitor.h | 18 +-
2 files changed, 174 insertions(+), 762 deletions(-)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index d5bdfb8..e5630f1 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -98,6 +98,32 @@ struct _qemuMonitor {
int logfd;
};
+/**
+ * QEMU_CHECK_MONITOR_FULL:
+ * @MON: monitor pointer variable to check, evaluated multiple times, no parentheses
+ * @FORCE_JSON: force JSON monitor, true or false
+ * @EXIT: statement that is used to exit the function
+ *
+ * This macro checks that the monitor is valid for given operation and exits
+ * the function if not. The macro also adds a debug statement regarding the
+ * monitor.
+ */
+#define QEMU_CHECK_MONITOR_FULL(MON, FORCE_JSON, EXIT) \
The uppercase parameters look ugly, I think we use lowercase everywhere.
+ if (!MON) {
\
+ virReportError(VIR_ERR_INVALID_ARG, "%s",
\
+ _("Monitor must not be NULL"));
\
s/Monitor/monitor/
+ EXIT;
\
+ } \
+ VIR_DEBUG("Mon:%p vm:%p json:%d fd:%d", MON, MON->vm, MON->json,
MON->fd); \
s/Mon/mon/
+ if (FORCE_JSON && !MON->json) {
\
+ virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
\
+ _("JSON monitor is required"));
\
+ EXIT; \
+ }
+
+#define QEMU_CHECK_MONITOR(MON) QEMU_CHECK_MONITOR_FULL(MON, false, return -1)
+#define QEMU_CHECK_MONITOR_JSON(MON) QEMU_CHECK_MONITOR_FULL(MON, true, return -1)
I think it would be nice to add a few more macros so that the FULL
variant does not have to be used anywhere. Especially having to use
true/false instead of QEMU_CHECK_MONITOR{_JSON,} is less obvious :-)
+
static virClassPtr qemuMonitorClass;
static void qemuMonitorDispose(void *obj);
...
@@ -1661,11 +1651,7 @@ qemuMonitorSetLink(qemuMonitorPtr mon,
{
VIR_DEBUG("mon=%p, name=%s, state=%u", mon, name, state);
Should we remove "mon=%p" from this (and other similar) VIR_DEBUG since
it will be logged by QEMU_CHECK_MONITOR?
- if (!mon) {
- virReportError(VIR_ERR_INVALID_ARG, "%s",
- _("monitor must not be NULL"));
- return -1;
- }
+ QEMU_CHECK_MONITOR(mon);
if (mon->json)
return qemuMonitorJSONSetLink(mon, name, state);
...
@@ -3763,44 +3423,23 @@ qemuMonitorOpenGraphics(qemuMonitorPtr mon,
int
qemuMonitorSystemWakeup(qemuMonitorPtr mon)
{
- VIR_DEBUG("mon=%p", mon);
-
- if (!mon) {
- virReportError(VIR_ERR_INVALID_ARG, "%s",
- _("monitor must not be NULL"));
- return -1;
- }
-
- if (!mon->json) {
- virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
- _("JSON monitor is required"));
- return -1;
- }
+ QEMU_CHECK_MONITOR_JSON(mon);
return qemuMonitorJSONSystemWakeup(mon);
}
-int qemuMonitorGetVersion(qemuMonitorPtr mon,
- int *major,
- int *minor,
- int *micro,
- char **package)
+int
+qemuMonitorGetVersion(qemuMonitorPtr mon,
+ int *major,
+ int *minor,
+ int *micro,
+ char **package)
This change should go to the first patch in this series.
{
VIR_DEBUG("mon=%p major=%p minor=%p micro=%p package=%p",
mon, major, minor, micro, package);
- if (!mon) {
- virReportError(VIR_ERR_INVALID_ARG, "%s",
- _("monitor must not be NULL"));
- return -1;
- }
-
- if (!mon->json) {
- virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
- _("JSON monitor is required"));
- return -1;
- }
+ QEMU_CHECK_MONITOR_JSON(mon);
return qemuMonitorJSONGetVersion(mon, major, minor, micro, package);
}
...
Jirka