[Spice-devel] [PATCH spice-server 4/4] Provide and reuse default implementation for config_socket

[Jonathon Jongsma]

On Tue, 2017-02-14 at 15:55 +0100, Christophe Fergeau wrote:
> On Mon, Feb 13, 2017 at 11:03:19AM +0000, Frediano Ziglio wrote:
> > Most channel don't need to do specific settings for the
> > client socket so provide a default implementation to
> > make easier to setup the client channnel.
> > 
> > Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
> > ---
> >  server/inputs-channel.c     | 6 ------
> >  server/main-channel.c       | 6 ------
> >  server/red-channel-client.c | 5 +++++
> >  server/red-channel-client.h | 2 ++
> >  server/red-channel.c        | 2 ++
> >  server/smartcard.c          | 6 ------
> >  server/spicevmc.c           | 6 ------
> >  7 files changed, 9 insertions(+), 24 deletions(-)
> > 
> > diff --git a/server/inputs-channel.c b/server/inputs-channel.c
> > index 897e8e7..6a6dce8 100644
> > --- a/server/inputs-channel.c
> > +++ b/server/inputs-channel.c
> > @@ -488,11 +488,6 @@ static void
> > inputs_pipe_add_init(RedChannelClient *rcc)
> >      red_channel_client_pipe_add_push(rcc, &item->base);
> >  }
> >  
> > -static int inputs_channel_config_socket(RedChannelClient *rcc)
> > -{
> > -    return TRUE;
> > -}
> > -
> >  static void inputs_connect(RedChannel *channel, RedClient *client,
> >                             RedsStream *stream, int migration,
> >                             int num_common_caps, uint32_t
> > *common_caps,
> > @@ -638,7 +633,6 @@ inputs_channel_class_init(InputsChannelClass
> > *klass)
> >      channel_class->handle_parsed = inputs_channel_handle_parsed;
> >  
> >      /* channel callbacks */
> > -    channel_class->config_socket = inputs_channel_config_socket;
> >      channel_class->on_disconnect = inputs_channel_on_disconnect;
> >      channel_class->send_item = inputs_channel_send_item;
> >      channel_class->alloc_recv_buf =
> > inputs_channel_alloc_msg_rcv_buf;
> > diff --git a/server/main-channel.c b/server/main-channel.c
> > index 1124506..4f39b91 100644
> > --- a/server/main-channel.c
> > +++ b/server/main-channel.c
> > @@ -277,11 +277,6 @@ static void
> > main_channel_release_msg_rcv_buf(RedChannelClient *rcc,
> >      }
> >  }
> >  
> > -static int main_channel_config_socket(RedChannelClient *rcc)
> > -{
> > -    return TRUE;
> > -}
> > -
> >  static int main_channel_handle_migrate_flush_mark(RedChannelClient
> > *rcc)
> >  {
> >      RedChannel *channel = red_channel_client_get_channel(rcc);
> > @@ -355,7 +350,6 @@ main_channel_class_init(MainChannelClass
> > *klass)
> >      channel_class->handle_parsed = main_channel_handle_parsed;
> >  
> >      /* channel callbacks */
> > -    channel_class->config_socket = main_channel_config_socket;
> >      channel_class->on_disconnect =
> > main_channel_client_on_disconnect;
> >      channel_class->send_item = main_channel_client_send_item;
> >      channel_class->alloc_recv_buf =
> > main_channel_alloc_msg_rcv_buf;
> > diff --git a/server/red-channel-client.c b/server/red-channel-
> > client.c
> > index 2450923..f55ba85 100644
> > --- a/server/red-channel-client.c
> > +++ b/server/red-channel-client.c
> > @@ -1820,6 +1820,11 @@ gboolean
> > red_channel_client_set_migration_seamless(RedChannelClient *rcc)
> >      return ret;
> >  }
> >  
> > +int red_channel_client_config_socket(RedChannelClient *rcc)
> > +{
> > +    return TRUE;
> > +}
> > +
> >  void red_channel_client_set_destroying(RedChannelClient *rcc)
> >  {
> >      rcc->priv->destroying = TRUE;
> > diff --git a/server/red-channel-client.h b/server/red-channel-
> > client.h
> > index fada609..a5dc709 100644
> > --- a/server/red-channel-client.h
> > +++ b/server/red-channel-client.h
> > @@ -190,6 +190,8 @@ gboolean
> > red_channel_client_set_migration_seamless(RedChannelClient *rcc);
> >  void red_channel_client_set_destroying(RedChannelClient *rcc);
> >  gboolean red_channel_client_is_destroying(RedChannelClient *rcc);
> >  
> > +int red_channel_client_config_socket(RedChannelClient *rcc);
> > +
> >  struct RedChannelClient
> >  {
> >      GObject parent;
> > diff --git a/server/red-channel.c b/server/red-channel.c
> > index f2a35f3..fb34406 100644
> > --- a/server/red-channel.c
> > +++ b/server/red-channel.c
> > @@ -268,6 +268,8 @@ red_channel_class_init(RedChannelClass *klass)
> >      object_class->finalize = red_channel_finalize;
> >      object_class->constructed = red_channel_constructed;
> >  
> > +    klass->config_socket = red_channel_client_config_socket;
> > +
> 
> I would not provide any default implementation, and just change
> red_channel_config_socket to
> 
> 
> @@ -739,6 +739,10 @@ int red_channel_config_socket(RedChannel *self,
> RedChannelClient *rcc)
>  {
>      RedChannelClass *klass = RED_CHANNEL_GET_CLASS(self);
> 
> +    if (!klass->config_socket) {
> +        return TRUE;
> +    }
> +
>      return klass->config_socket(rcc);
>  }
> 
> If you prefer to provide an empty stub as the default impl, I'd name
> it
> red_channel_client_default_config_socket()

I was going to suggest this name as well. In general, normal GObject
naming conventions are that base_object_vfunc() is a wrapper function
that calls klass->vfunc(), and base_object_default_vfunc() is a default
implementation of that virtual function. 

In this case, the names are a bit odd because the vfunc is actually in
RedChannel but the default implementation is in RedChannelClient...

Jonathon