With some simple steps I simplified the internal loop of this function * convert to infinite loop while (link) { dcc = link->data; dpi = SPICE_UPCAST(RedDrawablePipeItem, dpi_ring_item); - while (link && (!dpi || dcc != dpi->dcc)) { + for (;;) { + if (!link || (dpi && dcc == dpi->dcc)) { + break; + } dcc_prepend_drawable(dcc, drawable); link = link->next; if (link) * split if dcc = link->data; dpi = SPICE_UPCAST(RedDrawablePipeItem, dpi_ring_item); for (;;) { - if (!link || (dpi && dcc == dpi->dcc)) { + if (!link) { + break; + } + if (dpi && dcc == dpi->dcc) { break; } dcc_prepend_drawable(dcc, drawable); * the link condition make this case terminate loop dpi_ring_item is changed but ignored dpi = SPICE_UPCAST(RedDrawablePipeItem, dpi_ring_item); for (;;) { if (!link) { - break; + goto out_loop; } if (dpi && dcc == dpi->dcc) { break; } dcc_prepend_drawable(dcc, drawable); link = link->next; if (link) dcc = link->data; } if (dpi_ring_item) { dpi_ring_item = ring_next(&other_drawable->pipes, dpi_ring_item); } if (link) { link = link->next; } } +out_loop: /* not sending other_drawable where possible */ drawable_remove_from_pipes(other_drawable); * link is never null here if (dpi_ring_item) { dpi_ring_item = ring_next(&other_drawable->pipes, dpi_ring_item); } - if (link) { - link = link->next; - } + link = link->next; } out_loop: /* not sending other_drawable where possible */ * dcc used is always a value from current link dpi_ring_item = ring_get_head(&other_drawable->pipes); /* dpi contains a sublist of dcc's, ordered the same */ while (link) { - dcc = link->data; dpi = SPICE_UPCAST(RedDrawablePipeItem, dpi_ring_item); for (;;) { if (!link) { goto out_loop; } + dcc = link->data; if (dpi && dcc == dpi->dcc) { break; } dcc_prepend_drawable(dcc, drawable); link = link->next; - if (link) - dcc = link->data; } if (dpi_ring_item) { * move code inside if these lines are only executed then the condition are satisfied } dcc = link->data; if (dpi && dcc == dpi->dcc) { + if (dpi_ring_item) { + dpi_ring_item = ring_next(&other_drawable->pipes, dpi_ring_item); + } + link = link->next; break; } dcc_prepend_drawable(dcc, drawable); link = link->next; } - - if (dpi_ring_item) { - dpi_ring_item = ring_next(&other_drawable->pipes, dpi_ring_item); - } - link = link->next; } out_loop: /* not sending other_drawable where possible */ * dpi is always a value computed from dpi_ring_item dpi_ring_item = ring_get_head(&other_drawable->pipes); /* dpi contains a sublist of dcc's, ordered the same */ while (link) { - dpi = SPICE_UPCAST(RedDrawablePipeItem, dpi_ring_item); for (;;) { if (!link) { goto out_loop; } dcc = link->data; + dpi = SPICE_UPCAST(RedDrawablePipeItem, dpi_ring_item); if (dpi && dcc == dpi->dcc) { if (dpi_ring_item) { dpi_ring_item = ring_next(&other_drawable->pipes, dpi_ring_item); * if dpi is not NULL even dpi_ring_item is not NULL dcc = link->data; dpi = SPICE_UPCAST(RedDrawablePipeItem, dpi_ring_item); if (dpi && dcc == dpi->dcc) { - if (dpi_ring_item) { - dpi_ring_item = ring_next(&other_drawable->pipes, dpi_ring_item); - } + dpi_ring_item = ring_next(&other_drawable->pipes, dpi_ring_item); link = link->next; break; } * if link is NULL jumping outside loop will check variable and exit The outer loop will exit too. while (link) { for (;;) { if (!link) { - goto out_loop; + break; } dcc = link->data; dpi = SPICE_UPCAST(RedDrawablePipeItem, dpi_ring_item); if (dpi && dcc == dpi->dcc) { dpi_ring_item = ring_next(&other_drawable->pipes, dpi_ring_item); link = link->next; break; } dcc_prepend_drawable(dcc, drawable); link = link->next; } } -out_loop: /* not sending other_drawable where possible */ drawable_remove_from_pipes(other_drawable); * use while again Convert from for(;;) { if () break; ... } to while dpi_ring_item = ring_get_head(&other_drawable->pipes); /* dpi contains a sublist of dcc's, ordered the same */ while (link) { - for (;;) { - if (!link) { - break; - } + while (link) { dcc = link->data; dpi = SPICE_UPCAST(RedDrawablePipeItem, dpi_ring_item); if (dpi && dcc == dpi->dcc) { * convert break to continue Both jump to check worker_ring_item (internal or external loops) dpi = SPICE_UPCAST(RedDrawablePipeItem, dpi_ring_item); if (dpi && dcc == dpi->dcc) { dpi_ring_item = ring_next(&other_drawable->pipes, dpi_ring_item); link = link->next; - break; + continue; } dcc_prepend_drawable(dcc, drawable); link = link->next; * reuse code inside loop dpi = SPICE_UPCAST(RedDrawablePipeItem, dpi_ring_item); if (dpi && dcc == dpi->dcc) { dpi_ring_item = ring_next(&other_drawable->pipes, dpi_ring_item); - link = link->next; - continue; + } else { + dcc_prepend_drawable(dcc, drawable); } - dcc_prepend_drawable(dcc, drawable); link = link->next; } } * join the two loop They tested the same condition, no break in inside loop (which would exit all loops) dpi_ring_item = ring_get_head(&other_drawable->pipes); /* dpi contains a sublist of dcc's, ordered the same */ while (link) { - while (link) { - dcc = link->data; - dpi = SPICE_UPCAST(RedDrawablePipeItem, dpi_ring_item); - if (dpi && dcc == dpi->dcc) { - dpi_ring_item = ring_next(&other_drawable->pipes, dpi_ring_item); - } else { - dcc_prepend_drawable(dcc, drawable); - } - link = link->next; + dcc = link->data; + dpi = SPICE_UPCAST(RedDrawablePipeItem, dpi_ring_item); + if (dpi && dcc == dpi->dcc) { + dpi_ring_item = ring_next(&other_drawable->pipes, dpi_ring_item); + } else { + dcc_prepend_drawable(dcc, drawable); } + link = link->next; } /* not sending other_drawable where possible */ drawable_remove_from_pipes(other_drawable); * inline dpi computation Actually not exactly the same as this fix a bug if base is not the first element (in this case if dpi_ring_item is NULL dpi is not) if (is_same_drawable(drawable, other_drawable)) { DisplayChannelClient *dcc; - RedDrawablePipeItem *dpi; RingItem *dpi_ring_item; GList *link; other_drawable->refs++; current_remove_drawable(display, other_drawable); /* sending the drawable to clients that already received * (or will receive) other_drawable */ link = RED_CHANNEL(display)->clients; dpi_ring_item = ring_get_head(&other_drawable->pipes); /* dpi contains a sublist of dcc's, ordered the same */ while (link) { dcc = link->data; - dpi = SPICE_UPCAST(RedDrawablePipeItem, dpi_ring_item); - if (dpi && dcc == dpi->dcc) { + if (dpi_ring_item && dcc == SPICE_UPCAST(RedDrawablePipeItem, dpi_ring_item)->dcc) { dpi_ring_item = ring_next(&other_drawable->pipes, dpi_ring_item); } else { dcc_prepend_drawable(dcc, drawable); * use FOREACH_CLIENT macro for loop DisplayChannelClient *dcc; RingItem *dpi_ring_item; - GList *link; + GList *link, *next; other_drawable->refs++; current_remove_drawable(display, other_drawable); /* sending the drawable to clients that already received * (or will receive) other_drawable */ - link = RED_CHANNEL(display)->clients; dpi_ring_item = ring_get_head(&other_drawable->pipes); /* dpi contains a sublist of dcc's, ordered the same */ - while (link) { - dcc = link->data; + FOREACH_CLIENT(display, link, next, dcc) { if (dpi_ring_item && dcc == SPICE_UPCAST(RedDrawablePipeItem, dpi_ring_item)->dcc) { dpi_ring_item = ring_next(&other_drawable->pipes, dpi_ring_item); } else { dcc_prepend_drawable(dcc, drawable); } - link = link->next; } /* not sending other_drawable where possible */ drawable_remove_from_pipes(other_drawable); Signed-off-by: Frediano Ziglio <fziglio@xxxxxxxxxx> --- server/display-channel.c | 22 +++++----------------- 1 file changed, 5 insertions(+), 17 deletions(-) Changes from v3: - rebased on master. diff --git a/server/display-channel.c b/server/display-channel.c index 13be947..cfa182c 100644 --- a/server/display-channel.c +++ b/server/display-channel.c @@ -466,33 +466,21 @@ static int current_add_equal(DisplayChannel *display, DrawItem *item, TreeItem * if (is_same_drawable(drawable, other_drawable)) { DisplayChannelClient *dcc; - RedDrawablePipeItem *dpi; RingItem *dpi_ring_item; - GList *link; + GList *link, *next; other_drawable->refs++; current_remove_drawable(display, other_drawable); /* sending the drawable to clients that already received * (or will receive) other_drawable */ - link = RED_CHANNEL(display)->clients; dpi_ring_item = ring_get_head(&other_drawable->pipes); /* dpi contains a sublist of dcc's, ordered the same */ - while (link) { - dcc = link->data; - dpi = SPICE_UPCAST(RedDrawablePipeItem, dpi_ring_item); - while (link && (!dpi || dcc != dpi->dcc)) { - dcc_prepend_drawable(dcc, drawable); - link = link->next; - if (link) - dcc = link->data; - } - - if (dpi_ring_item) { + FOREACH_CLIENT(display, link, next, dcc) { + if (dpi_ring_item && dcc == SPICE_UPCAST(RedDrawablePipeItem, dpi_ring_item)->dcc) { dpi_ring_item = ring_next(&other_drawable->pipes, dpi_ring_item); - } - if (link) { - link = link->next; + } else { + dcc_prepend_drawable(dcc, drawable); } } /* not sending other_drawable where possible */ -- 2.7.4 _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel