[Spice-devel] [PATCH spice-server] reds: Remove stream watch handling link in a single place

[Jonathon Jongsma]

Catching up on code review. Apologies if I missed a review for this.
Otherwise:

Acked-by: Jonathon Jongsma <jjongsma at redhat.com>



On Tue, 2018-01-09 at 14:19 -0500, Frediano Ziglio wrote:
> ping
> 
> > 
> > On 12/19/2017 06:58 PM, Frediano Ziglio wrote:
> > > > 
> > > > Hi Frediano,
> > > > 
> > > > Note that there still is another call to
> > > > red_stream_remove_watch,
> > > > (in reds_handle_ssl_accept), so consider changing the subject.
> > > > 
> > > 
> > > Yes, but this one is not handling link but SSL/TLS.
> > > 
> > > > One comment below.
> > > > 
> > > > 
> > > > On 12/19/2017 03:38 PM, Frediano Ziglio wrote:
> > > > > Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
> > > > > ---
> > > > >    server/reds.c | 3 +--
> > > > >    1 file changed, 1 insertion(+), 2 deletions(-)
> > > > > 
> > > > > diff --git a/server/reds.c b/server/reds.c
> > > > > index 9102c5122..66f24c72e 100644
> > > > > --- a/server/reds.c
> > > > > +++ b/server/reds.c
> > > > > @@ -1791,7 +1791,6 @@ static void
> > > > > reds_handle_main_link(RedsState *reds,
> > > > > RedLinkInfo *link)
> > > > >    
> > > > >        reds_info_new_channel(link, connection_id);
> > > > >        stream = link->stream;
> > > > > -    red_stream_remove_watch(stream);
> > > > >        link->stream = NULL;
> > > > 
> > > > You probably want to take move line too (link->stream = NULL)
> > > > 
> > > 
> > > No, unless I disable all connections, I need the stream to
> > > initialize
> > > channel clients.
> > > The "= NULL" is more bound to "stream = link->stream" as these 2
> > > lines
> > > basically detach
> > > the stream from link.
> > 
> > You are right.
> > The function red_stream_remove_watch removes only the watch
> > and set it to NULL.
> > 
> > 
> 
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel