>
> On Wed, 2017-06-14 at 16:39 +0100, Frediano Ziglio wrote:
> > So can be used by the device to communicate with the clients.
> >
> > Signed-off-by: Frediano Ziglio <fziglio@xxxxxxxxxx>
> > ---
> > server/stream-device.c | 20 ++++++++++++++++++++
> > 1 file changed, 20 insertions(+)
> >
> > diff --git a/server/stream-device.c b/server/stream-device.c
> > index 47eb3ac..6c4eccb 100644
> > --- a/server/stream-device.c
> > +++ b/server/stream-device.c
> > @@ -22,6 +22,8 @@
> > #include <spice/stream-device.h>
> >
> > #include "char-device.h"
> > +#include "stream-channel.h"
> > +#include "reds.h"
> >
> > #define TYPE_STREAM_DEVICE stream_device_get_type()
> >
> > @@ -37,9 +39,11 @@ typedef struct StreamDeviceClass
> > StreamDeviceClass;
> >
> > struct StreamDevice {
> > RedCharDevice parent;
> > +
> > StreamDevHeader hdr;
> > uint8_t hdr_pos;
> > bool has_error;
> > + StreamChannel *channel;
> > };
> >
> > struct StreamDeviceClass {
> > @@ -189,7 +193,10 @@ stream_device_connect(RedsState *reds,
> > SpiceCharDeviceInstance *sin)
> > {
> > SpiceCharDeviceInterface *sif;
> >
> > + StreamChannel *channel = stream_channel_new(reds);
> > +
> > StreamDevice *dev = stream_device_new(sin, reds);
> > + dev->channel = channel;
>
> Perhaps pass channel as an argument to stream_device_new() similar to
> how we do with red_char_device_spicevmc_new()?
>
If you mean the assign inside stream_device_new is fine for me.
If you mean write some property code I don't like it.
> >
> > sif = spice_char_device_get_interface(sin);
> > if (sif->state) {
> > @@ -202,6 +209,19 @@ stream_device_connect(RedsState *reds,
> > SpiceCharDeviceInstance *sin)
> > static void
> > stream_device_dispose(GObject *object)
> > {
> > + StreamDevice *device = STREAM_DEVICE(object);
> > +
> > + if (device->channel) {
> > + RedChannel *red_channel = RED_CHANNEL(device->channel);
> > + RedsState *reds = red_channel_get_server(red_channel);
> > +
> > + // prevent future connection
> > + reds_unregister_channel(reds, red_channel);
> > +
> > + // close all current connections and drop the reference
> > + red_channel_destroy(red_channel);
> > + device->channel = NULL;
> > + }
>
> This function is also very similar to
> red_char_device_spicevmc_dispose(). I'm not sure it's worthwhile to
> extract a common base class for these, but I'm wondering if you
> considered it?
>
Considered, just I didn't have a good name for it.
Seems silly but unregistering and destroying the object are quite
separate. Maybe a red_channel_shutdown? A red_channel_close would
just sound like a "close all connections" but in my mind does not
suggest that channel is unregistered. Maybe red_channel_destroy
should do the unregistering job? unregistering is called before
red_channel_destroy in all cases except for the main_channel; this
would cause a warning then the main channel is destroyed (I would
personally remove the warning from the code).
>
> > }
> >
> > static void
>
Frediano
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel