[Spice-devel] [PATCH] cursor: fix wrong logic when initializing the channel

Frediano Ziglio fziglio at redhat.com
Wed Nov 4 03:24:56 PST 2015


> 
> On Wed, Nov 4, 2015 at 11:54 AM, Frediano Ziglio <fziglio at redhat.com> wrote:
> >>
> >> It's a regression introduced by commit e601e920bd5. The logic error was
> >> introduced when trying to achieve the following code[0]. but rewritten
> >> to prefer an early return.
> >>
> >> [0]:
> >> if (cursor_is_connected(worker)
> >>     && !COMMON_CHANNEL(worker->cursor_channel)->during_target_migrate) {
> >>         red_channel_pipes_add_type(RED_CHANNEL(worker->cursor_channel),
> >>                                    PIPE_ITEM_TYPE_CURSOR_INIT);
> >> }
> >>
> >> Signed-off-by: Fabiano FidĂȘncio <fidencio at redhat.com>
> >> ---
> >>  server/cursor-channel.c | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/server/cursor-channel.c b/server/cursor-channel.c
> >> index ce360e1..23d5b5a 100644
> >> --- a/server/cursor-channel.c
> >> +++ b/server/cursor-channel.c
> >> @@ -545,7 +545,7 @@ void cursor_channel_init(CursorChannel *cursor,
> >> CursorChannelClient *client)
> >>      spice_return_if_fail(cursor);
> >>
> >>      if (red_channel_is_connected(&cursor->common.base)
> >> -        || COMMON_CHANNEL(cursor)->during_target_migrate) {
> >> +        && COMMON_CHANNEL(cursor)->during_target_migrate) {
> >>          spice_debug("during_target_migrate: skip init");
> >>          return;
> >>      }
> >> --
> >> 2.5.0
> >
> > The original code was
> >
> >     if (cursor_is_connected(worker)
> >         && !COMMON_CHANNEL(worker->cursor_channel)->during_target_migrate)
> >         {
> >         red_channel_pipes_add_type(RED_CHANNEL(worker->cursor_channel),
> >                                    PIPE_ITEM_TYPE_CURSOR_INIT);
> >     }
> >
> > which was wrongly negated as
> >
> >     if (red_channel_is_connected(&cursor->common.base)
> >         || COMMON_CHANNEL(cursor)->during_target_migrate) {
> >         spice_debug("during_target_migrate: skip init");
> >         return;
> >     }
> >
> > I think the right code should be
> >
> >     if (!red_channel_is_connected(&cursor->common.base)
> >         || COMMON_CHANNEL(cursor)->during_target_migrate) {
> >         spice_debug("during_target_migrate: skip init");
> >         return;
> >     }
> >
> > Frediano
> 
> You're right. Just sent a v2.
> 

Merged

Frediano


More information about the Spice-devel mailing list