[Spice-devel] [PATCH spice-server v2 2/2] red-channel: Initialize base statistic node creating the object

Frediano Ziglio fziglio at redhat.com
Wed Mar 8 14:55:31 UTC 2017


Avoid usage of not initialized statistic node.
This happened for DisplayChannel which was initializing
statistics in the object constructor.

Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
---
 server/cursor-channel.c  |  4 +++-
 server/cursor-channel.h  |  5 +++--
 server/display-channel.c |  4 +++-
 server/display-channel.h |  3 ++-
 server/red-channel.c     | 24 +++++++++++++++---------
 server/red-channel.h     |  2 --
 server/red-worker.c      | 11 ++++++-----
 server/spicevmc.c        |  4 +++-
 8 files changed, 35 insertions(+), 22 deletions(-)

Changes since v1:
- rebased

diff --git a/server/cursor-channel.c b/server/cursor-channel.c
index 9d21962..48feab7 100644
--- a/server/cursor-channel.c
+++ b/server/cursor-channel.c
@@ -292,7 +292,8 @@ static void cursor_channel_send_item(RedChannelClient *rcc, RedPipeItem *pipe_it
 }
 
 CursorChannel* cursor_channel_new(RedsState *server, QXLInstance *qxl,
-                                  const SpiceCoreInterfaceInternal *core)
+                                  const SpiceCoreInterfaceInternal *core,
+                                  const RedStatNode *stat_node)
 {
     spice_debug("create cursor channel");
     return g_object_new(TYPE_CURSOR_CHANNEL,
@@ -303,6 +304,7 @@ CursorChannel* cursor_channel_new(RedsState *server, QXLInstance *qxl,
                         "migration-flags", 0,
                         "qxl", qxl,
                         "handle-acks", TRUE,
+                        "stat-node", stat_node,
                         NULL);
 }
 
diff --git a/server/cursor-channel.h b/server/cursor-channel.h
index 6bf8486..a7f862e 100644
--- a/server/cursor-channel.h
+++ b/server/cursor-channel.h
@@ -55,8 +55,9 @@ GType cursor_channel_get_type(void) G_GNUC_CONST;
  * provided as helper functions and should only be called from the
  * CursorChannel thread.
  */
-CursorChannel*       cursor_channel_new         (RedsState *server, QXLInstance *qxl,
-                                                 const SpiceCoreInterfaceInternal *core);
+CursorChannel* cursor_channel_new(RedsState *server, QXLInstance *qxl,
+                                  const SpiceCoreInterfaceInternal *core,
+                                  const RedStatNode *stat_node);
 
 /**
  * Cause the channel to disconnect all clients
diff --git a/server/display-channel.c b/server/display-channel.c
index 67a77ef..9d1bbf9 100644
--- a/server/display-channel.c
+++ b/server/display-channel.c
@@ -1987,7 +1987,8 @@ DisplayChannel* display_channel_new(RedsState *reds,
                                     const SpiceCoreInterfaceInternal *core,
                                     int migrate, int stream_video,
                                     GArray *video_codecs,
-                                    uint32_t n_surfaces)
+                                    uint32_t n_surfaces,
+                                    const RedStatNode *stat_node)
 {
     DisplayChannel *display;
 
@@ -2004,6 +2005,7 @@ DisplayChannel* display_channel_new(RedsState *reds,
                            "n-surfaces", n_surfaces,
                            "video-codecs", video_codecs,
                            "handle-acks", TRUE,
+                           "stat-node", stat_node,
                            NULL);
     if (display) {
         display_channel_set_stream_video(display, stream_video);
diff --git a/server/display-channel.h b/server/display-channel.h
index 3d4accf..571685e 100644
--- a/server/display-channel.h
+++ b/server/display-channel.h
@@ -204,7 +204,8 @@ DisplayChannel*            display_channel_new                       (RedsState
                                                                       int migrate,
                                                                       int stream_video,
                                                                       GArray *video_codecs,
-                                                                      uint32_t n_surfaces);
+                                                                      uint32_t n_surfaces,
+                                                                      const RedStatNode *stat_node);
 void                       display_channel_create_surface            (DisplayChannel *display, uint32_t surface_id,
                                                                       uint32_t width, uint32_t height,
                                                                       int32_t stride, uint32_t format, void *line_0,
diff --git a/server/red-channel.c b/server/red-channel.c
index 8e4d582..e0fb7f1 100644
--- a/server/red-channel.c
+++ b/server/red-channel.c
@@ -108,7 +108,8 @@ enum {
     PROP_TYPE,
     PROP_ID,
     PROP_HANDLE_ACKS,
-    PROP_MIGRATION_FLAGS
+    PROP_MIGRATION_FLAGS,
+    PROP_STAT_NODE,
 };
 
 static void
@@ -172,6 +173,11 @@ red_channel_set_property(GObject *object,
         case PROP_MIGRATION_FLAGS:
             self->priv->migration_flags = g_value_get_uint(value);
             break;
+        case PROP_STAT_NODE:
+            if (g_value_get_pointer(value)) {
+                self->priv->stat = *((const RedStatNode *)g_value_get_pointer(value));
+            }
+            break;
         default:
             G_OBJECT_WARN_INVALID_PROPERTY_ID(object, property_id, pspec);
     }
@@ -287,6 +293,14 @@ red_channel_class_init(RedChannelClass *klass)
                              G_PARAM_CONSTRUCT_ONLY |
                              G_PARAM_STATIC_STRINGS);
     g_object_class_install_property(object_class, PROP_MIGRATION_FLAGS, spec);
+
+    spec = g_param_spec_pointer("stat-node",
+                                "stat-node",
+                                "The statistics node associated with this channel",
+                                G_PARAM_WRITABLE |
+                                G_PARAM_CONSTRUCT_ONLY |
+                                G_PARAM_STATIC_STRINGS);
+    g_object_class_install_property(object_class, PROP_STAT_NODE, spec);
 }
 
 static void
@@ -361,14 +375,6 @@ int red_channel_is_waiting_for_migrate_data(RedChannel *channel)
     return red_channel_client_is_waiting_for_migrate_data(rcc);
 }
 
-void red_channel_init_stat_node(RedChannel *channel, const RedStatNode *parent, const char *name)
-{
-    spice_return_if_fail(channel != NULL);
-
-    // TODO check not already initialized
-    stat_init_node(&channel->priv->stat, channel->priv->reds, parent, name, TRUE);
-}
-
 const RedStatNode *red_channel_get_stat_node(RedChannel *channel)
 {
     return &channel->priv->stat;
diff --git a/server/red-channel.h b/server/red-channel.h
index 44282f6..ba1a584 100644
--- a/server/red-channel.h
+++ b/server/red-channel.h
@@ -134,8 +134,6 @@ GType red_channel_get_type(void) G_GNUC_CONST;
 void red_channel_add_client(RedChannel *channel, RedChannelClient *rcc);
 void red_channel_remove_client(RedChannel *channel, RedChannelClient *rcc);
 
-void red_channel_init_stat_node(RedChannel *channel, const RedStatNode *parent, const char *name);
-
 void red_channel_register_client_cbs(RedChannel *channel, const ClientCbs *client_cbs, gpointer cbs_data);
 // caps are freed when the channel is destroyed
 void red_channel_set_common_cap(RedChannel *channel, uint32_t cap);
diff --git a/server/red-worker.c b/server/red-worker.c
index c88034b..2f7a6de 100644
--- a/server/red-worker.c
+++ b/server/red-worker.c
@@ -1310,6 +1310,7 @@ RedWorker* red_worker_new(QXLInstance *qxl,
     Dispatcher *dispatcher;
     RedsState *reds = red_qxl_get_server(qxl->st);
     RedChannel *channel;
+    RedStatNode stat_node;
 
     red_qxl_get_init_info(qxl, &init_info);
 
@@ -1356,21 +1357,21 @@ RedWorker* red_worker_new(QXLInstance *qxl,
 
     worker->event_timeout = INF_EVENT_WAIT;
 
-    worker->cursor_channel = cursor_channel_new(reds, qxl,
-                                                &worker->core);
+    stat_init_node(&stat_node, reds, &worker->stat, "cursor_channel", TRUE);
+    worker->cursor_channel = cursor_channel_new(reds, qxl, &worker->core, &stat_node);
     channel = RED_CHANNEL(worker->cursor_channel);
-    red_channel_init_stat_node(channel, &worker->stat, "cursor_channel");
     red_channel_register_client_cbs(channel, client_cursor_cbs, dispatcher);
     g_object_set_data(G_OBJECT(channel), "dispatcher", dispatcher);
     reds_register_channel(reds, channel);
 
     // TODO: handle seemless migration. Temp, setting migrate to FALSE
+    stat_init_node(&stat_node, reds, &worker->stat, "display_channel", TRUE);
     worker->display_channel = display_channel_new(reds, qxl, &worker->core, FALSE,
                                                   reds_get_streaming_video(reds),
                                                   reds_get_video_codecs(reds),
-                                                  init_info.n_surfaces);
+                                                  init_info.n_surfaces,
+                                                  &stat_node);
     channel = RED_CHANNEL(worker->display_channel);
-    red_channel_init_stat_node(channel, &worker->stat, "display_channel");
     red_channel_register_client_cbs(channel, client_display_cbs, dispatcher);
     g_object_set_data(G_OBJECT(channel), "dispatcher", dispatcher);
     reds_register_channel(reds, channel);
diff --git a/server/spicevmc.c b/server/spicevmc.c
index f61ffc9..60f26ae 100644
--- a/server/spicevmc.c
+++ b/server/spicevmc.c
@@ -233,7 +233,6 @@ red_vmc_channel_constructed(GObject *object)
     client_cbs.connect = spicevmc_connect;
     red_channel_register_client_cbs(RED_CHANNEL(self), &client_cbs, NULL);
 
-    red_channel_init_stat_node(RED_CHANNEL(self), NULL, "spicevmc");
     const RedStatNode *stat = red_channel_get_stat_node(RED_CHANNEL(self));
     stat_init_counter(&self->in_data, reds, stat, "in_data", TRUE);
     stat_init_counter(&self->in_compressed, reds, stat, "in_compressed", TRUE);
@@ -273,6 +272,7 @@ 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, };
+    RedStatNode stat_node;
 
     switch (channel_type) {
         case SPICE_CHANNEL_USBREDIR:
@@ -288,6 +288,7 @@ 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;
     }
+    stat_init_node(&stat_node, reds, NULL, "spicevmc", TRUE);
     return g_object_new(gtype,
                         "spice-server", reds,
                         "core-interface", reds_get_core_interface(reds),
@@ -296,6 +297,7 @@ static RedVmcChannel *red_vmc_channel_new(RedsState *reds, uint8_t channel_type)
                         "handle-acks", FALSE,
                         "migration-flags",
                         (SPICE_MIGRATE_NEED_FLUSH | SPICE_MIGRATE_NEED_DATA_TRANSFER),
+                        "stat-node", &stat_node,
                         NULL);
 }
 
-- 
2.9.3



More information about the Spice-devel mailing list