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

[Frediano Ziglio]

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

> Looks good.
> 
> Uri.
> 
> >       link->link_mess = NULL;
> >       reds_link_free(link);
> > @@ -1993,7 +1992,6 @@ static void reds_handle_other_links(RedsState *reds,
> > RedLinkInfo *link)
> >   
> >       reds_send_link_result(link, SPICE_LINK_ERR_OK);
> >       reds_info_new_channel(link, link_mess->connection_id);
> > -    red_stream_remove_watch(link->stream);
> >   
> >       mig_client = reds_mig_target_client_find(reds, client);
> >       /*
> > @@ -2021,6 +2019,7 @@ static void reds_handle_other_links(RedsState *reds,
> > RedLinkInfo *link)
> >   
> >   static void reds_handle_link(RedsState *reds, RedLinkInfo *link)
> >   {
> > +    red_stream_remove_watch(link->stream);
> >       if (link->link_mess->channel_type == SPICE_CHANNEL_MAIN) {
> >           reds_handle_main_link(reds, link);
> >       } else {
> > 
> 
> 

Frediano