protocol: Add explicit nullable types

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 <christopher.halse.rogers@canonical.com>

2 files changed, 105 insertions, 8 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: