On Tue, Feb 26, 2013 at 12:37 PM, Kristian Høgsberg <hoegsb...@gmail.com> wrote:
> 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 <ja...@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.
Yeah, I went through that same thought process and series of checks.
I can't think of a good reason why we would keep a closure around.
However, if we ever do want to store it, we could always expose the
serialize_closure function and store it that way.
> 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@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
_______________________________________________
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
