To avoid checking for error in all the path use setjmp/longjmp.
This allow to jump directly to the error handling code in
spice_replay_next_cmd.
Signed-off-by: Frediano Ziglio <fziglio@xxxxxxxxxx>
---
server/red-replay-qxl.c | 142 ++++++++++++------------------------------------
1 file changed, 35 insertions(+), 107 deletions(-)
I'm not a great fun of setjmp/longjmp but I have to say
this way simplify different checks.
diff --git a/server/red-replay-qxl.c b/server/red-replay-qxl.c
index 78de48b..daaca0d 100644
--- a/server/red-replay-qxl.c
+++ b/server/red-replay-qxl.c
@@ -23,13 +23,16 @@
#include <inttypes.h>
#include <zlib.h>
#include <pthread.h>
+#include <glib.h>
+#include <assert.h>
+#include <setjmp.h>
+
#include "reds.h"
#include "red-qxl.h"
#include "red-common.h"
#include "memslot.h"
#include "red-parse-qxl.h"
#include "red-replay-qxl.h"
-#include <glib.h>
#define QXLPHYSICAL_FROM_PTR(ptr) ((QXLPHYSICAL)(intptr_t)(ptr))
#define QXLPHYSICAL_TO_PTR(phy) ((void*)(intptr_t)(phy))
@@ -42,6 +45,7 @@ typedef enum {
struct SpiceReplay {
FILE *fd;
gboolean error;
+ jmp_buf jmp_error;
int counter;
bool created_primary;
@@ -57,18 +61,24 @@ struct SpiceReplay {
pthread_cond_t cond;
};
+static void SPICE_GNUC_NORETURN replay_got_error(SpiceReplay *replay)
+{
+ replay->error = TRUE;
+ longjmp(replay->jmp_error, 1);
+}
+
static ssize_t replay_fread(SpiceReplay *replay, uint8_t *buf, size_t size)
{
if (replay->error || feof(replay->fd) ||
fread(buf, 1, size, replay->fd) != size) {
- replay->error = TRUE;
+ replay_got_error(replay);
return 0;
}
return size;
}
__attribute__((format(scanf, 2, 3)))
-static replay_t replay_fscanf_check(SpiceReplay *replay, const char *fmt, ...)
+static void replay_fscanf_check(SpiceReplay *replay, const char *fmt, ...)
{
va_list ap;
int ret;
@@ -76,19 +86,17 @@ static replay_t replay_fscanf_check(SpiceReplay *replay, const char *fmt, ...)
replay->end_pos = -1;
if (replay->error) {
- return REPLAY_ERROR;
+ replay_got_error(replay);
}
if (feof(replay->fd)) {
- replay->error = TRUE;
- return REPLAY_ERROR;
+ replay_got_error(replay);
}
va_start(ap, fmt);
ret = vfscanf(replay->fd, fmt, ap);
va_end(ap);
if (ret == EOF || replay->end_pos < 0) {
- replay->error = TRUE;
+ replay_got_error(replay);
}
- return replay->error ? REPLAY_ERROR : REPLAY_OK;
}
#define replay_fscanf(r, fmt, ...) \
replay_fscanf_check(r, fmt "%n", ## __VA_ARGS__, &r->end_pos)
@@ -217,8 +225,8 @@ static void hexdump(uint8_t *hex, uint8_t bytes)
}
#endif
-static replay_t read_binary(SpiceReplay *replay, const char *prefix, size_t *size, uint8_t
- **buf, size_t base_size)
+static void read_binary(SpiceReplay *replay, const char *prefix, size_t *size,
+ uint8_t **buf, size_t base_size)
{
char template[1024];
int with_zlib = -1;
@@ -228,9 +236,6 @@ static replay_t read_binary(SpiceReplay *replay, const char *prefix, size_t *siz
snprintf(template, sizeof(template), "binary %%d %s %%ld:%%n", prefix);
replay_fscanf_check(replay, template, &with_zlib, size, &replay->end_pos);
- if (replay->error) {
- return REPLAY_ERROR;
- }
if (*buf == NULL) {
*buf = replay_malloc(replay, *size + base_size);
@@ -246,13 +251,8 @@ static replay_t read_binary(SpiceReplay *replay, const char *prefix, size_t *siz
int ret;
replay_fscanf(replay, "%u:", &zlib_size);
- if (replay->error) {
- return REPLAY_ERROR;
- }
zlib_buffer = replay_malloc(replay, zlib_size);
- if (replay_fread(replay, zlib_buffer, zlib_size) != zlib_size) {
- return REPLAY_ERROR;
- }
+ replay_fread(replay, zlib_buffer, zlib_size);
strm.zalloc = Z_NULL;
strm.zfree = Z_NULL;
strm.opaque = Z_NULL;
@@ -272,7 +272,7 @@ static replay_t read_binary(SpiceReplay *replay, const char *prefix, size_t *siz
* it seems it may kill the red_worker thread (so a chunk is
* left hanging and the rest of the message is never written).
* Let it pass */
- return REPLAY_ERROR;
+ replay_got_error(replay);
}
if (ret != Z_OK) {
spice_warn_if_reached();
@@ -284,7 +284,7 @@ static replay_t read_binary(SpiceReplay *replay, const char *prefix, size_t *siz
replay_fread(replay, *buf + base_size, *size);
}
#endif
- return replay_fscanf(replay, "\n");
+ replay_fscanf(replay, "\n");
}
static ssize_t red_replay_data_chunks(SpiceReplay *replay, const char *prefix,
@@ -296,25 +296,19 @@ static ssize_t red_replay_data_chunks(SpiceReplay *replay, const char *prefix,
QXLDataChunk *cur, *next;
replay_fscanf(replay, "data_chunks %u %zu\n", &count_chunks, &data_size);
- if (replay->error) {
- return -1;
- }
if (base_size == 0) {
base_size = sizeof(QXLDataChunk);
}
- if (read_binary(replay, prefix, &next_data_size, mem, base_size) == REPLAY_ERROR) {
- return -1;
- }
+ read_binary(replay, prefix, &next_data_size, mem, base_size);
+
cur = (QXLDataChunk*)(*mem + base_size - sizeof(QXLDataChunk));
cur->data_size = next_data_size;
data_size = cur->data_size;
cur->next_chunk = cur->prev_chunk = 0;
while (count_chunks-- > 0) {
- if (read_binary(replay, prefix, &next_data_size, (uint8_t**)&cur->next_chunk,
- sizeof(QXLDataChunk)) == REPLAY_ERROR) {
- return -1;
- }
+ read_binary(replay, prefix, &next_data_size, (uint8_t**)&cur->next_chunk,
+ sizeof(QXLDataChunk));
data_size += next_data_size;
next = QXLPHYSICAL_TO_PTR(cur->next_chunk);
next->prev_chunk = QXLPHYSICAL_FROM_PTR(cur);
@@ -389,9 +383,6 @@ static QXLClipRects *red_replay_clip_rects(SpiceReplay *replay)
unsigned int num_rects;
replay_fscanf(replay, "num_rects %u\n", &num_rects);
- if (replay->error) {
- return NULL;
- }
if (red_replay_data_chunks(replay, "clip_rects", (uint8_t**)&qxl, sizeof(QXLClipRects)) < 0) {
return NULL;
}
@@ -423,9 +414,6 @@ static QXLImage *red_replay_image(SpiceReplay *replay, uint32_t flags)
int has_image;
replay_fscanf(replay, "image %d\n", &has_image);
- if (replay->error) {
- return NULL;
- }
if (!has_image) {
return NULL;
}
@@ -436,9 +424,6 @@ static QXLImage *red_replay_image(SpiceReplay *replay, uint32_t flags)
replay_fscanf(replay, "descriptor.flags %d\n", &temp); qxl->descriptor.flags = temp;
replay_fscanf(replay, "descriptor.width %d\n", &qxl->descriptor.width);
replay_fscanf(replay, "descriptor.height %d\n", &qxl->descriptor.height);
- if (replay->error) {
- return NULL;
- }
switch (qxl->descriptor.type) {
case SPICE_IMAGE_TYPE_BITMAP:
@@ -454,9 +439,6 @@ static QXLImage *red_replay_image(SpiceReplay *replay, uint32_t flags)
unsigned int i, num_ents;
replay_fscanf(replay, "qp.num_ents %u\n", &num_ents);
- if (replay->error) {
- return NULL;
- }
qp = replay_malloc(replay, sizeof(QXLPalette) + num_ents * sizeof(qp->ents[0]));
qp->num_ents = num_ents;
qxl->bitmap.palette = QXLPHYSICAL_FROM_PTR(qp);
@@ -481,18 +463,12 @@ static QXLImage *red_replay_image(SpiceReplay *replay, uint32_t flags)
break;
case SPICE_IMAGE_TYPE_SURFACE:
replay_fscanf(replay, "surface_image.surface_id %d\n", &qxl->surface_image.surface_id);
- if (replay->error) {
- return NULL;
- }
qxl->surface_image.surface_id = replay_id_get(replay, qxl->surface_image.surface_id);
break;
case SPICE_IMAGE_TYPE_QUIC:
// TODO - make this much nicer (precompute size and allocs, store them during
// record, then reread into them. and use MPEG-4).
replay_fscanf(replay, "quic.data_size %d\n", &qxl->quic.data_size);
- if (replay->error) {
- return NULL;
- }
qxl = replay_realloc(replay, qxl, sizeof(QXLImageDescriptor) + sizeof(QXLQUICData) +
qxl->quic.data_size);
size = red_replay_data_chunks(replay, "quic.data", (uint8_t**)&qxl->quic.data, 0);
@@ -534,9 +510,6 @@ static void red_replay_image_free(SpiceReplay *replay, QXLPHYSICAL p, uint32_t f
static void red_replay_brush_ptr(SpiceReplay *replay, QXLBrush *qxl, uint32_t flags)
{
replay_fscanf(replay, "type %d\n", &qxl->type);
- if (replay->error) {
- return;
- }
switch (qxl->type) {
case SPICE_BRUSH_TYPE_SOLID:
@@ -702,9 +675,6 @@ static void red_replay_stroke_ptr(SpiceReplay *replay, QXLStroke *qxl, uint32_t
qxl->path = QXLPHYSICAL_FROM_PTR(red_replay_path(replay));
replay_fscanf(replay, "attr.flags %d\n", &temp); qxl->attr.flags = temp;
- if (replay->error) {
- return;
- }
if (qxl->attr.flags & SPICE_LINE_FLAGS_STYLED) {
size_t size;
@@ -805,9 +775,6 @@ static void red_replay_invers_free(SpiceReplay *replay, QXLInvers *qxl, uint32_t
static void red_replay_clip_ptr(SpiceReplay *replay, QXLClip *qxl)
{
replay_fscanf(replay, "type %d\n", &qxl->type);
- if (replay->error) {
- return;
- }
switch (qxl->type) {
case SPICE_CLIP_TYPE_RECTS:
qxl->data = QXLPHYSICAL_FROM_PTR(red_replay_clip_rects(replay));
@@ -877,24 +844,15 @@ static QXLDrawable *red_replay_native_drawable(SpiceReplay *replay, uint32_t fla
replay_fscanf(replay, "self_bitmap %d\n", &temp); qxl->self_bitmap = temp;
red_replay_rect_ptr(replay, "self_bitmap_area", &qxl->self_bitmap_area);
replay_fscanf(replay, "surface_id %d\n", &qxl->surface_id);
- if (replay->error) {
- return NULL;
- }
qxl->surface_id = replay_id_get(replay, qxl->surface_id);
for (i = 0; i < 3; i++) {
replay_fscanf(replay, "surfaces_dest %d\n", &qxl->surfaces_dest[i]);
- if (replay->error) {
- return NULL;
- }
qxl->surfaces_dest[i] = replay_id_get(replay, qxl->surfaces_dest[i]);
red_replay_rect_ptr(replay, "surfaces_rects", &qxl->surfaces_rects[i]);
}
replay_fscanf(replay, "type %d\n", &temp); qxl->type = temp;
- if (replay->error) {
- return NULL;
- }
switch (qxl->type) {
case QXL_DRAW_ALPHA_BLEND:
red_replay_alpha_blend_ptr(replay, &qxl->u.alpha_blend, flags);
@@ -1017,9 +975,6 @@ static QXLCompatDrawable *red_replay_compat_drawable(SpiceReplay *replay, uint32
red_replay_rect_ptr(replay, "bitmap_area", &qxl->bitmap_area);
replay_fscanf(replay, "type %d\n", &temp); qxl->type = temp;
- if (replay->error) {
- return NULL;
- }
switch (qxl->type) {
case QXL_DRAW_ALPHA_BLEND:
red_replay_alpha_blend_ptr_compat(replay, &qxl->u.alpha_blend, flags);
@@ -1072,9 +1027,6 @@ static QXLCompatDrawable *red_replay_compat_drawable(SpiceReplay *replay, uint32
static QXLPHYSICAL red_replay_drawable(SpiceReplay *replay, uint32_t flags)
{
replay_fscanf(replay, "drawable\n");
- if (replay->error) {
- return 0;
- }
if (flags & QXL_COMMAND_FLAG_COMPAT) {
return QXLPHYSICAL_FROM_PTR(red_replay_compat_drawable(replay, flags));
} else {
@@ -1090,9 +1042,6 @@ static QXLUpdateCmd *red_replay_update_cmd(SpiceReplay *replay)
red_replay_rect_ptr(replay, "area", &qxl->area);
replay_fscanf(replay, "update_id %d\n", &qxl->update_id);
replay_fscanf(replay, "surface_id %d\n", &qxl->surface_id);
- if (replay->error) {
- return NULL;
- }
qxl->surface_id = replay_id_get(replay, qxl->surface_id);
return qxl;
@@ -1118,9 +1067,6 @@ static QXLSurfaceCmd *red_replay_surface_cmd(SpiceReplay *replay)
replay_fscanf(replay, "surface_id %d\n", &qxl->surface_id);
replay_fscanf(replay, "type %d\n", &temp); qxl->type = temp;
replay_fscanf(replay, "flags %d\n", &qxl->flags);
- if (replay->error) {
- return NULL;
- }
switch (qxl->type) {
case QXL_SURFACE_CMD_CREATE:
@@ -1128,9 +1074,6 @@ static QXLSurfaceCmd *red_replay_surface_cmd(SpiceReplay *replay)
replay_fscanf(replay, "u.surface_create.width %d\n", &qxl->u.surface_create.width);
replay_fscanf(replay, "u.surface_create.height %d\n", &qxl->u.surface_create.height);
replay_fscanf(replay, "u.surface_create.stride %d\n", &qxl->u.surface_create.stride);
- if (replay->error) {
- return NULL;
- }
size = qxl->u.surface_create.height * abs(qxl->u.surface_create.stride);
if ((qxl->flags & QXL_SURF_FLAG_KEEP_DATA) != 0) {
read_binary(replay, "data", &read_size, (uint8_t**)&qxl->u.surface_create.data, 0);
@@ -1178,9 +1121,6 @@ static QXLCursor *red_replay_cursor(SpiceReplay *replay)
cursor.header.hot_spot_y = temp;
replay_fscanf(replay, "data_size %d\n", &temp);
- if (replay->error) {
- return NULL;
- }
data_size = red_replay_data_chunks(replay, "cursor", (uint8_t**)&qxl, sizeof(QXLCursor));
if (data_size < 0) {
return NULL;
@@ -1197,9 +1137,6 @@ static QXLCursorCmd *red_replay_cursor_cmd(SpiceReplay *replay)
replay_fscanf(replay, "cursor_cmd\n");
replay_fscanf(replay, "type %d\n", &temp);
- if (replay->error) {
- return NULL;
- }
qxl->type = temp;
switch (qxl->type) {
case QXL_CURSOR_SET:
@@ -1249,9 +1186,6 @@ static void replay_handle_create_primary(QXLWorker *worker, SpiceReplay *replay)
&surface.stride, &surface.format);
replay_fscanf(replay, "%d %d %d %d\n", &surface.position, &surface.mouse_mode,
&surface.flags, &surface.type);
- if (replay->error) {
- return;
- }
read_binary(replay, "data", &size, &mem, 0);
surface.group_id = 0;
surface.mem = QXLPHYSICAL_FROM_PTR(mem);
@@ -1302,12 +1236,19 @@ SPICE_GNUC_VISIBLE QXLCommandExt* spice_replay_next_cmd(SpiceReplay *replay,
int what = -1;
int counter;
+ if (setjmp(replay->jmp_error)) {
+ /* if there were some allocation during the read free all
+ * buffers allocated to avoid leaks */
+ if (replay->allocated) {
+ g_list_free_full(replay->allocated, free);
+ replay->allocated = NULL;
+ }
+ return NULL;
+ }
+
while (what != 0) {
replay_fscanf(replay, "event %d %d %d %"SCNu64"\n", &counter,
&what, &type, ×tamp);
- if (replay->error) {
- goto error;
- }
if (what == 1) {
replay_handle_dev_input(worker, replay, type);
}
@@ -1335,10 +1276,6 @@ SPICE_GNUC_VISIBLE QXLCommandExt* spice_replay_next_cmd(SpiceReplay *replay,
break;
}
- if (replay->error) {
- goto error;
- }
-
QXLReleaseInfo *info;
switch (cmd->cmd.type) {
case QXL_CMD_DRAW:
@@ -1359,15 +1296,6 @@ SPICE_GNUC_VISIBLE QXLCommandExt* spice_replay_next_cmd(SpiceReplay *replay,
replay->counter++;
return cmd;
-
-error:
- /* if there were some allocation during the read free all
- * buffers allocated to avoid leaks */
- if (replay->allocated) {
- g_list_free_full(replay->allocated, free);
- replay->allocated = NULL;
- }
- return NULL;
}
SPICE_GNUC_VISIBLE void spice_replay_free_cmd(SpiceReplay *replay, QXLCommandExt *cmd)
--
2.7.4
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel