[PATCH wayland] Clean up and refactor wl_closure and associated functions
Kristian Høgsberg
hoegsberg at gmail.com
Tue Feb 26 10:37:47 PST 2013
On Sat, Feb 23, 2013 at 12:57:09PM -0600, Jason Ekstrand wrote:
> The primary purpose of this patch is to clean up wl_closure and separate
> closure storage, libffi, and the wire format. To that end, a number of changes
> have been made:
>
> - The maximum number of closure arguments has been changed from a magic number
> to a #define WL_CLOSURE_MAX_ARGS
>
> - A wl_argument union has been added for storing a generalized closure
> argument and wl_closure has been converted to use wl_argument instead of the
> combination of libffi, the wire format, and a dummy extra buffer. As of
> now, the "extra" field in wl_closure should be treated as bulk storage and
> never direclty referenced outside of wl_connection_demarshal.
>
> - Everything having to do with libffi has been moved into wl_closure_invoke
> and the convert_arguments_to_ffi helper function.
>
> - Everything having to do with the wire format has been restricted to
> wl_connection_demarshal and the new static serialize_closure function. The
> wl_closure_send and wl_closure_queue functions are now light wrappers around
> serialize_closure.
>
> Signed-off-by: Jason Ekstrand <jason at jlekstrand.net>
> ---
> I've re-worked my wl_closure cleanups so they should now be orthogonal to the
> custom dispatchers stuff. This patch should be good-to-go now and future
> dispatchers patches will have this as a prerequisite. Also, this should apply
> against 1.0. As a side-effect, this may fix that memory-alignment bug on 64bit
> MIPS.
Yeah, I agree, I always liked the cleanup part of the series. I've
applied this one (with just a couple stylistic fixes: no space between
'!' and its argument). I did notice one change during the review
that's a little subtle: before, vmarshal would create a self-contained
object, but now it relies on string pointers and array content to
remain valid during its lifetime. As it is, we always create a
closure and send it out within wl_proxy_marshal() or
wl_resource_post/qeue_event(), so it doesn't break anything and saves
a copy. But if we ever want to hold on to a closure after returning
from those functions, we need to copy the contents.
I think it might be 1.0 material, however I want to wait a little
while and let this sit on master before we pull it into 1.0. I do
expect that we can include it in a 1.0.6 release though.
Kristian
> src/connection.c | 661 +++++++++++++++++++++++++++-----------------------
> src/wayland-client.c | 28 +--
> src/wayland-private.h | 31 ++-
> src/wayland-server.c | 19 --
> 4 files changed, 400 insertions(+), 339 deletions(-)
>
> diff --git a/src/connection.c b/src/connection.c
> index 141875e..93f0105 100644
> --- a/src/connection.c
> +++ b/src/connection.c
> @@ -1,5 +1,6 @@
> /*
> * Copyright © 2008 Kristian Høgsberg
> + * Copyright © 2013 Jason Ekstrand
> *
> * Permission to use, copy, modify, distribute, and sell this software and its
> * documentation for any purpose is hereby granted without fee, provided that
> @@ -35,6 +36,7 @@
> #include <sys/types.h>
> #include <sys/socket.h>
> #include <time.h>
> +#include <ffi.h>
>
> #include "wayland-util.h"
> #include "wayland-private.h"
> @@ -59,14 +61,6 @@ struct wl_connection {
> int want_flush;
> };
>
> -union wl_value {
> - uint32_t uint32;
> - char *string;
> - struct wl_object *object;
> - uint32_t new_id;
> - struct wl_array *array;
> -};
> -
> static void
> wl_buffer_put(struct wl_buffer *b, const void *data, size_t count)
> {
> @@ -379,30 +373,16 @@ wl_connection_queue(struct wl_connection *connection,
> }
>
> static int
> -wl_message_size_extra(const struct wl_message *message)
> +wl_message_count_arrays(const struct wl_message *message)
> {
> - int i, extra;
> -
> - for (i = 0, extra = 0; message->signature[i]; i++) {
> + int i, arrays;
>
> - switch (message->signature[i]) {
> - case 's':
> - case 'o':
> - case 'n':
> - extra += sizeof (void *);
> - break;
> - case 'a':
> - extra += sizeof (void *) + sizeof (struct wl_array);
> - break;
> - case 'h':
> - extra += sizeof (int);
> - break;
> - default:
> - break;
> - }
> + for (i = 0, arrays = 0; message->signature[i]; i++) {
> + if (message->signature[i] == 'a')
> + ++arrays;
> }
>
> - return extra;
> + return arrays;
> }
>
> static int
> @@ -444,163 +424,111 @@ arg_count_for_signature(const char *signature)
> return count;
> }
>
> +void
> +wl_argument_from_va_list(const char *signature, union wl_argument *args,
> + int count, va_list ap)
> +{
> + int i;
> + const char *sig_iter;
> + struct argument_details arg;
> +
> + sig_iter = signature;
> + for (i = 0; i < count; i++) {
> + sig_iter = get_next_argument(sig_iter, &arg);
> +
> + switch(arg.type) {
> + case 'i':
> + args[i].i = va_arg(ap, int32_t);
> + break;
> + case 'u':
> + args[i].u = va_arg(ap, uint32_t);
> + break;
> + case 'f':
> + args[i].f = va_arg(ap, wl_fixed_t);
> + break;
> + case 's':
> + args[i].s = va_arg(ap, const char *);
> + break;
> + case 'o':
> + args[i].o = va_arg(ap, struct wl_object *);
> + break;
> + case 'n':
> + args[i].o = va_arg(ap, struct wl_object *);
> + break;
> + case 'a':
> + args[i].a = va_arg(ap, struct wl_array *);
> + break;
> + case 'h':
> + args[i].h = va_arg(ap, int32_t);
> + break;
> + case '\0':
> + return;
> + }
> + }
> +}
> +
> struct wl_closure *
> -wl_closure_vmarshal(struct wl_object *sender,
> - uint32_t opcode, va_list ap,
> - const struct wl_message *message)
> +wl_closure_marshal(struct wl_object *sender, uint32_t opcode,
> + union wl_argument *args,
> + const struct wl_message *message)
> {
> struct wl_closure *closure;
> - struct wl_object **objectp, *object;
> - uint32_t length, aligned, *p, *start, size, *end;
> - int dup_fd;
> - struct wl_array **arrayp, *array;
> - const char **sp, *s;
> - const char *signature = message->signature;
> + struct wl_object *object;
> + int i, count, fd, dup_fd;
> + const char *signature;
> struct argument_details arg;
> - char *extra;
> - int i, count, fd, extra_size, *fd_ptr;
>
> - /* FIXME: Match old fixed allocation for now */
> - closure = malloc(sizeof *closure + 1024);
> - if (closure == NULL)
> + count = arg_count_for_signature(message->signature);
> + if (count > WL_CLOSURE_MAX_ARGS) {
> + printf("too many args (%d)\n", count);
> + errno = EINVAL;
> return NULL;
> + }
>
> - extra_size = wl_message_size_extra(message);
> - count = arg_count_for_signature(signature) + 2;
> - extra = (char *) closure->buffer;
> - start = &closure->buffer[DIV_ROUNDUP(extra_size, sizeof *p)];
> - end = &closure->buffer[256];
> - p = &start[2];
> + closure = malloc(sizeof *closure);
> + if (closure == NULL) {
> + errno = ENOMEM;
> + return NULL;
> + }
>
> - closure->types[0] = &ffi_type_pointer;
> - closure->types[1] = &ffi_type_pointer;
> + memcpy(closure->args, args, count * sizeof *args);
>
> - for (i = 2; i < count; i++) {
> + signature = message->signature;
> + for (i = 0; i < count; i++) {
> signature = get_next_argument(signature, &arg);
>
> switch (arg.type) {
> case 'f':
> - closure->types[i] = &ffi_type_sint32;
> - closure->args[i] = p;
> - if (end - p < 1)
> - goto err;
> - *p++ = va_arg(ap, wl_fixed_t);
> - break;
> case 'u':
> - closure->types[i] = &ffi_type_uint32;
> - closure->args[i] = p;
> - if (end - p < 1)
> - goto err;
> - *p++ = va_arg(ap, uint32_t);
> - break;
> case 'i':
> - closure->types[i] = &ffi_type_sint32;
> - closure->args[i] = p;
> - if (end - p < 1)
> - goto err;
> - *p++ = va_arg(ap, int32_t);
> break;
> case 's':
> - closure->types[i] = &ffi_type_pointer;
> - closure->args[i] = extra;
> - sp = (const char **) extra;
> - extra += sizeof *sp;
> -
> - s = va_arg(ap, const char *);
> -
> - if (!arg.nullable && s == NULL)
> + if (! arg.nullable && args[i].s == NULL)
> goto err_null;
> -
> - length = s ? strlen(s) + 1: 0;
> - aligned = (length + 3) & ~3;
> - if (p + aligned / sizeof *p + 1 > end)
> - goto err;
> - *p++ = length;
> -
> - if (length > 0) {
> - memcpy(p, s, length);
> - *sp = (const char *) p;
> - } else
> - *sp = NULL;
> -
> - memset((char *) p + length, 0, aligned - length);
> - p += aligned / sizeof *p;
> break;
> case 'o':
> - closure->types[i] = &ffi_type_pointer;
> - closure->args[i] = extra;
> - objectp = (struct wl_object **) extra;
> - extra += sizeof *objectp;
> -
> - object = va_arg(ap, struct wl_object *);
> -
> - if (!arg.nullable && object == NULL)
> + if (! arg.nullable && args[i].o == NULL)
> goto err_null;
> -
> - *objectp = object;
> - if (end - p < 1)
> - goto err;
> - *p++ = object ? object->id : 0;
> break;
> -
> case 'n':
> - closure->types[i] = &ffi_type_uint32;
> - closure->args[i] = p;
> - object = va_arg(ap, struct wl_object *);
> - if (end - p < 1)
> - goto err;
> -
> + object = args[i].o;
> if (!arg.nullable && object == NULL)
> goto err_null;
>
> - *p++ = object ? object->id : 0;
> + closure->args[i].n = object ? object->id : 0;
> break;
> -
> case 'a':
> - closure->types[i] = &ffi_type_pointer;
> - closure->args[i] = extra;
> - arrayp = (struct wl_array **) extra;
> - extra += sizeof *arrayp;
> -
> - *arrayp = (struct wl_array *) extra;
> - extra += sizeof **arrayp;
> -
> - array = va_arg(ap, struct wl_array *);
> -
> - if (!arg.nullable && array == NULL)
> + if (! arg.nullable && args[i].a == NULL)
> goto err_null;
> -
> - if (array == NULL || array->size == 0) {
> - if (end - p < 1)
> - goto err;
> - *p++ = 0;
> - break;
> - }
> - if (p + DIV_ROUNDUP(array->size, sizeof *p) + 1 > end)
> - goto err;
> - *p++ = array->size;
> - memcpy(p, array->data, array->size);
> -
> - (*arrayp)->size = array->size;
> - (*arrayp)->alloc = array->alloc;
> - (*arrayp)->data = p;
> -
> - p += DIV_ROUNDUP(array->size, sizeof *p);
> break;
> -
> case 'h':
> - closure->types[i] = &ffi_type_sint;
> - closure->args[i] = extra;
> - fd_ptr = (int *) extra;
> - extra += sizeof *fd_ptr;
> -
> - fd = va_arg(ap, int);
> + fd = args[i].h;
> dup_fd = wl_os_dupfd_cloexec(fd, 0);
> if (dup_fd < 0) {
> fprintf(stderr, "dup failed: %m");
> abort();
> }
> - *fd_ptr = dup_fd;
> + closure->args[i].h = dup_fd;
> break;
> default:
> fprintf(stderr, "unhandled format code: '%c'\n",
> @@ -610,78 +538,78 @@ wl_closure_vmarshal(struct wl_object *sender,
> }
> }
>
> - size = (p - start) * sizeof *p;
> - start[0] = sender->id;
> - start[1] = opcode | (size << 16);
> -
> - closure->start = start;
> + closure->sender_id = sender->id;
> + closure->opcode = opcode;
> closure->message = message;
> closure->count = count;
>
> - ffi_prep_cif(&closure->cif, FFI_DEFAULT_ABI,
> - closure->count, &ffi_type_void, closure->types);
> -
> return closure;
>
> -err:
> - printf("request too big to marshal, maximum size is %zu\n",
> - sizeof closure->buffer);
> - errno = ENOMEM;
> - free(closure);
> -
> - return NULL;
> -
> err_null:
> - free(closure);
> - wl_log("error marshalling arguments for %s:%i.%s (signature %s): "
> - "null value passed for arg %i\n",
> - sender->interface->name, sender->id, message->name,
> + wl_closure_destroy(closure);
> + wl_log("error marshalling arguments for %s (signature %s): "
> + "null value passed for arg %i\n", message->name,
> message->signature, i);
> errno = EINVAL;
> return NULL;
> }
>
> struct wl_closure *
> +wl_closure_vmarshal(struct wl_object *sender, uint32_t opcode, va_list ap,
> + const struct wl_message *message)
> +{
> + union wl_argument args[WL_CLOSURE_MAX_ARGS];
> +
> + wl_argument_from_va_list(message->signature, args,
> + WL_CLOSURE_MAX_ARGS, ap);
> +
> + return wl_closure_marshal(sender, opcode, args, message);
> +}
> +
> +struct wl_closure *
> wl_connection_demarshal(struct wl_connection *connection,
> uint32_t size,
> struct wl_map *objects,
> const struct wl_message *message)
> {
> - uint32_t *p, *next, *end, length, **id;
> - int *fd;
> - char *extra, **s;
> - unsigned int i, count, extra_space;
> - const char *signature = message->signature;
> + uint32_t *p, *next, *end, length, id;
> + int fd;
> + char *s;
> + unsigned int i, count, num_arrays;
> + const char *signature;
> struct argument_details arg;
> - struct wl_array **array;
> struct wl_closure *closure;
> + struct wl_array *array, *array_extra;
>
> - count = arg_count_for_signature(signature) + 2;
> - if (count > ARRAY_LENGTH(closure->types)) {
> + count = arg_count_for_signature(message->signature);
> + if (count > WL_CLOSURE_MAX_ARGS) {
> printf("too many args (%d)\n", count);
> errno = EINVAL;
> wl_connection_consume(connection, size);
> return NULL;
> }
>
> - extra_space = wl_message_size_extra(message);
> - closure = malloc(sizeof *closure + 8 + size + extra_space);
> - if (closure == NULL)
> + num_arrays = wl_message_count_arrays(message);
> + closure = malloc(sizeof *closure + size + num_arrays * sizeof *array);
> + if (closure == NULL) {
> + errno = ENOMEM;
> + wl_connection_consume(connection, size);
> return NULL;
> + }
>
> - closure->message = message;
> - closure->types[0] = &ffi_type_pointer;
> - closure->types[1] = &ffi_type_pointer;
> - closure->start = closure->buffer;
> -
> - wl_connection_copy(connection, closure->buffer, size);
> - p = &closure->buffer[2];
> - end = (uint32_t *) ((char *) p + size);
> - extra = (char *) end;
> - for (i = 2; i < count; i++) {
> + array_extra = closure->extra;
> + p = (uint32_t *)(closure->extra + num_arrays);
> + end = p + size / sizeof *p;
> +
> + wl_connection_copy(connection, p, size);
> + closure->sender_id = *p++;
> + closure->opcode = *p++ & 0x0000ffff;
> +
> + signature = message->signature;
> + for (i = 0; i < count; i++) {
> signature = get_next_argument(signature, &arg);
>
> - if (p + 1 > end) {
> + if (arg.type != 'h' && p + 1 > end) {
> printf("message too short, "
> "object (%d), message %s(%s)\n",
> *p, message->name, message->signature);
> @@ -691,74 +619,62 @@ wl_connection_demarshal(struct wl_connection *connection,
>
> switch (arg.type) {
> case 'u':
> - closure->types[i] = &ffi_type_uint32;
> - closure->args[i] = p++;
> + closure->args[i].u = *p++;
> break;
> case 'i':
> - closure->types[i] = &ffi_type_sint32;
> - closure->args[i] = p++;
> + closure->args[i].i = *p++;
> break;
> case 'f':
> - closure->types[i] = &ffi_type_sint32;
> - closure->args[i] = p++;
> + closure->args[i].f = *p++;
> break;
> case 's':
> - closure->types[i] = &ffi_type_pointer;
> length = *p++;
>
> + if (length == 0) {
> + closure->args[i].s = NULL;
> + break;
> + }
> +
> next = p + DIV_ROUNDUP(length, sizeof *p);
> if (next > end) {
> printf("message too short, "
> "object (%d), message %s(%s)\n",
> - *p, message->name, message->signature);
> + closure->sender_id, message->name,
> + message->signature);
> errno = EINVAL;
> goto err;
> }
>
> - s = (char **) extra;
> - extra += sizeof *s;
> - closure->args[i] = s;
> -
> - if (length == 0) {
> - *s = NULL;
> - } else {
> - *s = (char *) p;
> - }
> + s = (char *) p;
>
> - if (length > 0 && (*s)[length - 1] != '\0') {
> + if (length > 0 && s[length - 1] != '\0') {
> printf("string not nul-terminated, "
> "message %s(%s)\n",
> message->name, message->signature);
> errno = EINVAL;
> goto err;
> }
> +
> + closure->args[i].s = s;
> p = next;
> break;
> case 'o':
> - closure->types[i] = &ffi_type_pointer;
> - id = (uint32_t **) extra;
> - extra += sizeof *id;
> - closure->args[i] = id;
> - *id = p;
> + id = *p++;
> + closure->args[i].n = id;
>
> - if (**id == 0 && !arg.nullable) {
> + if (id == 0 && !arg.nullable) {
> printf("NULL object received on non-nullable "
> "type, message %s(%s)\n", message->name,
> message->signature);
> errno = EINVAL;
> goto err;
> }
> -
> - p++;
> break;
> case 'n':
> - closure->types[i] = &ffi_type_pointer;
> - id = (uint32_t **) extra;
> - extra += sizeof *id;
> - closure->args[i] = id;
> - *id = p;
> + id = *p++;
> + closure->args[i].n = id;
>
> - if (**id == 0 && !arg.nullable) {
> + if (id == 0 && !arg.nullable) {
> printf("NULL new ID received on non-nullable "
> "type, message %s(%s)\n", message->name,
> message->signature);
> @@ -766,50 +682,39 @@ wl_connection_demarshal(struct wl_connection *connection,
> goto err;
> }
>
> - if (wl_map_reserve_new(objects, *p) < 0) {
> + if (wl_map_reserve_new(objects, id) < 0) {
> printf("not a valid new object id (%d), "
> "message %s(%s)\n",
> - *p, message->name, message->signature);
> + id, message->name, message->signature);
> errno = EINVAL;
> goto err;
> }
>
> - p++;
> break;
> case 'a':
> - closure->types[i] = &ffi_type_pointer;
> length = *p++;
>
> next = p + DIV_ROUNDUP(length, sizeof *p);
> if (next > end) {
> printf("message too short, "
> "object (%d), message %s(%s)\n",
> - *p, message->name, message->signature);
> + closure->sender_id, message->name,
> + message->signature);
> errno = EINVAL;
> goto err;
> }
>
> - array = (struct wl_array **) extra;
> - extra += sizeof *array;
> - closure->args[i] = array;
> + array_extra->size = length;
> + array_extra->alloc = 0;
> + array_extra->data = p;
>
> - *array = (struct wl_array *) extra;
> - extra += sizeof **array;
> -
> - (*array)->size = length;
> - (*array)->alloc = 0;
> - (*array)->data = p;
> + closure->args[i].a = array_extra++;
> p = next;
> break;
> case 'h':
> - closure->types[i] = &ffi_type_sint;
> -
> - fd = (int *) extra;
> - extra += sizeof *fd;
> - closure->args[i] = fd;
> -
> - wl_buffer_copy(&connection->fds_in, fd, sizeof *fd);
> - connection->fds_in.tail += sizeof *fd;
> + wl_buffer_copy(&connection->fds_in, &fd, sizeof fd);
> + connection->fds_in.tail += sizeof fd;
> + closure->args[i].h = fd;
> break;
> default:
> printf("unknown type\n");
> @@ -818,17 +723,14 @@ wl_connection_demarshal(struct wl_connection *connection,
> }
> }
>
> - closure->count = i;
> -
> - ffi_prep_cif(&closure->cif, FFI_DEFAULT_ABI,
> - closure->count, &ffi_type_void, closure->types);
> + closure->count = count;
> + closure->message = message;
>
> wl_connection_consume(connection, size);
>
> return closure;
>
> err:
> - closure->count = i;
> wl_closure_destroy(closure);
> wl_connection_consume(connection, size);
>
> @@ -851,7 +753,7 @@ interface_equal(const struct wl_interface *a, const struct wl_interface *b)
> int
> wl_closure_lookup_objects(struct wl_closure *closure, struct wl_map *objects)
> {
> - struct wl_object **object;
> + struct wl_object *object;
> const struct wl_message *message;
> const char *signature;
> struct argument_details arg;
> @@ -860,52 +762,121 @@ wl_closure_lookup_objects(struct wl_closure *closure, struct wl_map *objects)
>
> message = closure->message;
> signature = message->signature;
> - count = arg_count_for_signature(signature) + 2;
> - for (i = 2; i < count; i++) {
> + count = arg_count_for_signature(signature);
> + for (i = 0; i < count; i++) {
> signature = get_next_argument(signature, &arg);
> switch (arg.type) {
> case 'o':
> - id = **(uint32_t **) closure->args[i];
> - object = closure->args[i];
> - *object = wl_map_lookup(objects, id);
> - if (*object == WL_ZOMBIE_OBJECT) {
> + id = closure->args[i].n;
> + closure->args[i].o = NULL;
> +
> + object = wl_map_lookup(objects, id);
> + if (object == WL_ZOMBIE_OBJECT) {
> /* references object we've already
> * destroyed client side */
> - *object = NULL;
> - } else if (*object == NULL && id != 0) {
> + object = NULL;
> + } else if (object == NULL && id != 0) {
> printf("unknown object (%u), message %s(%s)\n",
> id, message->name, message->signature);
> - *object = NULL;
> + object = NULL;
> errno = EINVAL;
> return -1;
> }
>
> - if (*object != NULL && message->types[i-2] != NULL &&
> - !interface_equal((*object)->interface,
> - message->types[i-2])) {
> + if (object != NULL && message->types[i] != NULL &&
> + !interface_equal((object)->interface,
> + message->types[i])) {
> printf("invalid object (%u), type (%s), "
> "message %s(%s)\n",
> - id, (*object)->interface->name,
> + id, (object)->interface->name,
> message->name, message->signature);
> errno = EINVAL;
> return -1;
> }
> + closure->args[i].o = object;
> }
> }
>
> return 0;
> }
>
> +static void
> +convert_arguments_to_ffi(const char *signature, union wl_argument *args,
> + int count, ffi_type **ffi_types, void** ffi_args)
> +{
> + int i;
> + const char *sig_iter;
> + struct argument_details arg;
> +
> + sig_iter = signature;
> + for (i = 0; i < count; i++) {
> + sig_iter = get_next_argument(sig_iter, &arg);
> +
> + switch(arg.type) {
> + case 'i':
> + ffi_types[i] = &ffi_type_sint32;
> + ffi_args[i] = &args[i].i;
> + break;
> + case 'u':
> + ffi_types[i] = &ffi_type_uint32;
> + ffi_args[i] = &args[i].u;
> + break;
> + case 'f':
> + ffi_types[i] = &ffi_type_sint32;
> + ffi_args[i] = &args[i].f;
> + break;
> + case 's':
> + ffi_types[i] = &ffi_type_pointer;
> + ffi_args[i] = &args[i].s;
> + break;
> + case 'o':
> + ffi_types[i] = &ffi_type_pointer;
> + ffi_args[i] = &args[i].o;
> + break;
> + case 'n':
> + ffi_types[i] = &ffi_type_uint32;
> + ffi_args[i] = &args[i].n;
> + break;
> + case 'a':
> + ffi_types[i] = &ffi_type_pointer;
> + ffi_args[i] = &args[i].a;
> + break;
> + case 'h':
> + ffi_types[i] = &ffi_type_sint32;
> + ffi_args[i] = &args[i].h;
> + break;
> + default:
> + printf("unknown type\n");
> + assert(0);
> + break;
> + }
> + }
> +}
> +
> +
> void
> wl_closure_invoke(struct wl_closure *closure,
> struct wl_object *target, void (*func)(void), void *data)
> {
> - int result;
> + int count;
> + ffi_cif cif;
> + ffi_type *ffi_types[WL_CLOSURE_MAX_ARGS + 2];
> + void * ffi_args[WL_CLOSURE_MAX_ARGS + 2];
> +
> + count = arg_count_for_signature(closure->message->signature);
>
> - closure->args[0] = &data;
> - closure->args[1] = ⌖
> + ffi_types[0] = &ffi_type_pointer;
> + ffi_args[0] = &data;
> + ffi_types[1] = &ffi_type_pointer;
> + ffi_args[1] = ⌖
>
> - ffi_call(&closure->cif, func, &result, closure->args);
> + convert_arguments_to_ffi(closure->message->signature, closure->args,
> + count, ffi_types + 2, ffi_args + 2);
> +
> + ffi_prep_cif(&cif, FFI_DEFAULT_ABI,
> + count + 2, &ffi_type_void, ffi_types);
> +
> + ffi_call(&cif, func, NULL, ffi_args);
> }
>
> static int
> @@ -916,16 +887,16 @@ copy_fds_to_connection(struct wl_closure *closure,
> uint32_t i, count;
> struct argument_details arg;
> const char *signature = message->signature;
> - int *fd;
> + int fd;
>
> - count = arg_count_for_signature(signature) + 2;
> - for (i = 2; i < count; i++) {
> + count = arg_count_for_signature(signature);
> + for (i = 0; i < count; i++) {
> signature = get_next_argument(signature, &arg);
> if (arg.type != 'h')
> continue;
>
> - fd = closure->args[i];
> - if (wl_connection_put_fd(connection, *fd)) {
> + fd = closure->args[i].h;
> + if (wl_connection_put_fd(connection, fd)) {
> fprintf(stderr, "request could not be marshaled: "
> "can't send file descriptor");
> return -1;
> @@ -935,37 +906,131 @@ copy_fds_to_connection(struct wl_closure *closure,
> return 0;
> }
>
> +static int
> +serialize_closure(struct wl_closure *closure, uint32_t *buffer,
> + size_t buffer_count)
> +{
> + const struct wl_message *message = closure->message;
> + unsigned int i, count, size;
> + uint32_t *p, *end;
> + struct argument_details arg;
> + const char *signature;
> +
> + if (buffer_count < 2)
> + goto overflow;
> +
> + p = buffer + 2;
> + end = buffer + buffer_count;
> +
> + signature = message->signature;
> + count = arg_count_for_signature(signature);
> + for (i = 0; i < count; i++) {
> + signature = get_next_argument(signature, &arg);
> +
> + if (arg.type == 'h')
> + continue;
> +
> + if (p + 1 > end)
> + goto overflow;
> +
> + switch (arg.type) {
> + case 'u':
> + *p++ = closure->args[i].u;
> + break;
> + case 'i':
> + *p++ = closure->args[i].i;
> + break;
> + case 'f':
> + *p++ = closure->args[i].f;
> + break;
> + case 'o':
> + *p++ = closure->args[i].o ? closure->args[i].o->id : 0;
> + break;
> + case 'n':
> + *p++ = closure->args[i].n;
> + break;
> + case 's':
> + if (closure->args[i].s == NULL) {
> + *p++ = 0;
> + break;
> + }
> +
> + size = strlen(closure->args[i].s) + 1;
> + *p++ = size;
> +
> + if (p + DIV_ROUNDUP(size, sizeof *p) > end)
> + goto overflow;
> +
> + memcpy(p, closure->args[i].s, size);
> + p += DIV_ROUNDUP(size, sizeof *p);
> + break;
> + case 'a':
> + if (closure->args[i].a == NULL) {
> + *p++ = 0;
> + break;
> + }
> +
> + size = closure->args[i].a->size;
> + *p++ = size;
> +
> + if (p + DIV_ROUNDUP(size, sizeof *p) > end)
> + goto overflow;
> +
> + memcpy(p, closure->args[i].a->data, size);
> + p += DIV_ROUNDUP(size, sizeof *p);
> + break;
> + default:
> + break;
> + }
> + }
> +
> + size = (p - buffer) * sizeof *p;
> +
> + buffer[0] = closure->sender_id;
> + buffer[1] = size << 16 | (closure->opcode & 0x0000ffff);
> +
> + return size;
> +
> +overflow:
> + errno = ERANGE;
> + return -1;
> +}
> +
> int
> wl_closure_send(struct wl_closure *closure, struct wl_connection *connection)
> {
> - uint32_t size;
> + uint32_t buffer[256];
> + int size;
>
> if (copy_fds_to_connection(closure, connection))
> return -1;
>
> - size = closure->start[1] >> 16;
> + size = serialize_closure(closure, buffer, 256);
> + if (size < 0)
> + return -1;
>
> - return wl_connection_write(connection, closure->start, size);
> + return wl_connection_write(connection, buffer, size);
> }
>
> int
> wl_closure_queue(struct wl_closure *closure, struct wl_connection *connection)
> {
> - uint32_t size;
> + uint32_t buffer[256];
> + int size;
>
> if (copy_fds_to_connection(closure, connection))
> return -1;
>
> - size = closure->start[1] >> 16;
> + size = serialize_closure(closure, buffer, 256);
> + if (size < 0)
> + return -1;
>
> - return wl_connection_queue(connection, closure->start, size);
> + return wl_connection_queue(connection, buffer, size);
> }
>
> void
> wl_closure_print(struct wl_closure *closure, struct wl_object *target, int send)
> {
> - union wl_value *value;
> - int32_t si;
> int i;
> struct argument_details arg;
> const char *signature = closure->message->signature;
> @@ -981,44 +1046,40 @@ wl_closure_print(struct wl_closure *closure, struct wl_object *target, int send)
> target->interface->name, target->id,
> closure->message->name);
>
> - for (i = 2; i < closure->count; i++) {
> + for (i = 0; i < closure->count; i++) {
> signature = get_next_argument(signature, &arg);
> - if (i > 2)
> + if (i > 0)
> fprintf(stderr, ", ");
>
> - value = closure->args[i];
> switch (arg.type) {
> case 'u':
> - fprintf(stderr, "%u", value->uint32);
> + fprintf(stderr, "%u", closure->args[i].u);
> break;
> case 'i':
> - si = (int32_t) value->uint32;
> - fprintf(stderr, "%d", si);
> + fprintf(stderr, "%d", closure->args[i].i);
> break;
> case 'f':
> - si = (int32_t) value->uint32;
> - fprintf(stderr, "%f", wl_fixed_to_double(si));
> + fprintf(stderr, "%f",
> + wl_fixed_to_double(closure->args[i].f));
> break;
> case 's':
> - fprintf(stderr, "\"%s\"", value->string);
> + fprintf(stderr, "\"%s\"", closure->args[i].s);
> break;
> case 'o':
> - if (value->object)
> + if (closure->args[i].o)
> fprintf(stderr, "%s@%u",
> - value->object->interface->name,
> - value->object->id);
> + closure->args[i].o->interface->name,
> + closure->args[i].o->id);
> else
> fprintf(stderr, "nil");
> break;
> case 'n':
> fprintf(stderr, "new id %s@",
> - (closure->message->types[i - 2]) ?
> - closure->message->types[i - 2]->name :
> + (closure->message->types[i]) ?
> + closure->message->types[i]->name :
> "[unknown]");
> - if (send && value->new_id != 0)
> - fprintf(stderr, "%u", value->new_id);
> - else if (!send && value->object != NULL)
> - fprintf(stderr, "%u", value->object->id);
> + if (closure->args[i].n != 0)
> + fprintf(stderr, "%u", closure->args[i].n);
> else
> fprintf(stderr, "nil");
> break;
> @@ -1026,7 +1087,7 @@ wl_closure_print(struct wl_closure *closure, struct wl_object *target, int send)
> fprintf(stderr, "array");
> break;
> case 'h':
> - fprintf(stderr, "fd %d", value->uint32);
> + fprintf(stderr, "fd %d", closure->args[i].h);
> break;
> }
> }
> diff --git a/src/wayland-client.c b/src/wayland-client.c
> index 785f4ee..2f551f9 100644
> --- a/src/wayland-client.c
> +++ b/src/wayland-client.c
> @@ -669,21 +669,21 @@ create_proxies(struct wl_proxy *sender, struct wl_closure *closure)
> int count;
>
> signature = closure->message->signature;
> - count = arg_count_for_signature(signature) + 2;
> - for (i = 2; i < count; i++) {
> + count = arg_count_for_signature(signature);
> + for (i = 0; i < count; i++) {
> signature = get_next_argument(signature, &arg);
> switch (arg.type) {
> case 'n':
> - id = **(uint32_t **) closure->args[i];
> + id = closure->args[i].n;
> if (id == 0) {
> - *(void **) closure->args[i] = NULL;
> + closure->args[i].o = NULL;
> break;
> }
> proxy = wl_proxy_create_for_id(sender, id,
> - closure->message->types[i - 2]);
> + closure->message->types[i]);
> if (proxy == NULL)
> return -1;
> - *(void **) closure->args[i] = proxy;
> + closure->args[i].o = (struct wl_object *)proxy;
> break;
> default:
> break;
> @@ -702,13 +702,13 @@ increase_closure_args_refcount(struct wl_closure *closure)
> struct wl_proxy *proxy;
>
> signature = closure->message->signature;
> - count = arg_count_for_signature(signature) + 2;
> - for (i = 2; i < count; i++) {
> + count = arg_count_for_signature(signature);
> + for (i = 0; i < count; i++) {
> signature = get_next_argument(signature, &arg);
> switch (arg.type) {
> case 'n':
> case 'o':
> - proxy = *(struct wl_proxy **) closure->args[i];
> + proxy = (struct wl_proxy *) closure->args[i].o;
> if (proxy)
> proxy->refcount++;
> break;
> @@ -779,16 +779,16 @@ decrease_closure_args_refcount(struct wl_closure *closure)
> struct wl_proxy *proxy;
>
> signature = closure->message->signature;
> - count = arg_count_for_signature(signature) + 2;
> - for (i = 2; i < count; i++) {
> + count = arg_count_for_signature(signature);
> + for (i = 0; i < count; i++) {
> signature = get_next_argument(signature, &arg);
> switch (arg.type) {
> case 'n':
> case 'o':
> - proxy = *(struct wl_proxy **) closure->args[i];
> + proxy = (struct wl_proxy *) closure->args[i].o;
> if (proxy) {
> if (proxy->flags & WL_PROXY_FLAG_DESTROYED)
> - *(void **) closure->args[i] = NULL;
> + closure->args[i].o = NULL;
>
> proxy->refcount--;
> if (!proxy->refcount)
> @@ -812,7 +812,7 @@ dispatch_event(struct wl_display *display, struct wl_event_queue *queue)
> closure = container_of(queue->event_list.next,
> struct wl_closure, link);
> wl_list_remove(&closure->link);
> - opcode = closure->buffer[1] & 0xffff;
> + opcode = closure->opcode;
>
> /* Verify that the receiving object is still valid by checking if has
> * been destroyed by the application. */
> diff --git a/src/wayland-private.h b/src/wayland-private.h
> index 4ec9896..f0c9010 100644
> --- a/src/wayland-private.h
> +++ b/src/wayland-private.h
> @@ -1,6 +1,7 @@
> /*
> * Copyright © 2008-2011 Kristian Høgsberg
> * Copyright © 2011 Intel Corporation
> + * Copyright © 2013 Jason Ekstrand
> *
> * Permission to use, copy, modify, distribute, and sell this software and its
> * documentation for any purpose is hereby granted without fee, provided that
> @@ -25,7 +26,6 @@
> #define WAYLAND_PRIVATE_H
>
> #include <stdarg.h>
> -#include <ffi.h>
> #include "wayland-util.h"
>
> #define ARRAY_LENGTH(a) (sizeof (a) / sizeof (a)[0])
> @@ -39,6 +39,7 @@
> #define WL_MAP_SERVER_SIDE 0
> #define WL_MAP_CLIENT_SIDE 1
> #define WL_SERVER_ID_START 0xff000000
> +#define WL_CLOSURE_MAX_ARGS 20
>
> struct wl_map {
> struct wl_array client_entries;
> @@ -73,16 +74,26 @@ int wl_connection_write(struct wl_connection *connection, const void *data, size
> int wl_connection_queue(struct wl_connection *connection,
> const void *data, size_t count);
>
> +union wl_argument {
> + int32_t i;
> + uint32_t u;
> + wl_fixed_t f;
> + const char *s;
> + struct wl_object *o;
> + uint32_t n;
> + struct wl_array *a;
> + int32_t h;
> +};
> +
> struct wl_closure {
> int count;
> const struct wl_message *message;
> - ffi_type *types[20];
> - ffi_cif cif;
> - void *args[20];
> - uint32_t *start;
> + uint32_t opcode;
> + uint32_t sender_id;
> + union wl_argument args[WL_CLOSURE_MAX_ARGS];
> struct wl_list link;
> struct wl_proxy *proxy;
> - uint32_t buffer[0];
> + struct wl_array extra[0];
> };
>
> struct argument_details {
> @@ -96,6 +107,14 @@ get_next_argument(const char *signature, struct argument_details *details);
> int
> arg_count_for_signature(const char *signature);
>
> +void
> +wl_argument_from_va_list(const char *signature, union wl_argument *args,
> + int count, va_list ap);
> +
> +struct wl_closure *
> +wl_closure_marshal(struct wl_object *sender,
> + uint32_t opcode, union wl_argument *args,
> + const struct wl_message *message);
> struct wl_closure *
> wl_closure_vmarshal(struct wl_object *sender,
> uint32_t opcode, va_list ap,
> diff --git a/src/wayland-server.c b/src/wayland-server.c
> index dae7177..2f3ddc9 100644
> --- a/src/wayland-server.c
> +++ b/src/wayland-server.c
> @@ -191,23 +191,6 @@ wl_resource_post_error(struct wl_resource *resource,
> WL_DISPLAY_ERROR, resource, code, buffer);
> }
>
> -static void
> -deref_new_objects(struct wl_closure *closure)
> -{
> - const char *signature;
> - int i;
> -
> - signature = closure->message->signature;
> - for (i = 0; signature[i]; i++) {
> - switch (signature[i]) {
> - case 'n':
> - closure->args[i + 2] = *(uint32_t **) closure->args[i + 2];
> - closure->types[i] = &ffi_type_uint32;
> - break;
> - }
> - }
> -}
> -
> static int
> wl_client_connection_data(int fd, uint32_t mask, void *data)
> {
> @@ -294,8 +277,6 @@ wl_client_connection_data(int fd, uint32_t mask, void *data)
> if (wl_debug)
> wl_closure_print(closure, object, false);
>
> - deref_new_objects(closure);
> -
> wl_closure_invoke(closure, object,
> object->implementation[opcode], client);
>
> --
> 1.8.1.2
>
> _______________________________________________
> wayland-devel mailing list
> wayland-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
More information about the wayland-devel
mailing list