[Spice-devel] [PATCH 3.12/12] Various minor style changes to worker and cursor channel

[Jonathon Jongsma]

On Fri, 2015-10-30 at 12:21 -0400, Marc-André Lureau wrote:
> Hi
> 
> ----- Original Message -----
> > > > @@ -446,10 +446,11 @@ void
> > > > cursor_channel_process_cmd(CursorChannel
> > > > *cursor,
> > > > RedCursorCmd *cursor_cmd,
> > > >          return;
> > > >      }
> > > >  
> > > > -    if (red_channel_is_connected(&cursor->common.base) &&
> > > > (cursor->mouse_mode == SPICE_MOUSE_MODE_SERVER ||
> > > > -                                   cursor_cmd->type !=
> > > > QXL_CURSOR_MOVE ||
> > > > cursor_show)) {
> > > > -        red_channel_pipes_new_add(&cursor->common.base,
> > > > new_cursor_pipe_item,
> > > > -                                  (void*)cursor_item);
> > > > +    if (cursor->mouse_mode == SPICE_MOUSE_MODE_SERVER
> > > > +        || cursor_cmd->type != QXL_CURSOR_MOVE
> > > > +        || cursor_show) {
> > > > +        red_channel_pipes_new_add(&cursor->common.base,
> > > > +                                  new_cursor_pipe_item,
> > > > cursor_item);
> > > >      }
> > > 
> > > Why silently remove the red_channel_is_connected check?
> > 
> > Yes, I had the same question and was going to comment on it as
> > well.
> > Marc-Andre, if you're reading this, do you remember?
> 
> I don't remember. It looks quite a useless check to me, if
> process_cmd() is called, it suggests the channel is connected, isn't
> it? Furthermore, such a check could possibly exists in
> red_channel_pipes_new_add(). And even if it didn't, I suppose that
> wouldn't be a big deal to free it later. But anyway, it looks safer
> to add back if we don't know why it's changed.


I assumed that was probably the reason they were removed. But I think
I'll add them back just to be safe, since there's not much downside to
having them there.

Thanks,
Jonathon