[Qemu-devel] [PATCH v2.1 02.5/11] monitor: Restrict use of Monitor member qmp to actual QMP monitors

[Markus Armbruster]

From: Kevin Wolf <address@hidden>

We currently use Monitor member qmp even in HMP monitors.  Harmless,
but it's in the next commit's way.  Restrict its use to QMP monitors.

Several functions have a tacit "is a QMP monitor" precondition.  Add
explicit assertions there.  The next commit will replace most of them
by compile-time type checks.

Signed-off-by: Kevin Wolf <address@hidden>
Signed-off-by: Markus Armbruster <address@hidden>
---
 monitor.c | 34 ++++++++++++++++++++++++++++------
 1 file changed, 28 insertions(+), 6 deletions(-)

diff --git a/monitor.c b/monitor.c
index bb23cc0450..0e145959d7 100644
--- a/monitor.c
+++ b/monitor.c
@@ -357,6 +357,8 @@ static void qmp_request_free(QMPRequest *req)
 /* Caller must hold mon->qmp.qmp_queue_lock */
 static void monitor_qmp_cleanup_req_queue_locked(Monitor *mon)
 {
+    assert(monitor_is_qmp(mon));
+
     while (!g_queue_is_empty(mon->qmp.qmp_requests)) {
         qmp_request_free(g_queue_pop_head(mon->qmp.qmp_requests));
     }
@@ -364,6 +366,8 @@ static void monitor_qmp_cleanup_req_queue_locked(Monitor 
*mon)
 
 static void monitor_qmp_cleanup_queues(Monitor *mon)
 {
+    assert(monitor_is_qmp(mon));
+
     qemu_mutex_lock(&mon->qmp.qmp_queue_lock);
     monitor_qmp_cleanup_req_queue_locked(mon);
     qemu_mutex_unlock(&mon->qmp.qmp_queue_lock);
@@ -710,29 +714,32 @@ static void monitor_data_init(Monitor *mon, int flags, 
bool skip_flush,
     }
     memset(mon, 0, sizeof(Monitor));
     qemu_mutex_init(&mon->mon_lock);
-    qemu_mutex_init(&mon->qmp.qmp_queue_lock);
     mon->outbuf = qstring_new();
     /* Use *mon_cmds by default. */
     mon->cmd_table = mon_cmds;
     mon->skip_flush = skip_flush;
     mon->use_io_thread = use_io_thread;
-    mon->qmp.qmp_requests = g_queue_new();
     mon->flags = flags;
 }
 
+static void monitor_data_destroy_qmp(Monitor *mon)
+{
+    json_message_parser_destroy(&mon->qmp.parser);
+    qemu_mutex_destroy(&mon->qmp.qmp_queue_lock);
+    monitor_qmp_cleanup_req_queue_locked(mon);
+    g_queue_free(mon->qmp.qmp_requests);
+}
+
 static void monitor_data_destroy(Monitor *mon)
 {
     g_free(mon->mon_cpu_path);
     qemu_chr_fe_deinit(&mon->chr, false);
     if (monitor_is_qmp(mon)) {
-        json_message_parser_destroy(&mon->qmp.parser);
+        monitor_data_destroy_qmp(mon);
     }
     readline_free(mon->rs);
     qobject_unref(mon->outbuf);
     qemu_mutex_destroy(&mon->mon_lock);
-    qemu_mutex_destroy(&mon->qmp.qmp_queue_lock);
-    monitor_qmp_cleanup_req_queue_locked(mon);
-    g_queue_free(mon->qmp.qmp_requests);
 }
 
 char *qmp_human_monitor_command(const char *command_line, bool has_cpu_index,
@@ -1086,6 +1093,7 @@ CommandInfoList *qmp_query_commands(Error **errp)
 {
     CommandInfoList *list = NULL;
 
+    assert(monitor_is_qmp(cur_mon));
     qmp_for_each_command(cur_mon->qmp.commands, query_commands_cb, &list);
 
     return list;
@@ -1155,11 +1163,13 @@ static void monitor_init_qmp_commands(void)
 
 static bool qmp_oob_enabled(Monitor *mon)
 {
+    assert(monitor_is_qmp(mon));
     return mon->qmp.capab[QMP_CAPABILITY_OOB];
 }
 
 static void monitor_qmp_caps_reset(Monitor *mon)
 {
+    assert(monitor_is_qmp(mon));
     memset(mon->qmp.capab_offered, 0, sizeof(mon->qmp.capab_offered));
     memset(mon->qmp.capab, 0, sizeof(mon->qmp.capab));
     mon->qmp.capab_offered[QMP_CAPABILITY_OOB] = mon->use_io_thread;
@@ -1176,6 +1186,7 @@ static bool qmp_caps_accept(Monitor *mon, 
QMPCapabilityList *list,
     GString *unavailable = NULL;
     bool capab[QMP_CAPABILITY__MAX];
 
+    assert(monitor_is_qmp(mon));
     memset(capab, 0, sizeof(capab));
 
     for (; list; list = list->next) {
@@ -1203,6 +1214,7 @@ static bool qmp_caps_accept(Monitor *mon, 
QMPCapabilityList *list,
 void qmp_qmp_capabilities(bool has_enable, QMPCapabilityList *enable,
                           Error **errp)
 {
+    assert(monitor_is_qmp(cur_mon));
     if (cur_mon->qmp.commands == &qmp_commands) {
         error_set(errp, ERROR_CLASS_COMMAND_NOT_FOUND,
                   "Capabilities negotiation is already complete, command "
@@ -4134,6 +4146,7 @@ static void monitor_qmp_dispatch(Monitor *mon, QObject 
*req)
     QDict *rsp;
     QDict *error;
 
+    assert(monitor_is_qmp(mon));
     old_mon = cur_mon;
     cur_mon = mon;
 
@@ -4177,6 +4190,9 @@ static QMPRequest 
*monitor_qmp_requests_pop_any_with_lock(void)
     qemu_mutex_lock(&monitor_lock);
 
     QTAILQ_FOREACH(mon, &mon_list, entry) {
+        if (!monitor_is_qmp(mon)) {
+            continue;
+        }
         qemu_mutex_lock(&mon->qmp.qmp_queue_lock);
         req_obj = g_queue_pop_head(mon->qmp.qmp_requests);
         if (req_obj) {
@@ -4212,6 +4228,7 @@ static void monitor_qmp_bh_dispatcher(void *data)
     }
 
     mon = req_obj->mon;
+    assert(monitor_is_qmp(mon));
     /*  qmp_oob_enabled() might change after "qmp_capabilities" */
     need_resume = !qmp_oob_enabled(mon) ||
         mon->qmp.qmp_requests->length == QMP_REQ_QUEUE_LEN_MAX - 1;
@@ -4395,6 +4412,8 @@ static QDict *qmp_greeting(Monitor *mon)
     QObject *ver = NULL;
     QMPCapability cap;
 
+    assert(monitor_is_qmp(mon));
+
     qmp_marshal_query_version(NULL, &ver, NULL);
 
     for (cap = 0; cap < QMP_CAPABILITY__MAX; cap++) {
@@ -4612,6 +4631,9 @@ static void monitor_init_qmp(Chardev *chr, int flags)
     monitor_data_init(mon, flags, false,
                       qemu_chr_has_feature(chr, QEMU_CHAR_FEATURE_GCONTEXT));
 
+    qemu_mutex_init(&mon->qmp.qmp_queue_lock);
+    mon->qmp.qmp_requests = g_queue_new();
+
     qemu_chr_fe_init(&mon->chr, chr, &error_abort);
     qemu_chr_fe_set_echo(&mon->chr, true);
 
-- 
2.21.0