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

[Frediano Ziglio]

> 
> 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