From 161c690b558fff9d0f407311ae4e1fbe389775f7 Mon Sep 17 00:00:00 2001 From: Christopher James Halse Rogers Date: Mon, 2 Jul 2012 20:03:30 +1000 Subject: [PATCH] protocol: Add explicit nullable types MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Most of the time it does not make sense to pass a NULL object, string, or array to a protocol request. This commit adds an explicit “allow-null” attribute to mark the request arguments where NULL makes sense. Passing a NULL object, string, or array to a protocol request which is not marked as allow-null is now an error. An implementation will never receive a NULL value for these arguments from a client. Signed-off-by: Christopher James Halse Rogers --- src/connection.c | 81 ++++++++++++++++++++++++++++++++++++++++----- src/scanner.c | 32 ++++++++++++++++++ tests/connection-test.c | 87 ++++++++++++++++++++++++++++++++++++++++++++++--- 3 files changed, 188 insertions(+), 12 deletions(-) diff --git a/src/connection.c b/src/connection.c index 72ca908..5946556 100644 --- a/src/connection.c +++ b/src/connection.c @@ -404,6 +404,36 @@ wl_connection_put_fd(struct wl_connection *connection, int32_t fd) return 0; } +struct argument_details { + char type; + int nullable; +}; + +static const char * +get_next_argument(const char *signature, struct argument_details *details) +{ + if (*signature == '?') { + details->nullable = 1; + signature++; + } else + details->nullable = 0; + + details->type = *signature; + return signature + 1; +} + +static int +arg_count_for_signature(const char *signature) +{ + int count = 0; + while (*signature) { + if (*signature != '?') + count++; + signature++; + } + return count; +} + struct wl_closure * wl_closure_vmarshal(struct wl_object *sender, uint32_t opcode, va_list ap, @@ -415,6 +445,8 @@ wl_closure_vmarshal(struct wl_object *sender, int dup_fd; struct wl_array **arrayp, *array; const char **sp, *s; + const char *signature = message->signature; + struct argument_details arg; char *extra; int i, count, fd, extra_size, *fd_ptr; @@ -424,7 +456,7 @@ wl_closure_vmarshal(struct wl_object *sender, return NULL; extra_size = wl_message_size_extra(message); - count = strlen(message->signature) + 2; + count = arg_count_for_signature(signature) + 2; extra = (char *) closure->buffer; start = &closure->buffer[DIV_ROUNDUP(extra_size, sizeof *p)]; end = &closure->buffer[256]; @@ -434,7 +466,9 @@ wl_closure_vmarshal(struct wl_object *sender, closure->types[1] = &ffi_type_pointer; for (i = 2; i < count; i++) { - switch (message->signature[i - 2]) { + signature = get_next_argument(signature, &arg); + + switch (arg.type) { case 'f': closure->types[i] = &ffi_type_sint32; closure->args[i] = p; @@ -463,6 +497,10 @@ wl_closure_vmarshal(struct wl_object *sender, extra += sizeof *sp; s = va_arg(ap, const char *); + + if (!arg.nullable && s == NULL) + goto err_null; + length = s ? strlen(s) + 1: 0; if (p + DIV_ROUNDUP(length, sizeof *p) + 1 > end) goto err; @@ -483,6 +521,10 @@ wl_closure_vmarshal(struct wl_object *sender, extra += sizeof *objectp; object = va_arg(ap, struct wl_object *); + + if (!arg.nullable && object == NULL) + goto err_null; + *objectp = object; if (end - p < 1) goto err; @@ -508,6 +550,10 @@ wl_closure_vmarshal(struct wl_object *sender, extra += sizeof **arrayp; array = va_arg(ap, struct wl_array *); + + if (!arg.nullable && array == NULL) + goto err_null; + if (array == NULL || array->size == 0) { if (end - p < 1) goto err; @@ -542,7 +588,7 @@ wl_closure_vmarshal(struct wl_object *sender, break; default: fprintf(stderr, "unhandled format code: '%c'\n", - message->signature[i - 2]); + arg.type); assert(0); break; } @@ -567,6 +613,15 @@ err: errno = ENOMEM; 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, + message->signature, i); + errno = EINVAL; + return NULL; } struct wl_closure * @@ -579,11 +634,13 @@ wl_connection_demarshal(struct wl_connection *connection, int *fd; char *extra, **s; unsigned int i, count, extra_space; + const char *signature = message->signature; + struct argument_details arg; struct wl_object **object; struct wl_array **array; struct wl_closure *closure; - count = strlen(message->signature) + 2; + count = arg_count_for_signature(signature) + 2; if (count > ARRAY_LENGTH(closure->types)) { printf("too many args (%d)\n", count); errno = EINVAL; @@ -606,6 +663,8 @@ wl_connection_demarshal(struct wl_connection *connection, end = (uint32_t *) ((char *) p + size); extra = (char *) end; for (i = 2; i < count; i++) { + signature = get_next_argument(signature, &arg); + if (p + 1 > end) { printf("message too short, " "object (%d), message %s(%s)\n", @@ -614,7 +673,7 @@ wl_connection_demarshal(struct wl_connection *connection, goto err; } - switch (message->signature[i - 2]) { + switch (arg.type) { case 'u': closure->types[i] = &ffi_type_uint32; closure->args[i] = p++; @@ -784,11 +843,14 @@ copy_fds_to_connection(struct wl_closure *closure, { const struct wl_message *message = closure->message; uint32_t i, count; + struct argument_details arg; + const char *signature = message->signature; int *fd; - count = strlen(message->signature) + 2; + count = arg_count_for_signature(signature) + 2; for (i = 2; i < count; i++) { - if (message->signature[i - 2] != 'h') + signature = get_next_argument(signature, &arg); + if (arg.type != 'h') continue; fd = closure->args[i]; @@ -834,6 +896,8 @@ 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; struct timespec tp; unsigned int time; @@ -847,11 +911,12 @@ wl_closure_print(struct wl_closure *closure, struct wl_object *target, int send) closure->message->name); for (i = 2; i < closure->count; i++) { + signature = get_next_argument(signature, &arg); if (i > 2) fprintf(stderr, ", "); value = closure->args[i]; - switch (closure->message->signature[i - 2]) { + switch (arg.type) { case 'u': fprintf(stderr, "%u", value->uint32); break; diff --git a/src/scanner.c b/src/scanner.c index be8d3d6..4d4537c 100644 --- a/src/scanner.c +++ b/src/scanner.c @@ -93,6 +93,7 @@ enum arg_type { struct arg { char *name; enum arg_type type; + int nullable; char *interface_name; struct wl_list link; char *summary; @@ -220,6 +221,20 @@ fail(struct parse_context *ctx, const char *msg) exit(EXIT_FAILURE); } +static int +is_nullable_type(struct arg *arg) +{ + switch (arg->type) { + /* Strings, objects, and arrays are possibly nullable */ + case STRING: + case OBJECT: + case ARRAY: + return 1; + default: + return 0; + } +} + static void start_element(void *data, const char *element_name, const char **atts) { @@ -231,6 +246,7 @@ start_element(void *data, const char *element_name, const char **atts) struct entry *entry; struct description *description; const char *name, *type, *interface_name, *value, *summary, *since; + const char *allow_null; char *end; int i, version; @@ -242,6 +258,7 @@ start_element(void *data, const char *element_name, const char **atts) summary = NULL; description = NULL; since = NULL; + allow_null = NULL; for (i = 0; atts[i]; i += 2) { if (strcmp(atts[i], "name") == 0) name = atts[i + 1]; @@ -257,6 +274,8 @@ start_element(void *data, const char *element_name, const char **atts) summary = atts[i + 1]; if (strcmp(atts[i], "since") == 0) since = atts[i + 1]; + if (strcmp(atts[i], "allow-null") == 0) + allow_null = atts[i + 1]; } ctx->character_data_length = 0; @@ -364,6 +383,16 @@ start_element(void *data, const char *element_name, const char **atts) break; } + if (allow_null == NULL || strcmp(allow_null, "false") == 0) + arg->nullable = 0; + else if (strcmp(allow_null, "true") == 0) + arg->nullable = 1; + else + fail(ctx, "invalid value for allow-null attribute"); + + if (allow_null != NULL && !is_nullable_type(arg)) + fail(ctx, "allow-null is only valid for objects, strings, and arrays"); + arg->summary = NULL; if (summary) arg->summary = strdup(summary); @@ -969,6 +998,9 @@ emit_messages(struct wl_list *message_list, wl_list_for_each(m, message_list, link) { printf("\t{ \"%s\", \"", m->name); wl_list_for_each(a, &m->arg_list, link) { + if (is_nullable_type(a) && a->nullable) + printf("?"); + switch (a->type) { default: case INT: diff --git a/tests/connection-test.c b/tests/connection-test.c index 066b4bc..a852c17 100644 --- a/tests/connection-test.c +++ b/tests/connection-test.c @@ -233,9 +233,6 @@ TEST(connection_marshal) marshal(&data, "o", 12, &object); assert(data.buffer[2] == object.id); - marshal(&data, "o", 12, NULL); - assert(data.buffer[2] == 0); - marshal(&data, "n", 12, &object); assert(data.buffer[2] == object.id); @@ -252,6 +249,68 @@ TEST(connection_marshal) } static void +expected_fail_marshal(int expected_error, const char *format, ...) +{ + struct wl_closure *closure; + static const uint32_t opcode = 4444; + static const struct wl_interface test_interface = { + .name = "test_object" + }; + static struct wl_object sender = { 0 }; + struct wl_message message = { "test", format, NULL }; + + sender.interface = &test_interface; + sender.id = 1234; + va_list ap; + + va_start(ap, format); + closure = wl_closure_vmarshal(&sender, opcode, ap, &message); + va_end(ap); + + assert(closure == NULL); + assert(errno == expected_error); +} + +TEST(connection_marshal_nullables) +{ + struct marshal_data data; + struct wl_object object; + struct wl_array array; + const char text[] = "curry"; + + setup_marshal_data(&data); + + expected_fail_marshal(EINVAL, "o", NULL); + expected_fail_marshal(EINVAL, "s", NULL); + expected_fail_marshal(EINVAL, "a", NULL); + + marshal(&data, "?o", 12, NULL); + assert(data.buffer[2] == 0); + + marshal(&data, "?a", 12, NULL); + assert(data.buffer[2] == 0); + + marshal(&data, "?s", 12, NULL); + assert(data.buffer[2] == 0); + + object.id = 55293; + marshal(&data, "?o", 12, &object); + assert(data.buffer[2] == object.id); + + array.data = (void *) text; + array.size = sizeof text; + marshal(&data, "?a", 20, &array); + assert(data.buffer[2] == array.size); + assert(memcmp(&data.buffer[3], text, array.size) == 0); + + marshal(&data, "?s", 20, text); + assert(data.buffer[2] == sizeof text); + assert(strcmp((char *) &data.buffer[3], text) == 0); + + release_marshal_data(&data); +} + +static void validate_demarshal_u(struct marshal_data *data, struct wl_object *object, uint32_t u) { @@ -269,7 +328,10 @@ static void validate_demarshal_s(struct marshal_data *data, struct wl_object *object, const char *s) { - assert(strcmp(data->value.s, s) == 0); + if (data->value.s != NULL) + assert(strcmp(data->value.s, s) == 0); + else + assert(s == NULL); } static void @@ -343,12 +405,25 @@ TEST(connection_demarshal) memcpy(&msg[3], data.value.s, msg[2]); demarshal(&data, "s", msg, (void *) validate_demarshal_s); + data.value.s = "superdude"; + msg[0] = 400200; + msg[1] = 24; + msg[2] = 10; + memcpy(&msg[3], data.value.s, msg[2]); + demarshal(&data, "?s", msg, (void *) validate_demarshal_s); + data.value.i = wl_fixed_from_double(-90000.2390); msg[0] = 400200; msg[1] = 12; msg[2] = data.value.i; demarshal(&data, "f", msg, (void *) validate_demarshal_f); + data.value.s = NULL; + msg[0] = 400200; + msg[1] = 12; + msg[2] = 0; + demarshal(&data, "?s", msg, (void *) validate_demarshal_s); + release_marshal_data(&data); } @@ -408,6 +483,10 @@ TEST(connection_marshal_demarshal) marshal_demarshal(&data, (void *) validate_demarshal_s, 28, "s", data.value.s); + data.value.s = "cookie robots"; + marshal_demarshal(&data, (void *) validate_demarshal_s, + 28, "?s", data.value.s); + data.value.h = mkstemp(f); assert(data.value.h >= 0); marshal_demarshal(&data, (void *) validate_demarshal_h, -- 2.7.4