[Spice-devel] [RFC PATCH spice-server v4 01/22] Avoid to use global variable for channel IDs

[Frediano Ziglio]

This patch allocate VMC IDs finding the first ID not used
instead of using a global variable and incrementing the value
for each channel created.
This solve some potential issues:
- remove the global state potentially making possible
  to use multiple SpiceServer on the same process;
- avoid to potentially overflow the variable. This can happen
  if channels are allocated/deallocated multiple times
  (currently not done by Qemu).

Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
---
 server/reds.c     | 34 ++++++++++++++++++++++++++++++++++
 server/reds.h     |  1 +
 server/spicevmc.c | 10 ++++++++--
 3 files changed, 43 insertions(+), 2 deletions(-)

diff --git a/server/reds.c b/server/reds.c
index 84c2e466..005ead47 100644
--- a/server/reds.c
+++ b/server/reds.c
@@ -408,6 +408,40 @@ RedChannel *reds_find_channel(RedsState *reds, uint32_t type, uint32_t id)
     return NULL;
 }
 
+/* Search for first free channel id for a specific channel type.
+ * Return first id free or <0 if not found. */
+int reds_get_free_channel_id(RedsState *reds, uint32_t type)
+{
+    RedChannel *channel;
+
+    // this mark if some IDs are used.
+    // Actually used to store boolean values but char take less memory
+    // then gboolean/bool.
+    // The size of the array limit the possible id returned but
+    // usually the IDs used for a channel type are not much.
+    bool used_ids[256];
+
+    unsigned n;
+
+    // mark id used for the specific channel type
+    memset(used_ids, 0, sizeof(used_ids));
+    GLIST_FOREACH(reds->channels, RedChannel, channel) {
+        uint32_t this_type, this_id;
+        g_object_get(channel, "channel-type", &this_type, "id", &this_id, NULL);
+        if (this_type == type && this_id < SPICE_N_ELEMENTS(used_ids)) {
+            used_ids[this_id] = true;
+        }
+    }
+
+    // find first ID not marked as used
+    for (n = 0; n < SPICE_N_ELEMENTS(used_ids); ++n) {
+        if (!used_ids[n]) {
+            return n;
+        }
+    }
+    return -1;
+}
+
 static void reds_mig_cleanup(RedsState *reds)
 {
     if (reds->mig_inprogress) {
diff --git a/server/reds.h b/server/reds.h
index ee5a46c0..4f5fc28c 100644
--- a/server/reds.h
+++ b/server/reds.h
@@ -48,6 +48,7 @@ uint32_t reds_get_mm_time(void);
 void reds_register_channel(RedsState *reds, RedChannel *channel);
 void reds_unregister_channel(RedsState *reds, RedChannel *channel);
 RedChannel *reds_find_channel(RedsState *reds, uint32_t type, uint32_t id);
+int reds_get_free_channel_id(RedsState *reds, uint32_t type);
 SpiceMouseMode reds_get_mouse_mode(RedsState *reds); // used by inputs_channel
 gboolean reds_config_get_agent_mouse(const RedsState *reds); // used by inputs_channel
 int reds_has_vdagent(RedsState *reds); // used by inputs channel
diff --git a/server/spicevmc.c b/server/spicevmc.c
index 9305c9b4..08d9954b 100644
--- a/server/spicevmc.c
+++ b/server/spicevmc.c
@@ -272,7 +272,6 @@ red_vmc_channel_finalize(GObject *object)
 static RedVmcChannel *red_vmc_channel_new(RedsState *reds, uint8_t channel_type)
 {
     GType gtype = G_TYPE_NONE;
-    static uint8_t id[SPICE_END_CHANNEL] = { 0, };
 
     switch (channel_type) {
         case SPICE_CHANNEL_USBREDIR:
@@ -288,11 +287,18 @@ static RedVmcChannel *red_vmc_channel_new(RedsState *reds, uint8_t channel_type)
             g_error("Unsupported channel_type for red_vmc_channel_new(): %u", channel_type);
             return NULL;
     }
+
+    int id = reds_get_free_channel_id(reds, channel_type);
+    if (id < 0) {
+        g_warning("Free ID not found creating new VMC channel");
+        return NULL;
+    }
+
     return g_object_new(gtype,
                         "spice-server", reds,
                         "core-interface", reds_get_core_interface(reds),
                         "channel-type", channel_type,
-                        "id", id[channel_type]++,
+                        "id", id,
                         "handle-acks", FALSE,
                         "migration-flags",
                         (SPICE_MIGRATE_NEED_FLUSH | SPICE_MIGRATE_NEED_DATA_TRANSFER),
-- 
2.13.5