Nice thorough explanation
Acked-by: Jonathon Jongsma <jjongsma@xxxxxxxxxx>
On Tue, 2016-02-09 at 10:27 +0000, Frediano Ziglio wrote:
> It was not clear when req_cmd_notification was called.
> This code reproduce just the old behavior but is easier to read.
>
> Steps are:
>
> // original
>
> if (worker->display_poll_tries < CMD_RING_POLL_RETRIES) {
> worker->display_poll_tries++;
> worker->event_timeout = MIN(worker->event_timeout, CMD_RING_POLL_TIMEOUT);
> return n;
> }
> if (worker->display_poll_tries > CMD_RING_POLL_RETRIES ||
> worker->qxl->st->qif->req_cmd_notification(worker->qxl)) {
> worker->display_poll_tries++;
> return n;
> }
> continue;
>
> // split if
>
> if (worker->display_poll_tries < CMD_RING_POLL_RETRIES) {
> worker->display_poll_tries++;
> worker->event_timeout = MIN(worker->event_timeout, CMD_RING_POLL_TIMEOUT);
> return n;
> }
> if (worker->display_poll_tries > CMD_RING_POLL_RETRIES) {
> worker->display_poll_tries++;
> return n;
> }
> if (worker->qxl->st->qif->req_cmd_notification(worker->qxl)) {
> worker->display_poll_tries++;
> return n;
> }
> continue;
>
> // order inside if
>
> if (worker->display_poll_tries < CMD_RING_POLL_RETRIES) {
> worker->event_timeout = MIN(worker->event_timeout, CMD_RING_POLL_TIMEOUT);
> worker->display_poll_tries++;
> return n;
> }
> if (worker->display_poll_tries > CMD_RING_POLL_RETRIES) {
> worker->display_poll_tries++;
> return n;
> }
> if (worker->qxl->st->qif->req_cmd_notification(worker->qxl)) {
> worker->display_poll_tries++;
> return n;
> }
> continue;
>
> // consider cases
>
> if (worker->display_poll_tries < CMD_RING_POLL_RETRIES) {
> worker->event_timeout = MIN(worker->event_timeout, CMD_RING_POLL_TIMEOUT);
> worker->display_poll_tries++;
> return n;
> }
> if (worker->display_poll_tries > CMD_RING_POLL_RETRIES) {
> worker->display_poll_tries++;
> return n;
> }
> if (worker->display_poll_tries == CMD_RING_POLL_RETRIES) {
> if (worker->qxl->st->qif->req_cmd_notification(worker->qxl)) {
> worker->display_poll_tries++;
> return n;
> }
> continue;
> }
> // unreachable
>
> // invert if
>
> if (worker->display_poll_tries < CMD_RING_POLL_RETRIES) {
> worker->event_timeout = MIN(worker->event_timeout, CMD_RING_POLL_TIMEOUT);
> worker->display_poll_tries++;
> return n;
> }
> if (worker->display_poll_tries > CMD_RING_POLL_RETRIES) {
> worker->display_poll_tries++;
> return n;
> }
> if (worker->display_poll_tries == CMD_RING_POLL_RETRIES) {
> if (!worker->qxl->st->qif->req_cmd_notification(worker->qxl)) {
> continue;
> }
> worker->display_poll_tries++;
> return n;
> }
> // unreachable
>
> // reuse code
>
> if (worker->display_poll_tries < CMD_RING_POLL_RETRIES) {
> worker->event_timeout = MIN(worker->event_timeout, CMD_RING_POLL_TIMEOUT);
> } else if (worker->display_poll_tries > CMD_RING_POLL_RETRIES) {
> } else if (worker->display_poll_tries == CMD_RING_POLL_RETRIES) {
> if (!worker->qxl->st->qif->req_cmd_notification(worker->qxl)) {
> continue;
> }
> }
> worker->display_poll_tries++;
> return n;
>
> // remove empty if
>
> if (worker->display_poll_tries < CMD_RING_POLL_RETRIES) {
> worker->event_timeout = MIN(worker->event_timeout, CMD_RING_POLL_TIMEOUT);
> } else if (worker->display_poll_tries == CMD_RING_POLL_RETRIES) {
> if (!worker->qxl->st->qif->req_cmd_notification(worker->qxl)) {
> continue;
> }
> }
> worker->display_poll_tries++;
> return n;
>
> // collapse two conditions
>
> if (worker->display_poll_tries < CMD_RING_POLL_RETRIES) {
> worker->event_timeout = MIN(worker->event_timeout, CMD_RING_POLL_TIMEOUT);
> } else if (worker->display_poll_tries == CMD_RING_POLL_RETRIES &&
> !worker->qxl->st->qif->req_cmd_notification(worker->qxl)) {
> continue;
> }
> worker->display_poll_tries++;
> return n;
>
> Signed-off-by: Frediano Ziglio <fziglio@xxxxxxxxxx>
> ---
> server/red-worker.c | 26 ++++++++++----------------
> 1 file changed, 10 insertions(+), 16 deletions(-)
>
> diff --git a/server/red-worker.c b/server/red-worker.c
> index e70c9df..ba37b6c 100644
> --- a/server/red-worker.c
> +++ b/server/red-worker.c
> @@ -167,16 +167,13 @@ static int red_process_cursor(RedWorker *worker, int
> *ring_is_empty)
> if (!worker->qxl->st->qif->get_cursor_command(worker->qxl, &ext_cmd))
> {
> *ring_is_empty = TRUE;
> if (worker->cursor_poll_tries < CMD_RING_POLL_RETRIES) {
> - worker->cursor_poll_tries++;
> worker->event_timeout = MIN(worker->event_timeout,
> CMD_RING_POLL_TIMEOUT);
> - return n;
> + } else if (worker->cursor_poll_tries == CMD_RING_POLL_RETRIES &&
> + !worker->qxl->st->qif->req_cursor_notification(worker
> ->qxl)) {
> + continue;
> }
> - if (worker->cursor_poll_tries > CMD_RING_POLL_RETRIES ||
> - worker->qxl->st->qif->req_cursor_notification(worker->qxl)) {
> - worker->cursor_poll_tries++;
> - return n;
> - }
> - continue;
> + worker->cursor_poll_tries++;
> + return n;
> }
> worker->cursor_poll_tries = 0;
> switch (ext_cmd.cmd.type) {
> @@ -228,16 +225,13 @@ static int red_process_display(RedWorker *worker, int
> *ring_is_empty)
> if (!worker->qxl->st->qif->get_command(worker->qxl, &ext_cmd)) {
> *ring_is_empty = TRUE;
> if (worker->display_poll_tries < CMD_RING_POLL_RETRIES) {
> - worker->display_poll_tries++;
> worker->event_timeout = MIN(worker->event_timeout,
> CMD_RING_POLL_TIMEOUT);
> - return n;
> + } else if (worker->display_poll_tries == CMD_RING_POLL_RETRIES &&
> + !worker->qxl->st->qif->req_cmd_notification(worker
> ->qxl)) {
> + continue;
> }
> - if (worker->display_poll_tries > CMD_RING_POLL_RETRIES ||
> - worker->qxl->st->qif->req_cmd_notification(worker
> ->qxl)) {
> - worker->display_poll_tries++;
> - return n;
> - }
> - continue;
> + worker->display_poll_tries++;
> + return n;
> }
>
> if (worker->record_fd)
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel