With some simple steps I simplified the internal loop of this
function
* convert to infinite loop
/* dpi contains a sublist of dcc's, ordered the same */
while (worker_ring_item) {
dcc = SPICE_CONTAINEROF(worker_ring_item, DisplayChannelClient,
common.base.channel_link);
dpi = SPICE_CONTAINEROF(dpi_ring_item, RedDrawablePipeItem, base);
- while (worker_ring_item && (!dpi || dcc != dpi->dcc)) {
+ for (;;) {
+ if (!worker_ring_item || (dpi && dcc == dpi->dcc)) {
+ break;
+ }
dcc_prepend_drawable(dcc, drawable);
worker_ring_item = ring_next(&RED_CHANNEL(display)->clients,
worker_ring_item);
dcc = SPICE_CONTAINEROF(worker_ring_item, DisplayChannelClient,
* split if
while (worker_ring_item) {
dcc = SPICE_CONTAINEROF(worker_ring_item, DisplayChannelClient,
common.base.channel_link);
dpi = SPICE_CONTAINEROF(dpi_ring_item, RedDrawablePipeItem, base);
for (;;) {
- if (!worker_ring_item || (dpi && dcc == dpi->dcc)) {
+ if (!worker_ring_item) {
+ break;
+ }
+ if (dpi && dcc == dpi->dcc) {
break;
}
dcc_prepend_drawable(dcc, drawable);
worker_ring_item = ring_next(&RED_CHANNEL(display)->clients,
worker_ring_item);
* the worker_ring_item condition make this case terminate loop
dpi_ring_item is changed but ignored
dcc = SPICE_CONTAINEROF(worker_ring_item, DisplayChannelClient,
common.base.channel_link);
dpi = SPICE_CONTAINEROF(dpi_ring_item, RedDrawablePipeItem, base);
for (;;) {
if (!worker_ring_item) {
- break;
+ goto out_loop;
}
if (dpi && dcc == dpi->dcc) {
break;
}
dcc_prepend_drawable(dcc, drawable);
worker_ring_item = ring_next(&RED_CHANNEL(display)->clients,
worker_ring_item);
dpi_ring_item = ring_next(&other_drawable->pipes, dpi_ring_item);
}
if (worker_ring_item) {
worker_ring_item = ring_next(&RED_CHANNEL(display)->clients,
worker_ring_item);
}
}
+out_loop:
/* not sending other_drawable where possible */
drawable_remove_from_pipes(other_drawable);
drawable_unref(other_drawable);
return TRUE;
* worker_ring_item is never null here
if (dpi_ring_item) {
dpi_ring_item = ring_next(&other_drawable->pipes, dpi_ring_item);
}
- if (worker_ring_item) {
- worker_ring_item = ring_next(&RED_CHANNEL(display)->clients,
- worker_ring_item);
- }
+ worker_ring_item = ring_next(&RED_CHANNEL(display)->clients,
+ worker_ring_item);
}
out_loop:
/* not sending other_drawable where possible */
drawable_remove_from_pipes(other_drawable);
* dcc used is always a value from current worker_ring_item
worker_ring_item = ring_get_head(&RED_CHANNEL(display)->clients);
dpi_ring_item = ring_get_head(&other_drawable->pipes);
/* dpi contains a sublist of dcc's, ordered the same */
while (worker_ring_item) {
- dcc = SPICE_CONTAINEROF(worker_ring_item, DisplayChannelClient,
- common.base.channel_link);
dpi = SPICE_CONTAINEROF(dpi_ring_item, RedDrawablePipeItem, base);
for (;;) {
if (!worker_ring_item) {
goto out_loop;
}
+ dcc = SPICE_CONTAINEROF(worker_ring_item, DisplayChannelClient,
+ common.base.channel_link);
if (dpi && dcc == dpi->dcc) {
break;
}
dcc_prepend_drawable(dcc, drawable);
worker_ring_item = ring_next(&RED_CHANNEL(display)->clients,
worker_ring_item);
- dcc = SPICE_CONTAINEROF(worker_ring_item, DisplayChannelClient,
- common.base.channel_link);
}
if (dpi_ring_item) {
dpi_ring_item = ring_next(&other_drawable->pipes, dpi_ring_item);
}
* move code inside if
these lines are only executed then the condition are satisfied
goto out_loop;
}
dcc = SPICE_CONTAINEROF(worker_ring_item, DisplayChannelClient,
common.base.channel_link);
if (dpi && dcc == dpi->dcc) {
+ if (dpi_ring_item) {
+ dpi_ring_item = ring_next(&other_drawable->pipes, dpi_ring_item);
+ }
+ worker_ring_item = ring_next(&RED_CHANNEL(display)->clients,
+ worker_ring_item);
break;
}
dcc_prepend_drawable(dcc, drawable);
worker_ring_item = ring_next(&RED_CHANNEL(display)->clients,
worker_ring_item);
}
-
- if (dpi_ring_item) {
- dpi_ring_item = ring_next(&other_drawable->pipes, dpi_ring_item);
- }
- worker_ring_item = ring_next(&RED_CHANNEL(display)->clients,
- worker_ring_item);
}
out_loop:
/* not sending other_drawable where possible */
drawable_remove_from_pipes(other_drawable);
* dpi is always a value computed from dpi_ring_item
worker_ring_item = ring_get_head(&RED_CHANNEL(display)->clients);
dpi_ring_item = ring_get_head(&other_drawable->pipes);
/* dpi contains a sublist of dcc's, ordered the same */
while (worker_ring_item) {
- dpi = SPICE_CONTAINEROF(dpi_ring_item, RedDrawablePipeItem, base);
for (;;) {
if (!worker_ring_item) {
goto out_loop;
}
dcc = SPICE_CONTAINEROF(worker_ring_item, DisplayChannelClient,
common.base.channel_link);
+ dpi = SPICE_CONTAINEROF(dpi_ring_item, RedDrawablePipeItem, base);
if (dpi && dcc == dpi->dcc) {
if (dpi_ring_item) {
dpi_ring_item = ring_next(&other_drawable->pipes, dpi_ring_item);
}
worker_ring_item = ring_next(&RED_CHANNEL(display)->clients,
* if dpi is not NULL even dpi_ring_item is not NULL
dcc = SPICE_CONTAINEROF(worker_ring_item, DisplayChannelClient,
common.base.channel_link);
dpi = SPICE_CONTAINEROF(dpi_ring_item, RedDrawablePipeItem, base);
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);
worker_ring_item = ring_next(&RED_CHANNEL(display)->clients,
worker_ring_item);
break;
}
dcc_prepend_drawable(dcc, drawable);
* if worker_ring_item is NULL jumping outside loop will check
variable and exit
The outer loop would exit too.
dpi_ring_item = ring_get_head(&other_drawable->pipes);
/* dpi contains a sublist of dcc's, ordered the same */
while (worker_ring_item) {
for (;;) {
if (!worker_ring_item) {
- goto out_loop;
+ break;
}
dcc = SPICE_CONTAINEROF(worker_ring_item, DisplayChannelClient,
common.base.channel_link);
dpi = SPICE_CONTAINEROF(dpi_ring_item, RedDrawablePipeItem, base);
if (dpi && dcc == dpi->dcc) {
dpi_ring_item = ring_next(&other_drawable->pipes, dpi_ring_item);
worker_ring_item = ring_next(&RED_CHANNEL(display)->clients,
break;
}
dcc_prepend_drawable(dcc, drawable);
worker_ring_item = ring_next(&RED_CHANNEL(display)->clients,
worker_ring_item);
}
}
-out_loop:
/* not sending other_drawable where possible */
drawable_remove_from_pipes(other_drawable);
drawable_unref(other_drawable);
return TRUE;
* use while again
Convert from for(;;) { if () break; ... } to while
worker_ring_item = ring_get_head(&RED_CHANNEL(display)->clients);
dpi_ring_item = ring_get_head(&other_drawable->pipes);
/* dpi contains a sublist of dcc's, ordered the same */
while (worker_ring_item) {
- for (;;) {
- if (!worker_ring_item) {
- break;
- }
+ while (worker_ring_item) {
dcc = SPICE_CONTAINEROF(worker_ring_item, DisplayChannelClient,
common.base.channel_link);
dpi = SPICE_CONTAINEROF(dpi_ring_item, RedDrawablePipeItem, base);
if (dpi && dcc == dpi->dcc) {
dpi_ring_item = ring_next(&other_drawable->pipes, dpi_ring_item);
* convert break to continue
Both jump to check worker_ring_item (internal or external loops)
dpi = SPICE_CONTAINEROF(dpi_ring_item, RedDrawablePipeItem, base);
if (dpi && dcc == dpi->dcc) {
dpi_ring_item = ring_next(&other_drawable->pipes, dpi_ring_item);
worker_ring_item = ring_next(&RED_CHANNEL(display)->clients,
worker_ring_item);
- break;
+ continue;
}
dcc_prepend_drawable(dcc, drawable);
worker_ring_item = ring_next(&RED_CHANNEL(display)->clients,
worker_ring_item);
}
* reuse code inside loop
dcc = SPICE_CONTAINEROF(worker_ring_item, DisplayChannelClient,
common.base.channel_link);
dpi = SPICE_CONTAINEROF(dpi_ring_item, RedDrawablePipeItem, base);
if (dpi && dcc == dpi->dcc) {
dpi_ring_item = ring_next(&other_drawable->pipes, dpi_ring_item);
- worker_ring_item = ring_next(&RED_CHANNEL(display)->clients,
- worker_ring_item);
- continue;
+ } else {
+ dcc_prepend_drawable(dcc, drawable);
}
- dcc_prepend_drawable(dcc, drawable);
worker_ring_item = ring_next(&RED_CHANNEL(display)->clients,
worker_ring_item);
}
}
* join the two loop
They tested the same condition, no break in inside loop (which would
exit all loops)
worker_ring_item = ring_get_head(&RED_CHANNEL(display)->clients);
dpi_ring_item = ring_get_head(&other_drawable->pipes);
/* dpi contains a sublist of dcc's, ordered the same */
while (worker_ring_item) {
- while (worker_ring_item) {
- dcc = SPICE_CONTAINEROF(worker_ring_item, DisplayChannelClient,
- common.base.channel_link);
- dpi = SPICE_CONTAINEROF(dpi_ring_item, RedDrawablePipeItem, base);
- if (dpi && dcc == dpi->dcc) {
- dpi_ring_item = ring_next(&other_drawable->pipes, dpi_ring_item);
- } else {
- dcc_prepend_drawable(dcc, drawable);
- }
- worker_ring_item = ring_next(&RED_CHANNEL(display)->clients,
- worker_ring_item);
+ dcc = SPICE_CONTAINEROF(worker_ring_item, DisplayChannelClient,
+ common.base.channel_link);
+ dpi = SPICE_CONTAINEROF(dpi_ring_item, RedDrawablePipeItem, base);
+ if (dpi && dcc == dpi->dcc) {
+ dpi_ring_item = ring_next(&other_drawable->pipes, dpi_ring_item);
+ } else {
+ dcc_prepend_drawable(dcc, drawable);
}
+ worker_ring_item = ring_next(&RED_CHANNEL(display)->clients,
+ worker_ring_item);
}
/* not sending other_drawable where possible */
drawable_remove_from_pipes(other_drawable);
drawable_unref(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)
switch (item->effect) {
case QXL_EFFECT_REVERT_ON_DUP:
if (is_same_drawable(drawable, other_drawable)) {
DisplayChannelClient *dcc;
- RedDrawablePipeItem *dpi;
RingItem *worker_ring_item, *dpi_ring_item;
other_drawable->refs++;
current_remove_drawable(display, other_drawable);
/* sending the drawable to clients that already received
* (or will receive) other_drawable */
worker_ring_item = ring_get_head(&RED_CHANNEL(display)->clients);
dpi_ring_item = ring_get_head(&other_drawable->pipes);
/* dpi contains a sublist of dcc's, ordered the same */
while (worker_ring_item) {
dcc = SPICE_CONTAINEROF(worker_ring_item, DisplayChannelClient,
common.base.channel_link);
- dpi = SPICE_CONTAINEROF(dpi_ring_item, RedDrawablePipeItem, base);
- if (dpi && dcc == dpi->dcc) {
+ if (dpi_ring_item && dcc == SPICE_CONTAINEROF(dpi_ring_item, RedDrawablePipeItem, base)->dcc) {
dpi_ring_item = ring_next(&other_drawable->pipes, dpi_ring_item);
} else {
dcc_prepend_drawable(dcc, drawable);
}
worker_ring_item = ring_next(&RED_CHANNEL(display)->clients,
* use RING_FOREACH macro for loop
other_drawable->refs++;
current_remove_drawable(display, other_drawable);
/* sending the drawable to clients that already received
* (or will receive) other_drawable */
- worker_ring_item = ring_get_head(&RED_CHANNEL(display)->clients);
dpi_ring_item = ring_get_head(&other_drawable->pipes);
/* dpi contains a sublist of dcc's, ordered the same */
- while (worker_ring_item) {
+ RING_FOREACH (worker_ring_item, &RED_CHANNEL(display)->clients) {
dcc = SPICE_CONTAINEROF(worker_ring_item, DisplayChannelClient,
common.base.channel_link);
if (dpi_ring_item && dcc == SPICE_CONTAINEROF(dpi_ring_item, RedDrawablePipeItem, base)->dcc) {
dpi_ring_item = ring_next(&other_drawable->pipes, dpi_ring_item);
} else {
dcc_prepend_drawable(dcc, drawable);
}
- worker_ring_item = ring_next(&RED_CHANNEL(display)->clients,
- worker_ring_item);
}
/* not sending other_drawable where possible */
drawable_remove_from_pipes(other_drawable);
drawable_unref(other_drawable);
Signed-off-by: Frediano Ziglio <fziglio@xxxxxxxxxx>
---
server/display-channel.c | 21 ++++-----------------
1 file changed, 4 insertions(+), 17 deletions(-)
Changes from v2:
- update comment, some notes were missing.
diff --git a/server/display-channel.c b/server/display-channel.c
index 96a6f2f..6d459fe 100644
--- a/server/display-channel.c
+++ b/server/display-channel.c
@@ -464,7 +464,6 @@ static int current_add_equal(DisplayChannel *display, DrawItem *item, TreeItem *
if (is_same_drawable(drawable, other_drawable)) {
DisplayChannelClient *dcc;
- RedDrawablePipeItem *dpi;
RingItem *worker_ring_item, *dpi_ring_item;
other_drawable->refs++;
@@ -472,27 +471,15 @@ static int current_add_equal(DisplayChannel *display, DrawItem *item, TreeItem *
/* sending the drawable to clients that already received
* (or will receive) other_drawable */
- worker_ring_item = ring_get_head(&RED_CHANNEL(display)->clients);
dpi_ring_item = ring_get_head(&other_drawable->pipes);
/* dpi contains a sublist of dcc's, ordered the same */
- while (worker_ring_item) {
+ RING_FOREACH (worker_ring_item, &RED_CHANNEL(display)->clients) {
dcc = SPICE_CONTAINEROF(worker_ring_item, DisplayChannelClient,
common.base.channel_link);
- dpi = SPICE_CONTAINEROF(dpi_ring_item, RedDrawablePipeItem, base);
- while (worker_ring_item && (!dpi || dcc != dpi->dcc)) {
- dcc_prepend_drawable(dcc, drawable);
- worker_ring_item = ring_next(&RED_CHANNEL(display)->clients,
- worker_ring_item);
- dcc = SPICE_CONTAINEROF(worker_ring_item, DisplayChannelClient,
- common.base.channel_link);
- }
-
- if (dpi_ring_item) {
+ if (dpi_ring_item && dcc == SPICE_CONTAINEROF(dpi_ring_item, RedDrawablePipeItem, base)->dcc) {
dpi_ring_item = ring_next(&other_drawable->pipes, dpi_ring_item);
- }
- if (worker_ring_item) {
- worker_ring_item = ring_next(&RED_CHANNEL(display)->clients,
- worker_ring_item);
+ } 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