> > 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@xxxxxxxxxx> > > > > Christophe <OT> On the many things reds.c do there are some network stuff (like TLS/SASL) that perhaps are responsibility of RedsStream. Frediano _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel