[Spice-devel] [PATCH 2/2] Convert RedChannelClient hierarchy to GObject

[Jonathon Jongsma]

On Tue, 2016-09-27 at 05:50 -0400, Frediano Ziglio wrote:
> > 
> > 
> > Convert the RedChannelClient heirarchy into GObjects. Since the
> > existing
> > constructors could fail and return NULL, I inherited the base
> > channel
> > client from GInitable, which introduces a dependency on gio.
> > 
> > When using private structs with GObject, there's a maximum size of
> > (I
> > think) 64k, which was exceeded by some of the private structs. To
> > avoid
> > this limitation I changed some members to dynamically allocated.
> > ---
> >  configure.ac                        |   4 +-
> >  server/Makefile.am                  |   2 +
> >  server/cursor-channel-client.c      |  74 ++--
> >  server/cursor-channel-client.h      |  34 +-
> >  server/dcc-private.h                |  14 +-
> >  server/dcc.c                        | 202 ++++++++--
> >  server/dcc.h                        |  37 +-
> >  server/display-channel.c            |   4 +-
> >  server/dummy-channel-client.c       | 156 ++++++++
> >  server/dummy-channel-client.h       |  59 +++
> >  server/inputs-channel-client.c      |  56 ++-
> >  server/inputs-channel-client.h      |  47 ++-
> >  server/main-channel-client.c        | 146 +++++++-
> >  server/main-channel-client.h        |  34 +-
> >  server/red-channel-client-private.h |   4 +-
> >  server/red-channel-client.c         | 714
> >  ++++++++++++++++++++++--------------
> >  server/red-channel-client.h         |  80 +++-
> >  server/red-channel.h                |  33 +-
> >  server/reds.h                       |   1 +
> >  server/smartcard-channel-client.c   | 120 +++++-
> >  server/smartcard-channel-client.h   |  42 ++-
> >  server/smartcard.c                  |  30 +-
> >  server/sound.c                      |   9 +-
> >  server/spice-server.h               |  16 +
> >  server/spicevmc.c                   |   6 +-
> >  server/tests/test_display_base.c    |   4 +-
> >  26 files changed, 1441 insertions(+), 487 deletions(-)
> >  create mode 100644 server/dummy-channel-client.c
> >  create mode 100644 server/dummy-channel-client.h
> > 
> 
> I didn't find much time to look a bit deeper at this patch as a
> whole.
> 
> I tried to focus on the DummyChannelClient and the reason for this.
> First, this "feature" can be easily implemented supporting the
> case having stream == NULL in RedChannelClient.
> But I don't understand the rationale for this feature. It's only
> used in sound.c to handle record/playback. After a first read I
> though was to handle message encapsulated in a different way but
> sound.c use the same encapsulation. So I don't really understand
> why sound.c have to handle RedsStream directly and cannot use
> the same code provided by RedChannelClient as all other channels
> do. SndChannel seems to contain lot of code copied and pasted
> from RedChannelClient.
> I personally think that the approach should use RedChannelClient
> and not reinventing the wheel and the creation of DummyChannelClient
> is just going in the wrong direction.
> 
> Does anybody is comfortable with sound.c and can explain why
> not using ReChannelClient?
> 
> Frediano
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel



Yes, I agree that it's strange, and I don't like it. But as part of the
refactoring, I decided to keep the "dummy" channel client idea and just
port it to a GObject style. I did not want to do a lot of intrusive re-
designing as part of this refactory. I think we can make more
improvements later. 

Jonathon