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

Fabiano FidĂȘncio fidencio at redhat.com
Wed Nov 4 03:18:45 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.


More information about the Spice-devel mailing list