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

[Frediano Ziglio]

> 
> On Tue, Feb 14, 2017 at 03:55:25PM +0100, Christophe Fergeau wrote:
> > I would not provide any default implementation, and just change
> > red_channel_config_socket to
> > 
> 
> Thinking more about this, I actually would provide a default
> implementation, and move the setsockopt/fnctl code from
> reds_init_client_connection() in the default implementation, and make
> sure the child classes properly chain up to it (or call it directly if
> chaining up is complicated)
> 
> Christophe
> 

This would introduce possible race conditions.
On reds.c the sockets are initialized before any usage.
Setting not blocking is quite mandatory to avoid some races using
some layers (like TLS). In this case before config_socket there
are the headers to tell which channel is this. A client could use
the race to cause the Qemu main thread to stop waiting for data
(potentially this could happen without authentication).

> > 
> > @@ -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()
> > 
> > Reviewed-by: Christophe Fergeau <cfergeau at redhat.com>
> > 
> > Christophe

<OT>
On the many things reds.c do there are some network stuff (like
TLS/SASL) that perhaps are responsibility of RedsStream.

Frediano