[Spice-devel] [PATCH] fixup! spicevmc: Change creation of RedCharDeviceSpiceVmc

[Jonathon Jongsma]

---
Possible fixup to Frediano's proposed patch. In general, GObject _new()
functions should try not to do more than call g_object_new(). This is mostly to
make things simpler for language bindings, but I think it's good practice to do
as much initialization via the GObject construction framework as possible. So I
made 'channel' a GObject property in RedCharDeviceSpicevmc, and moved some of
the additional work up to spicevmc_device_connect().

For the time being, I've left the duplicated 'sin' parameter in the
RedVmcChannel. But if we removed that from the channel (and simply retrieved it
as-needed from the char device as discussed in another patch), it would
simplify things further, since spicevmc_device_connect() would not really have
to do any additional work besides creating the objects.

 server/spicevmc.c | 81 ++++++++++++++++++++++++++++++++++++++++---------------
 1 file changed, 59 insertions(+), 22 deletions(-)

diff --git a/server/spicevmc.c b/server/spicevmc.c
index cf743de..084f09a 100644
--- a/server/spicevmc.c
+++ b/server/spicevmc.c
@@ -89,7 +89,7 @@ struct RedCharDeviceSpiceVmcClass
 static GType red_char_device_spicevmc_get_type(void) G_GNUC_CONST;
 static RedCharDevice *red_char_device_spicevmc_new(SpiceCharDeviceInstance *sin,
                                                    RedsState *reds,
-                                                   uint8_t channel_type);
+                                                   RedVmcChannel *channel);
 
 G_DEFINE_TYPE(RedCharDeviceSpiceVmc, red_char_device_spicevmc, RED_TYPE_CHAR_DEVICE)
 
@@ -806,7 +806,20 @@ RedCharDevice *spicevmc_device_connect(RedsState *reds,
                                        SpiceCharDeviceInstance *sin,
                                        uint8_t channel_type)
 {
-    return red_char_device_spicevmc_new(sin, reds, channel_type);
+    RedCharDevice *dev;
+    RedVmcChannel *channel = red_vmc_channel_new(reds, channel_type);
+    if (!channel) {
+        return NULL;
+    }
+
+    /* char device takes ownership of channel */
+    dev = red_char_device_spicevmc_new(sin, reds, channel);
+
+    channel->chardev_sin = sin;
+    g_object_unref(channel);
+
+    return dev;
 }
 
 /* Must be called from RedClient handling thread. */
@@ -861,18 +874,54 @@ red_char_device_spicevmc_dispose(GObject *object)
     }
 }
 
+enum {
+    PROP0,
+    PROP_CHANNEL
+};
+
+static void
+red_char_device_spicevmc_set_property(GObject *object,
+                                      guint property_id,
+                                      const GValue *value,
+                                      GParamSpec *pspec)
+{
+    RedCharDeviceSpiceVmc *self = RED_CHAR_DEVICE_SPICEVMC(object);
+    switch (property_id)
+    {
+        case PROP_CHANNEL:
+            spice_assert(self->channel == NULL);
+            self->channel = g_value_dup_object(value);
+            self->channel->chardev = RED_CHAR_DEVICE(self);
+
+            break;
+        default:
+            G_OBJECT_WARN_INVALID_PROPERTY_ID(object, property_id, pspec);
+    }
+}
+
 static void
 red_char_device_spicevmc_class_init(RedCharDeviceSpiceVmcClass *klass)
 {
     GObjectClass *object_class = G_OBJECT_CLASS(klass);
     RedCharDeviceClass *char_dev_class = RED_CHAR_DEVICE_CLASS(klass);
 
+    object_class->set_property = red_char_device_spicevmc_set_property;
     object_class->dispose = red_char_device_spicevmc_dispose;
 
     char_dev_class->read_one_msg_from_device = spicevmc_chardev_read_msg_from_dev;
     char_dev_class->send_msg_to_client = spicevmc_chardev_send_msg_to_client;
     char_dev_class->send_tokens_to_client = spicevmc_char_dev_send_tokens_to_client;
     char_dev_class->remove_client = spicevmc_char_dev_remove_client;
+
+    g_object_class_install_property(object_class,
+                                    PROP_CHANNEL,
+                                    g_param_spec_object("channel",
+                                                         "Channel",
+                                                         "Channel associated with this device",
+                                                         RED_TYPE_VMC_CHANNEL,
+                                                         G_PARAM_STATIC_STRINGS |
+                                                         G_PARAM_WRITABLE |
+                                                         G_PARAM_CONSTRUCT));
 }
 
 static void
@@ -883,25 +932,13 @@ red_char_device_spicevmc_init(RedCharDeviceSpiceVmc *self)
 static RedCharDevice *
 red_char_device_spicevmc_new(SpiceCharDeviceInstance *sin,
                              RedsState *reds,
-                             uint8_t channel_type)
+                             RedVmcChannel *channel)
 {
-    RedCharDeviceSpiceVmc *dev;
-    RedVmcChannel *channel = red_vmc_channel_new(reds, channel_type);
-    if (!channel) {
-        return NULL;
-    }
-
-    dev = g_object_new(RED_TYPE_CHAR_DEVICE_SPICEVMC,
-                       "sin", sin,
-                       "spice-server", reds,
-                       "client-tokens-interval", 0ULL,
-                       "self-tokens", ~0ULL,
-                       "opaque", channel,
-                       NULL);
-
-    channel->chardev = RED_CHAR_DEVICE(dev);
-    channel->chardev_sin = sin;
-    dev->channel = channel;
-
-    return RED_CHAR_DEVICE(dev);
+    return g_object_new(RED_TYPE_CHAR_DEVICE_SPICEVMC,
+                        "sin", sin,
+                        "spice-server", reds,
+                        "client-tokens-interval", 0ULL,
+                        "self-tokens", ~0ULL,
+                        "opaque", channel,
+                        NULL);
 }
-- 
2.7.4