[Spice-devel] [PATCH 3/6] worker: make it clear it returns from process when no cmd

[Jonathon Jongsma]

On Tue, 2015-09-29 at 11:56 +0100, Frediano Ziglio wrote:
> From: Marc-André Lureau <marcandre.lureau at gmail.com>
> 
> ---
>  server/red_worker.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/server/red_worker.c b/server/red_worker.c
> index 2efb311..d8a081e 100644
> --- a/server/red_worker.c
> +++ b/server/red_worker.c
> @@ -4966,12 +4966,12 @@ static int red_process_cursor(RedWorker *worker, uint32_t max_pipe_size, int *ri
>              if (worker->repoll_cursor_ring < CMD_RING_POLL_RETRIES) {
>                  worker->repoll_cursor_ring++;
>                  worker->event_timeout = MIN(worker->event_timeout, CMD_RING_POLL_TIMEOUT);
> -                break;
> +                return n;
>              }
>              if (worker->repoll_cursor_ring > CMD_RING_POLL_RETRIES ||
>                  worker->qxl->st->qif->req_cursor_notification(worker->qxl)) {
>                  worker->repoll_cursor_ring++;
> -                break;
> +                return n;
>              }
>              continue;
>          }
> @@ -5028,12 +5028,12 @@ static int red_process_commands(RedWorker *worker, uint32_t max_pipe_size, int *
>              if (worker->repoll_cmd_ring < CMD_RING_POLL_RETRIES) {
>                  worker->repoll_cmd_ring++;
>                  worker->event_timeout = MIN(worker->event_timeout, CMD_RING_POLL_TIMEOUT);
> -                break;
> +                return n;
>              }
>              if (worker->repoll_cmd_ring > CMD_RING_POLL_RETRIES ||
>                           worker->qxl->st->qif->req_cmd_notification(worker->qxl)) {
>                  worker->repoll_cmd_ring++;
> -                break;
> +                return n;
>              }
>              continue;
>          }


This change looks fine to me, but while looking at the code, I noticed
something unrelated. It seems that e.g.
worker->qxl->st->qif->req_cursor_notification() will never be called.
worker->repoll_cursor_ring is incremented if it is less than
CMD_RING_POLL_RETRIES and is incremented if it's greater than
CMD_RING_POLL_RETRIES, but it is not incremented if it is equal. Which
means that once it reaches CMD_RING_POLL_RETRIES, it will stay there and
the second if statement will never be satisfied. The same appears to be
true in the second hunk.

Jonathon