[Spice-commits] 14 commits - server/red_channel.c server/red_worker.c server/tests
Uri Lublin
uril at kemper.freedesktop.org
Wed Jul 17 07:00:06 PDT 2013
server/red_channel.c | 8 -
server/red_worker.c | 183 +++++++++++++------------------
server/tests/test_display_base.c | 40 ++++--
server/tests/test_display_width_stride.c | 9 +
server/tests/test_empty_success.c | 10 +
5 files changed, 127 insertions(+), 123 deletions(-)
New commits:
commit 8511c747d12eaa3e60baf56316cb66b3ef0b7501
Author: Uri Lublin <uril at redhat.com>
Date: Tue Jul 9 17:01:10 2013 +0300
server/tests: fix timer for test_empty_success
diff --git a/server/tests/test_empty_success.c b/server/tests/test_empty_success.c
index ccabe6b..0176a52 100644
--- a/server/tests/test_empty_success.c
+++ b/server/tests/test_empty_success.c
@@ -4,9 +4,15 @@
#include <spice.h>
+struct SpiceTimer {
+ int a,b;
+};
+
SpiceTimer* timer_add(SpiceTimerFunc func, void *opaque)
{
- return NULL;
+ static struct SpiceTimer t = {0,};
+
+ return &t;
}
void timer_start(SpiceTimer *timer, uint32_t ms)
@@ -57,5 +63,7 @@ int main(void)
spice_server_set_port(server, 5911);
spice_server_init(server, &core);
+ spice_server_destroy(server);
+
return 0;
}
commit 3ede55b2ba602fd08870a007caea56ecb31022b3
Author: Uri Lublin <uril at redhat.com>
Date: Tue Jul 9 14:31:23 2013 +0300
server/tests: test_display_width_stride: add destroy command
Otherwise, the test exits after the first iteration over all tests,
on the second attempt to create an already created surface.
diff --git a/server/tests/test_display_width_stride.c b/server/tests/test_display_width_stride.c
index 20de57a..f938373 100644
--- a/server/tests/test_display_width_stride.c
+++ b/server/tests/test_display_width_stride.c
@@ -54,6 +54,14 @@ void set_surface_params(Test *test, Command *command)
create->data = g_surface_data;
}
+void set_destroy_parameters(Test *test, Command *command)
+{
+ if (g_surface_data) {
+ free(g_surface_data);
+ g_surface_data = NULL;
+ }
+}
+
static Command commands[] = {
{SIMPLE_CREATE_SURFACE, set_surface_params},
{SIMPLE_DRAW_SOLID, set_draw_parameters},
@@ -65,6 +73,7 @@ static Command commands[] = {
{SIMPLE_DRAW_SOLID, set_draw_parameters},
{SIMPLE_DRAW_SOLID, set_draw_parameters},
{SIMPLE_DRAW_SOLID, set_draw_parameters},
+ {SIMPLE_DESTROY_SURFACE, set_destroy_parameters},
};
void on_client_connected(Test *test)
commit b66d755447c0b91e4d09bbe2d942696106514046
Author: Uri Lublin <uril at redhat.com>
Date: Tue Jul 9 14:22:55 2013 +0300
server/tests: remove option from usage if AUTOMATED_TESTS is not configured
diff --git a/server/tests/test_display_base.c b/server/tests/test_display_base.c
index a4fdae9..d8ff8d1 100644
--- a/server/tests/test_display_base.c
+++ b/server/tests/test_display_base.c
@@ -62,6 +62,7 @@ static int control = 3; //used to know when we can take a screenshot
static int rects = 16; //number of rects that will be draw
static int has_automated_tests = 0; //automated test flag
+__attribute__((noreturn))
static void sigchld_handler(int signal_num) // wait for the child process and exit
{
int status;
@@ -878,6 +879,19 @@ void init_automated()
sigaction(SIGCHLD, &sa, NULL);
}
+__attribute__((noreturn))
+void usage(const char *argv0, const int exitcode)
+{
+#ifdef AUTOMATED_TESTS
+ const char *autoopt=" [--automated-tests]";
+#else
+ const char *autoopt="";
+#endif
+
+ printf("usage: %s%s\n", argv0, autoopt);
+ exit(exitcode);
+}
+
void spice_test_config_parse_args(int argc, char **argv)
{
struct option options[] = {
@@ -893,19 +907,18 @@ void spice_test_config_parse_args(int argc, char **argv)
switch (val) {
case '?':
printf("unrecognized option '%s'\n", argv[optind - 1]);
- goto invalid_option;
+ usage(argv[0], EXIT_FAILURE);
case 0:
break;
}
}
+ if (argc > optind) {
+ printf("unknown argument '%s'\n", argv[optind]);
+ usage(argv[0], EXIT_FAILURE);
+ }
if (has_automated_tests) {
init_automated();
}
return;
-
-invalid_option:
- printf("Invalid option!\n"
- "usage: %s [--automated-tests]\n", argv[0]);
- exit(0);
}
commit cbcd9eea363d287708e244e5afa3d25e9aa00810
Author: Uri Lublin <uril at redhat.com>
Date: Tue Jul 9 12:49:56 2013 +0300
server/tests: invalid-option: print the bad argument
optind points to the next argument to parse.
diff --git a/server/tests/test_display_base.c b/server/tests/test_display_base.c
index cf3990f..a4fdae9 100644
--- a/server/tests/test_display_base.c
+++ b/server/tests/test_display_base.c
@@ -892,7 +892,7 @@ void spice_test_config_parse_args(int argc, char **argv)
while ((val = getopt_long(argc, argv, "", options, &option_index)) != -1) {
switch (val) {
case '?':
- printf("unrecognized option %s", argv[optind]);
+ printf("unrecognized option '%s'\n", argv[optind - 1]);
goto invalid_option;
case 0:
break;
commit 694f4b9e5797af60e1586c2a99c19671483e5b44
Author: Uri Lublin <uril at redhat.com>
Date: Mon Jul 8 19:38:19 2013 +0300
server/tests: fix produce_command for create surface
Earlier in this function, test->target_surface is set to 1, which
is the only allowed non-primary surface currently.
If surface parameters are given (and specifically data is checked)
they are being used, otherwise a default surface is used.
Earlier in this function, "command" is set to a non-NULL value.
Thus, the else part was unreachable code, which is fixed now.
diff --git a/server/tests/test_display_base.c b/server/tests/test_display_base.c
index ad2c687..cf3990f 100644
--- a/server/tests/test_display_base.c
+++ b/server/tests/test_display_base.c
@@ -553,8 +553,10 @@ static void produce_command(Test *test)
case SIMPLE_CREATE_SURFACE: {
SimpleSurfaceCmd *update;
- test->target_surface = MAX_SURFACE_NUM - 1;
- if (command) {
+ if (command->create_surface.data) {
+ ASSERT(command->create_surface.surface_id > 0);
+ ASSERT(command->create_surface.surface_id < MAX_SURFACE_NUM);
+ ASSERT(command->create_surface.surface_id == 1);
update = create_surface(command->create_surface.surface_id,
command->create_surface.format,
command->create_surface.width,
commit ff03d8dd2a4827c3b81657a1b9b3c9767481bba2
Author: Uri Lublin <uril at redhat.com>
Date: Mon Jul 8 16:17:27 2013 +0300
server/tests: test_display_base: set rect according to appropriate surface
When surface_id == 0, primary is used.
Otherwise (currently 1), secondary is used.
Also, remove unused test_width and test_height.
Since commit caea7699434c20dceef8fc79d21b8eeb663fbf53,
test->width and test->height are used.
diff --git a/server/tests/test_display_base.c b/server/tests/test_display_base.c
index b4c93f3..ad2c687 100644
--- a/server/tests/test_display_base.c
+++ b/server/tests/test_display_base.c
@@ -48,9 +48,6 @@ static void test_spice_destroy_update(SimpleSpiceUpdate *update)
free(update);
}
-static uint32_t test_width;
-static uint32_t test_height;
-
#define DEFAULT_WIDTH 640
#define DEFAULT_HEIGHT 320
@@ -507,9 +504,9 @@ static void produce_command(Test *test)
case SIMPLE_UPDATE: {
QXLRect rect = {
.left = 0,
- .right = (test->target_surface == 0 ? test_width : SURF_WIDTH),
+ .right = (test->target_surface == 0 ? test->primary_width : test->width),
.top = 0,
- .bottom = (test->target_surface == 0 ? test_height : SURF_HEIGHT)
+ .bottom = (test->target_surface == 0 ? test->primary_height : test->height)
};
if (rect.right > 0 && rect.bottom > 0) {
qxl_worker->update_area(qxl_worker, test->target_surface, &rect, NULL, 0, 1);
commit 1960ebb5b391a9633d892c80f84146cdd2c81082
Author: Uri Lublin <uril at redhat.com>
Date: Thu Jul 4 19:50:27 2013 +0300
red_channel: replace RING_FOREACH with RING_FOREACH_SAFE in some places
This was originally intended to fix the problem fixed by
commit 53488f0275d6c8a121af49f7ac817d09ce68090d.
What is left are FOREACH loops that are at less risk and maybe safe (no
read/write or disconnect/destroy are called from within them).
diff --git a/server/red_channel.c b/server/red_channel.c
index 8742008..85d7ebc 100644
--- a/server/red_channel.c
+++ b/server/red_channel.c
@@ -2025,7 +2025,7 @@ void red_client_set_main(RedClient *client, MainChannelClient *mcc) {
void red_client_semi_seamless_migrate_complete(RedClient *client)
{
- RingItem *link;
+ RingItem *link, *next;
pthread_mutex_lock(&client->lock);
if (!client->during_target_migrate || client->seamless_migrate) {
@@ -2034,7 +2034,7 @@ void red_client_semi_seamless_migrate_complete(RedClient *client)
return;
}
client->during_target_migrate = FALSE;
- RING_FOREACH(link, &client->channels) {
+ RING_FOREACH_SAFE(link, next, &client->channels) {
RedChannelClient *rcc = SPICE_CONTAINEROF(link, RedChannelClient, client_link);
if (rcc->latency_monitor.timer) {
@@ -2073,12 +2073,12 @@ static void red_channel_pipes_create_batch(RedChannel *channel,
new_pipe_item_t creator, void *data,
rcc_item_t callback)
{
- RingItem *link;
+ RingItem *link, *next;
RedChannelClient *rcc;
PipeItem *item;
int num = 0;
- RING_FOREACH(link, &channel->clients) {
+ RING_FOREACH_SAFE(link, next, &channel->clients) {
rcc = SPICE_CONTAINEROF(link, RedChannelClient, channel_link);
item = (*creator)(rcc, data, num++);
if (callback) {
commit cf905b7b68605a19d5af7660b89b40a5b6d90ac3
Author: Uri Lublin <uril at redhat.com>
Date: Wed Jul 3 11:35:38 2013 +0300
red_worker: use a generic SAFE_FOREACH macro
Introduce SAFE_FOREACH macro
Make other safe iterators use SAFE_FOREACH
diff --git a/server/red_worker.c b/server/red_worker.c
index e2ab776..597008e 100644
--- a/server/red_worker.c
+++ b/server/red_worker.c
@@ -1113,24 +1113,25 @@ static inline uint64_t red_now(void);
* given a channel, iterate over it's clients
*/
+/* a generic safe for loop macro */
+#define SAFE_FOREACH(link, next, cond, ring, data, get_data) \
+ for ((((link) = ((cond) ? ring_get_head(ring) : NULL)), \
+ ((next) = ((link) ? ring_next((ring), (link)) : NULL)), \
+ ((data) = ((link)? (get_data) : NULL))); \
+ (link); \
+ (((link) = (next)), \
+ ((next) = ((link) ? ring_next((ring), (link)) : NULL)), \
+ ((data) = ((link)? (get_data) : NULL))))
+
+#define LINK_TO_RCC(ptr) SPICE_CONTAINEROF(ptr, RedChannelClient, channel_link)
#define RCC_FOREACH_SAFE(link, next, rcc, channel) \
- for (link = ring_get_head(&(channel)->clients), \
- rcc = SPICE_CONTAINEROF(link, RedChannelClient, channel_link), \
- (next) = (link) ? ring_next(&(channel)->clients, (link)) : NULL; \
- (link); \
- (link) = (next), \
- (next) = (link) ? ring_next(&(channel)->clients, (link)) : NULL, \
- rcc = SPICE_CONTAINEROF(link, RedChannelClient, channel_link))
+ SAFE_FOREACH(link, next, channel, &(channel)->clients, rcc, LINK_TO_RCC(link))
+
+#define LINK_TO_DCC(ptr) SPICE_CONTAINEROF(ptr, DisplayChannelClient, \
+ common.base.channel_link)
#define DCC_FOREACH_SAFE(link, next, dcc, channel) \
- for ((link) = ((channel) ? ring_get_head(&(channel)->clients) : NULL), \
- (next) = ((link) ? ring_next(&(channel)->clients, (link)) : NULL), \
- (dcc) = ((link) ? SPICE_CONTAINEROF((link), DisplayChannelClient, \
- common.base.channel_link) : NULL); \
- (link); \
- (link) = (next), \
- (next) = ((link) ? ring_next(&(channel)->clients, (link)) : NULL), \
- (dcc) = SPICE_CONTAINEROF((link), DisplayChannelClient, common.base.channel_link))
+ SAFE_FOREACH(link, next, channel, &(channel)->clients, dcc, LINK_TO_DCC(link))
#define WORKER_FOREACH_DCC_SAFE(worker, link, next, dcc) \
@@ -1139,23 +1140,15 @@ static inline uint64_t red_now(void);
(&(worker)->display_channel->common.base) : NULL))
+#define LINK_TO_DPI(ptr) SPICE_CONTAINEROF((ptr), DrawablePipeItem, base)
#define DRAWABLE_FOREACH_DPI_SAFE(drawable, link, next, dpi) \
- for (link = (drawable) ? ring_get_head(&(drawable)->pipes) : NULL,\
- (next) = ((link) ? ring_next(&(drawable)->pipes, (link)) : NULL), \
- dpi = (link) ? SPICE_CONTAINEROF((link), DrawablePipeItem, base) : NULL; \
- (link);\
- (link) = (next), \
- (next) = ((link) ? ring_next(&(drawable)->pipes, (link)) : NULL), \
- dpi = (link) ? SPICE_CONTAINEROF((link), DrawablePipeItem, base) : NULL)
+ SAFE_FOREACH(link, next, drawable, &(drawable)->pipes, dpi, LINK_TO_DPI(link))
+
+#define LINK_TO_GLZ(ptr) SPICE_CONTAINEROF((ptr), RedGlzDrawable, \
+ drawable_link)
#define DRAWABLE_FOREACH_GLZ_SAFE(drawable, link, next, glz) \
- for (link = (drawable) ? ring_get_head(&drawable->glz_ring) : NULL,\
- next = (link) ? ring_next(&drawable->glz_ring, link) : NULL,\
- glz = (link) ? SPICE_CONTAINEROF((link), RedGlzDrawable, drawable_link) : NULL;\
- (link);\
- (link) = (next),\
- (next) = (link) ? ring_next(&drawable->glz_ring, (link)) : NULL,\
- glz = (link) ? SPICE_CONTAINEROF((link), RedGlzDrawable, drawable_link) : NULL)
+ SAFE_FOREACH(link, next, drawable, &(drawable)->glz_ring, glz, LINK_TO_GLZ(link))
#define DCC_TO_WORKER(dcc) \
commit 6c95bb3c59a8eca79d53f8f8561690beb60363b1
Author: Uri Lublin <uril at redhat.com>
Date: Thu Jun 27 01:09:20 2013 +0300
red_worker: delete unused CCC_FOREACH
diff --git a/server/red_worker.c b/server/red_worker.c
index 72e5ea5..e2ab776 100644
--- a/server/red_worker.c
+++ b/server/red_worker.c
@@ -1157,12 +1157,6 @@ static inline uint64_t red_now(void);
(next) = (link) ? ring_next(&drawable->glz_ring, (link)) : NULL,\
glz = (link) ? SPICE_CONTAINEROF((link), RedGlzDrawable, drawable_link) : NULL)
-#define CCC_FOREACH(link, ccc, channel) \
- for (link = ring_get_head(&(channel)->clients),\
- ccc = SPICE_CONTAINEROF(link, CommonChannelClient, base.channel_link);\
- (link); \
- (link) = ring_next(&(channel)->clients, link),\
- ccc = SPICE_CONTAINEROF(link, CommonChannelClient, base.channel_link))
#define DCC_TO_WORKER(dcc) \
(SPICE_CONTAINEROF((dcc)->common.base.channel, CommonChannel, base)->worker)
commit 57dba5615e33a8920fa2d505861712fa507c482c
Author: Uri Lublin <uril at redhat.com>
Date: Thu Jun 27 00:53:45 2013 +0300
red_worker: make DRAWABLE_FOREACH_DPI safe
diff --git a/server/red_worker.c b/server/red_worker.c
index ffd278c..72e5ea5 100644
--- a/server/red_worker.c
+++ b/server/red_worker.c
@@ -1139,12 +1139,14 @@ static inline uint64_t red_now(void);
(&(worker)->display_channel->common.base) : NULL))
-#define DRAWABLE_FOREACH_DPI(drawable, link, dpi) \
+#define DRAWABLE_FOREACH_DPI_SAFE(drawable, link, next, dpi) \
for (link = (drawable) ? ring_get_head(&(drawable)->pipes) : NULL,\
+ (next) = ((link) ? ring_next(&(drawable)->pipes, (link)) : NULL), \
dpi = (link) ? SPICE_CONTAINEROF((link), DrawablePipeItem, base) : NULL; \
(link);\
- (link) = ring_next(&(drawable)->pipes, (link)),\
- dpi = (link) ? SPICE_CONTAINEROF((link), DrawablePipeItem, base) : NULL)
+ (link) = (next), \
+ (next) = ((link) ? ring_next(&(drawable)->pipes, (link)) : NULL), \
+ dpi = (link) ? SPICE_CONTAINEROF((link), DrawablePipeItem, base) : NULL)
#define DRAWABLE_FOREACH_GLZ_SAFE(drawable, link, next, glz) \
for (link = (drawable) ? ring_get_head(&drawable->glz_ring) : NULL,\
@@ -1533,11 +1535,11 @@ static inline void red_pipes_add_drawable_after(RedWorker *worker,
Drawable *drawable, Drawable *pos_after)
{
DrawablePipeItem *dpi, *dpi_pos_after;
- RingItem *dpi_link;
+ RingItem *dpi_link, *dpi_next;
DisplayChannelClient *dcc;
int num_other_linked = 0;
- DRAWABLE_FOREACH_DPI(pos_after, dpi_link, dpi_pos_after) {
+ DRAWABLE_FOREACH_DPI_SAFE(pos_after, dpi_link, dpi_next, dpi_pos_after) {
num_other_linked++;
dcc = dpi_pos_after->dcc;
red_handle_drawable_surfaces_client_synced(dcc, drawable);
@@ -1554,7 +1556,7 @@ static inline void red_pipes_add_drawable_after(RedWorker *worker,
spice_debug("TODO: not O(n^2)");
WORKER_FOREACH_DCC_SAFE(worker, worker_item, next, dcc) {
int sent = 0;
- DRAWABLE_FOREACH_DPI(pos_after, dpi_link, dpi_pos_after) {
+ DRAWABLE_FOREACH_DPI_SAFE(pos_after, dpi_link, dpi_next, dpi_pos_after) {
if (dpi_pos_after->dcc == dcc) {
sent = 1;
break;
@@ -2687,9 +2689,9 @@ static void red_stop_stream(RedWorker *worker, Stream *stream)
static int red_display_drawable_is_in_pipe(DisplayChannelClient *dcc, Drawable *drawable)
{
DrawablePipeItem *dpi;
- RingItem *dpi_link;
+ RingItem *dpi_link, *dpi_next;
- DRAWABLE_FOREACH_DPI(drawable, dpi_link, dpi) {
+ DRAWABLE_FOREACH_DPI_SAFE(drawable, dpi_link, dpi_next, dpi) {
if (dpi->dcc == dcc) {
return TRUE;
}
@@ -3323,7 +3325,7 @@ static inline void pre_stream_item_swap(RedWorker *worker, Stream *stream, Drawa
}
index = get_stream_id(worker, stream);
- DRAWABLE_FOREACH_DPI(stream->current, ring_item, dpi) {
+ DRAWABLE_FOREACH_DPI_SAFE(stream->current, ring_item, next, dpi) {
dcc = dpi->dcc;
agent = &dcc->stream_agents[index];
commit 2af81e96f54d9deed52231b4c8cea79623b68fab
Author: Uri Lublin <uril at redhat.com>
Date: Thu Jun 27 00:36:34 2013 +0300
red_worker: use only DRAWABLE_FOREACH_GLZ_SAFE
diff --git a/server/red_worker.c b/server/red_worker.c
index 0599a0e..ffd278c 100644
--- a/server/red_worker.c
+++ b/server/red_worker.c
@@ -1146,13 +1146,6 @@ static inline uint64_t red_now(void);
(link) = ring_next(&(drawable)->pipes, (link)),\
dpi = (link) ? SPICE_CONTAINEROF((link), DrawablePipeItem, base) : NULL)
-#define DRAWABLE_FOREACH_GLZ(drawable, link, glz) \
- for (link = (drawable) ? ring_get_head(&drawable->glz_ring) : NULL,\
- glz = (link) ? SPICE_CONTAINEROF((link), RedGlzDrawable, drawable_link) : NULL;\
- (link);\
- (link) = ring_next(&drawable->glz_ring, (link)),\
- glz = (link) ? SPICE_CONTAINEROF((link), RedGlzDrawable, drawable_link) : NULL)
-
#define DRAWABLE_FOREACH_GLZ_SAFE(drawable, link, next, glz) \
for (link = (drawable) ? ring_get_head(&drawable->glz_ring) : NULL,\
next = (link) ? ring_next(&drawable->glz_ring, link) : NULL,\
@@ -5547,12 +5540,12 @@ static void red_display_destroy_compress_bufs(DisplayChannel *display_channel)
static RedGlzDrawable *red_display_get_glz_drawable(DisplayChannelClient *dcc, Drawable *drawable)
{
RedGlzDrawable *ret;
- RingItem *item;
+ RingItem *item, *next;
// TODO - I don't really understand what's going on here, so doing the technical equivalent
// now that we have multiple glz_dicts, so the only way to go from dcc to drawable glz is to go
// over the glz_ring (unless adding some better data structure then a ring)
- DRAWABLE_FOREACH_GLZ(drawable, item, ret) {
+ DRAWABLE_FOREACH_GLZ_SAFE(drawable, item, next, ret) {
if (ret->dcc == dcc) {
return ret;
}
commit b1330fcd5d49964a724ce0cfca755318f2918a35
Author: Uri Lublin <uril at redhat.com>
Date: Thu Jun 27 00:29:22 2013 +0300
red_worker: make WORKER_FOREACH_DCC safe
Specifically, the loop in red_pipes_add_draw can cause spice to abort.
In red_worker.c (WORKER_FOREACH_DCC):
red_pipes_add_drawable
red_pipe_add_drawable
red_handle_drawable_surfaces_client_synced
red_push_surface_image
red_channel_client_push
red_channel_client_send
red_peer_handle_outgoing
reds_stream_writev (if fails -- EPIPE)
handler->cb->on_error = red_channel_client_disconnect()
red_channel_remove_client()
ring_remove() -- of rcc from channel.clients ring.
diff --git a/server/red_worker.c b/server/red_worker.c
index 8503d16..0599a0e 100644
--- a/server/red_worker.c
+++ b/server/red_worker.c
@@ -1122,16 +1122,19 @@ static inline uint64_t red_now(void);
(next) = (link) ? ring_next(&(channel)->clients, (link)) : NULL, \
rcc = SPICE_CONTAINEROF(link, RedChannelClient, channel_link))
-#define DCC_FOREACH(link, dcc, channel) \
- for (link = channel ? ring_get_head(&(channel)->clients) : NULL,\
- dcc = link ? SPICE_CONTAINEROF((link), DisplayChannelClient,\
- common.base.channel_link) : NULL;\
- (link); \
- (link) = ring_next(&(channel)->clients, link),\
- dcc = SPICE_CONTAINEROF((link), DisplayChannelClient, common.base.channel_link))
-
-#define WORKER_FOREACH_DCC(worker, link, dcc) \
- DCC_FOREACH(link, dcc, \
+#define DCC_FOREACH_SAFE(link, next, dcc, channel) \
+ for ((link) = ((channel) ? ring_get_head(&(channel)->clients) : NULL), \
+ (next) = ((link) ? ring_next(&(channel)->clients, (link)) : NULL), \
+ (dcc) = ((link) ? SPICE_CONTAINEROF((link), DisplayChannelClient, \
+ common.base.channel_link) : NULL); \
+ (link); \
+ (link) = (next), \
+ (next) = ((link) ? ring_next(&(channel)->clients, (link)) : NULL), \
+ (dcc) = SPICE_CONTAINEROF((link), DisplayChannelClient, common.base.channel_link))
+
+
+#define WORKER_FOREACH_DCC_SAFE(worker, link, next, dcc) \
+ DCC_FOREACH_SAFE(link, next, dcc, \
((((worker) && (worker)->display_channel))? \
(&(worker)->display_channel->common.base) : NULL))
@@ -1513,10 +1516,10 @@ static inline void red_pipe_add_drawable(DisplayChannelClient *dcc, Drawable *dr
static inline void red_pipes_add_drawable(RedWorker *worker, Drawable *drawable)
{
DisplayChannelClient *dcc;
- RingItem *dcc_ring_item;
+ RingItem *dcc_ring_item, *next;
spice_warn_if(!ring_is_empty(&drawable->pipes));
- WORKER_FOREACH_DCC(worker, dcc_ring_item, dcc) {
+ WORKER_FOREACH_DCC_SAFE(worker, dcc_ring_item, next, dcc) {
red_pipe_add_drawable(dcc, drawable);
}
}
@@ -1554,9 +1557,9 @@ static inline void red_pipes_add_drawable_after(RedWorker *worker,
return;
}
if (num_other_linked != worker->display_channel->common.base.clients_num) {
- RingItem *worker_item;
+ RingItem *worker_item, *next;
spice_debug("TODO: not O(n^2)");
- WORKER_FOREACH_DCC(worker, worker_item, dcc) {
+ WORKER_FOREACH_DCC_SAFE(worker, worker_item, next, dcc) {
int sent = 0;
DRAWABLE_FOREACH_DPI(pos_after, dpi_link, dpi_pos_after) {
if (dpi_pos_after->dcc == dcc) {
@@ -1743,7 +1746,7 @@ static inline void red_destroy_surface(RedWorker *worker, uint32_t surface_id)
{
RedSurface *surface = &worker->surfaces[surface_id];
DisplayChannelClient *dcc;
- RingItem *link;
+ RingItem *link, *next;
if (!--surface->refs) {
// only primary surface streams are supported
@@ -1762,7 +1765,7 @@ static inline void red_destroy_surface(RedWorker *worker, uint32_t surface_id)
region_destroy(&surface->draw_dirty_region);
surface->context.canvas = NULL;
- WORKER_FOREACH_DCC(worker, link, dcc) {
+ WORKER_FOREACH_DCC_SAFE(worker, link, next, dcc) {
red_destroy_surface_item(worker, dcc, surface_id);
}
@@ -2122,10 +2125,10 @@ static void red_wait_outgoing_item(RedChannelClient *rcc);
static void red_clear_surface_drawables_from_pipes(RedWorker *worker, int surface_id,
int force, int wait_for_outgoing_item)
{
- RingItem *item;
+ RingItem *item, *next;
DisplayChannelClient *dcc;
- WORKER_FOREACH_DCC(worker, item, dcc) {
+ WORKER_FOREACH_DCC_SAFE(worker, item, next, dcc) {
red_clear_surface_drawables_from_pipe(dcc, surface_id, force);
if (wait_for_outgoing_item) {
// in case that the pipe didn't contain any item that is dependent on the surface, but
@@ -2587,7 +2590,7 @@ static inline int get_stream_id(RedWorker *worker, Stream *stream)
static void red_attach_stream(RedWorker *worker, Drawable *drawable, Stream *stream)
{
DisplayChannelClient *dcc;
- RingItem *item;
+ RingItem *item, *next;
spice_assert(!drawable->stream && !stream->current);
spice_assert(drawable && stream);
@@ -2596,7 +2599,7 @@ static void red_attach_stream(RedWorker *worker, Drawable *drawable, Stream *str
stream->last_time = drawable->creation_time;
stream->num_input_frames++;
- WORKER_FOREACH_DCC(worker, item, dcc) {
+ WORKER_FOREACH_DCC_SAFE(worker, item, next, dcc) {
StreamAgent *agent;
QRegion clip_in_draw_dest;
@@ -2657,12 +2660,12 @@ static void red_print_stream_stats(DisplayChannelClient *dcc, StreamAgent *agent
static void red_stop_stream(RedWorker *worker, Stream *stream)
{
DisplayChannelClient *dcc;
- RingItem *item;
+ RingItem *item, *next;
spice_assert(ring_item_is_linked(&stream->link));
spice_assert(!stream->current);
spice_debug("stream %d", get_stream_id(worker, stream));
- WORKER_FOREACH_DCC(worker, item, dcc) {
+ WORKER_FOREACH_DCC_SAFE(worker, item, next, dcc) {
StreamAgent *stream_agent;
stream_agent = &dcc->stream_agents[get_stream_id(worker, stream)];
@@ -2776,10 +2779,10 @@ clear_vis_region:
static inline void red_detach_stream_gracefully(RedWorker *worker, Stream *stream,
Drawable *update_area_limit)
{
- RingItem *item;
+ RingItem *item, *next;
DisplayChannelClient *dcc;
- WORKER_FOREACH_DCC(worker, item, dcc) {
+ WORKER_FOREACH_DCC_SAFE(worker, item, next, dcc) {
red_display_detach_stream_gracefully(dcc, stream, update_area_limit);
}
if (stream->current) {
@@ -2801,7 +2804,7 @@ static void red_detach_streams_behind(RedWorker *worker, QRegion *region, Drawab
{
Ring *ring = &worker->streams;
RingItem *item = ring_get_head(ring);
- RingItem *dcc_ring_item;
+ RingItem *dcc_ring_item, *next;
DisplayChannelClient *dcc;
int has_clients = display_is_connected(worker);
@@ -2810,7 +2813,7 @@ static void red_detach_streams_behind(RedWorker *worker, QRegion *region, Drawab
int detach_stream = 0;
item = ring_next(ring, item);
- WORKER_FOREACH_DCC(worker, dcc_ring_item, dcc) {
+ WORKER_FOREACH_DCC_SAFE(worker, dcc_ring_item, next, dcc) {
StreamAgent *agent = &dcc->stream_agents[get_stream_id(worker, stream)];
if (region_intersects(&agent->vis_region, region)) {
@@ -2834,7 +2837,7 @@ static void red_streams_update_visible_region(RedWorker *worker, Drawable *drawa
{
Ring *ring;
RingItem *item;
- RingItem *dcc_ring_item;
+ RingItem *dcc_ring_item, *next;
DisplayChannelClient *dcc;
if (!display_is_connected(worker)) {
@@ -2858,7 +2861,7 @@ static void red_streams_update_visible_region(RedWorker *worker, Drawable *drawa
continue;
}
- WORKER_FOREACH_DCC(worker, dcc_ring_item, dcc) {
+ WORKER_FOREACH_DCC_SAFE(worker, dcc_ring_item, next, dcc) {
agent = &dcc->stream_agents[get_stream_id(worker, stream)];
if (region_intersects(&agent->vis_region, &drawable->tree_item.base.rgn)) {
@@ -3125,7 +3128,7 @@ static void red_stream_input_fps_timer_cb(void *opaque)
static void red_create_stream(RedWorker *worker, Drawable *drawable)
{
DisplayChannelClient *dcc;
- RingItem *dcc_ring_item;
+ RingItem *dcc_ring_item, *next;
Stream *stream;
SpiceRect* src_rect;
@@ -3156,7 +3159,7 @@ static void red_create_stream(RedWorker *worker, Drawable *drawable)
stream->input_fps = MAX_FPS;
worker->streams_size_total += stream->width * stream->height;
worker->stream_count++;
- WORKER_FOREACH_DCC(worker, dcc_ring_item, dcc) {
+ WORKER_FOREACH_DCC_SAFE(worker, dcc_ring_item, next, dcc) {
red_display_create_stream(dcc, stream);
}
spice_debug("stream %d %dx%d (%d, %d) (%d, %d)", (int)(stream - worker->streams_buf), stream->width,
@@ -3313,7 +3316,7 @@ static inline void pre_stream_item_swap(RedWorker *worker, Stream *stream, Drawa
DisplayChannelClient *dcc;
int index;
StreamAgent *agent;
- RingItem *ring_item;
+ RingItem *ring_item, *next;
spice_assert(stream->current);
@@ -3349,7 +3352,7 @@ static inline void pre_stream_item_swap(RedWorker *worker, Stream *stream, Drawa
}
- WORKER_FOREACH_DCC(worker, ring_item, dcc) {
+ WORKER_FOREACH_DCC_SAFE(worker, ring_item, next, dcc) {
double drop_factor;
agent = &dcc->stream_agents[index];
@@ -5278,11 +5281,11 @@ static void red_free_some(RedWorker *worker)
{
int n = 0;
DisplayChannelClient *dcc;
- RingItem *item;
+ RingItem *item, *next;
spice_debug("#draw=%d, #red_draw=%d, #glz_draw=%d", worker->drawable_count,
worker->red_drawable_count, worker->glz_drawable_count);
- WORKER_FOREACH_DCC(worker, item, dcc) {
+ WORKER_FOREACH_DCC_SAFE(worker, item, next, dcc) {
GlzSharedDictionary *glz_dict = dcc ? dcc->glz_dict : NULL;
if (glz_dict) {
@@ -5297,7 +5300,7 @@ static void red_free_some(RedWorker *worker)
free_one_drawable(worker, TRUE);
}
- WORKER_FOREACH_DCC(worker, item, dcc) {
+ WORKER_FOREACH_DCC_SAFE(worker, item, next, dcc) {
GlzSharedDictionary *glz_dict = dcc ? dcc->glz_dict : NULL;
if (glz_dict) {
@@ -5710,13 +5713,13 @@ static void red_display_client_clear_glz_drawables(DisplayChannelClient *dcc)
static void red_display_clear_glz_drawables(DisplayChannel *display_channel)
{
- RingItem *link;
+ RingItem *link, *next;
DisplayChannelClient *dcc;
if (!display_channel) {
return;
}
- DCC_FOREACH(link, dcc, &display_channel->common.base) {
+ DCC_FOREACH_SAFE(link, next, dcc, &display_channel->common.base) {
red_display_client_clear_glz_drawables(dcc);
}
}
@@ -9637,9 +9640,9 @@ static inline void red_create_surface_item(DisplayChannelClient *dcc, int surfac
static void red_worker_create_surface_item(RedWorker *worker, int surface_id)
{
DisplayChannelClient *dcc;
- RingItem *item;
+ RingItem *item, *next;
- WORKER_FOREACH_DCC(worker, item, dcc) {
+ WORKER_FOREACH_DCC_SAFE(worker, item, next, dcc) {
red_create_surface_item(dcc, surface_id);
}
}
@@ -9648,9 +9651,9 @@ static void red_worker_create_surface_item(RedWorker *worker, int surface_id)
static void red_worker_push_surface_image(RedWorker *worker, int surface_id)
{
DisplayChannelClient *dcc;
- RingItem *item;
+ RingItem *item, *next;
- WORKER_FOREACH_DCC(worker, item, dcc) {
+ WORKER_FOREACH_DCC_SAFE(worker, item, next, dcc) {
red_push_surface_image(dcc, surface_id);
}
}
@@ -10738,7 +10741,7 @@ static void guest_set_client_capabilities(RedWorker *worker)
int i;
DisplayChannelClient *dcc;
RedChannelClient *rcc;
- RingItem *link;
+ RingItem *link, *next;
uint8_t caps[58] = { 0 };
int caps_available[] = {
SPICE_DISPLAY_CAP_SIZED_STREAM,
@@ -10771,7 +10774,7 @@ static void guest_set_client_capabilities(RedWorker *worker)
for (i = 0 ; i < sizeof(caps_available) / sizeof(caps_available[0]); ++i) {
SET_CAP(caps, caps_available[i]);
}
- DCC_FOREACH(link, dcc, &worker->display_channel->common.base) {
+ DCC_FOREACH_SAFE(link, next, dcc, &worker->display_channel->common.base) {
rcc = (RedChannelClient *)dcc;
for (i = 0 ; i < sizeof(caps_available) / sizeof(caps_available[0]); ++i) {
if (!red_channel_client_test_remote_cap(rcc, caps_available[i]))
@@ -11386,9 +11389,9 @@ static void red_push_monitors_config(DisplayChannelClient *dcc)
static void red_worker_push_monitors_config(RedWorker *worker)
{
DisplayChannelClient *dcc;
- RingItem *item;
+ RingItem *item, *next;
- WORKER_FOREACH_DCC(worker, item, dcc) {
+ WORKER_FOREACH_DCC_SAFE(worker, item, next, dcc) {
red_push_monitors_config(dcc);
}
}
commit e4029833da5bde9a70993daeb78345a0f331d1c6
Author: Uri Lublin <uril at redhat.com>
Date: Wed Jun 26 23:34:58 2013 +0300
red_worker: reuse DCC_FOREACH in WORKER_DCC_FOREACH
The only thing that is needed is to get the channel out of the worker.
diff --git a/server/red_worker.c b/server/red_worker.c
index a7f8d79..8503d16 100644
--- a/server/red_worker.c
+++ b/server/red_worker.c
@@ -1131,13 +1131,10 @@ static inline uint64_t red_now(void);
dcc = SPICE_CONTAINEROF((link), DisplayChannelClient, common.base.channel_link))
#define WORKER_FOREACH_DCC(worker, link, dcc) \
- for (link = ((worker) && (worker)->display_channel) ?\
- ring_get_head(&(worker)->display_channel->common.base.clients) : NULL,\
- dcc = link ? SPICE_CONTAINEROF((link), DisplayChannelClient,\
- common.base.channel_link) : NULL;\
- (link); \
- (link) = ring_next(&(worker)->display_channel->common.base.clients, link),\
- dcc = SPICE_CONTAINEROF((link), DisplayChannelClient, common.base.channel_link))
+ DCC_FOREACH(link, dcc, \
+ ((((worker) && (worker)->display_channel))? \
+ (&(worker)->display_channel->common.base) : NULL))
+
#define DRAWABLE_FOREACH_DPI(drawable, link, dpi) \
for (link = (drawable) ? ring_get_head(&(drawable)->pipes) : NULL,\
commit 255abf0a5726ed203937dce61ec570842c137708
Author: Uri Lublin <uril at redhat.com>
Date: Wed Jun 26 22:20:01 2013 +0300
red_worker: use only RCC_FOREACH_SAFE
RCC_FOREACH may be dangerous
The following patches replace FOREACH loops with a SAFE version.
Using unsafe loops may cause spice-server to abort (assert fails).
Specifically a read/write fail in those loops, may cause the client
to disconnect, removing the node currently iterated, which cause spice
to abort in ring_next():
-- assertion `pos->next != NULL && pos->prev != NULL' failed
diff --git a/server/red_worker.c b/server/red_worker.c
index 825bca0..a7f8d79 100644
--- a/server/red_worker.c
+++ b/server/red_worker.c
@@ -1113,13 +1113,6 @@ static inline uint64_t red_now(void);
* given a channel, iterate over it's clients
*/
-#define RCC_FOREACH(link, rcc, channel) \
- for (link = ring_get_head(&(channel)->clients),\
- rcc = SPICE_CONTAINEROF(link, RedChannelClient, channel_link);\
- (link); \
- (link) = ring_next(&(channel)->clients, link),\
- rcc = SPICE_CONTAINEROF(link, RedChannelClient, channel_link))
-
#define RCC_FOREACH_SAFE(link, next, rcc, channel) \
for (link = ring_get_head(&(channel)->clients), \
rcc = SPICE_CONTAINEROF(link, RedChannelClient, channel_link), \
@@ -1426,9 +1419,9 @@ static void red_push_surface_image(DisplayChannelClient *dcc, int surface_id);
static void red_pipes_add_verb(RedChannel *channel, uint16_t verb)
{
RedChannelClient *rcc;
- RingItem *link;
+ RingItem *link, *next;
- RCC_FOREACH(link, rcc, channel) {
+ RCC_FOREACH_SAFE(link, next, rcc, channel) {
red_pipe_add_verb(rcc, verb);
}
}
More information about the Spice-commits
mailing list