[PATCH 1/2] protocol: Allow versioned message arguments

alexl at redhat.com alexl at redhat.com
Wed May 8 03:51:06 PDT 2013


From: Alexander Larsson <alexl at redhat.com>

This allows an event to be extended in a backwards compatible way (on
the client side) by marking an argument with a since attribute.
Any arguments with a since value later than then value of the message
itself will be marked optional and the demarshaller will supply
default values for these arguments.

Versioned arguments must be added at the end of the message in strictly
increasing version order. We don't allow optional arguments of fd type
as we can't make useful "default" fds.

Optional arguments have a default value of 0 or 0 length, but messages
of int/unsigned/fixed can have a different default specified.

The changes in the generated client code are ABI compatible, in two senses:
 * For old servers not passing the optional arguments a new client
   will get the default value for the argument
 * A client compiled against the old client headers will not see the
   new arguments, but c calling conventions makes the call work as
   before.

However, this is not strictly API compatible. Building a client that uses
the old version of the interface against a new client header will get a
warning about the callback types mismatching (due to the new arguments).
We could possibly solve this by adding some #ifdefs that you then need
to define to get the new arguments.
---
 src/connection.c      | 81 +++++++++++++++++++++++++++++++++++++++++++--------
 src/scanner.c         | 38 ++++++++++++++++++++++++
 src/wayland-private.h |  2 ++
 3 files changed, 109 insertions(+), 12 deletions(-)

diff --git a/src/connection.c b/src/connection.c
index dca134b..f74f261 100644
--- a/src/connection.c
+++ b/src/connection.c
@@ -399,9 +399,33 @@ wl_connection_put_fd(struct wl_connection *connection, int32_t fd)
 	return 0;
 }
 
+static int
+char_is_digit (char c)
+{
+  return c >= '0' && c <= '9';
+}
+
+static int
+digit_value (char c)
+{
+  return c - '0';
+}
+
 const char *
 get_next_argument(const char *signature, struct argument_details *details)
 {
+	if (*signature == '#') {
+		details->optional = 1;
+		details->default_value = 0;
+		signature++;
+
+		while (char_is_digit (*signature)) {
+			details->default_value = 10*details->default_value + digit_value (*signature);
+			signature++;
+		}
+	} else
+		details->optional = 0;
+
 	if (*signature == '?') {
 		details->nullable = 1;
 		signature++;
@@ -417,8 +441,15 @@ arg_count_for_signature(const char *signature)
 {
 	int count = 0;
 	while (*signature) {
-		if (*signature != '?')
-			count++;
+		if (*signature == '#') {
+			signature++;
+			while (char_is_digit (*signature))
+				signature++;
+		}
+		if (*signature == '?') {
+			signature++;
+		}
+		count++;
 		signature++;
 	}
 	return count;
@@ -572,10 +603,10 @@ wl_connection_demarshal(struct wl_connection *connection,
 			struct wl_map *objects,
 			const struct wl_message *message)
 {
-	uint32_t *p, *next, *end, length, id;
+	uint32_t *p, *next, *end, length, id, first;
 	int fd;
 	char *s;
-	unsigned int i, count, num_arrays;
+	unsigned int i, count, num_arrays, is_optional;;
 	const char *signature;
 	struct argument_details arg;
 	struct wl_closure *closure;
@@ -609,7 +640,7 @@ wl_connection_demarshal(struct wl_connection *connection,
 	for (i = 0; i < count; i++) {
 		signature = get_next_argument(signature, &arg);
 
-		if (arg.type != 'h' && p + 1 > end) {
+		if (!arg.optional && arg.type != 'h' && p + 1 > end) {
 			printf("message too short, "
 			       "object (%d), message %s(%s)\n",
 			       *p, message->name, message->signature);
@@ -617,24 +648,41 @@ wl_connection_demarshal(struct wl_connection *connection,
 			goto err;
 		}
 
+		is_optional = p + 1 > end && arg.type != 'h';
+		if (is_optional)
+			first = arg.default_value;
+		else if (arg.type != 'h')
+			first = *p++;
+		else
+			first = 0;
+
 		switch (arg.type) {
 		case 'u':
-			closure->args[i].u = *p++;
+			closure->args[i].u = first;
 			break;
 		case 'i':
-			closure->args[i].i = *p++;
+			closure->args[i].i = first;
 			break;
 		case 'f':
-			closure->args[i].f = *p++;
+			closure->args[i].f = first;
 			break;
 		case 's':
-			length = *p++;
+			length = first;
 
 			if (length == 0) {
 				closure->args[i].s = NULL;
 				break;
 			}
 
+			if (is_optional) {
+				printf("non-empty optional string, "
+				       "object (%d), message %s(%s)\n",
+				       closure->sender_id, message->name,
+				       message->signature);
+				errno = EINVAL;
+				goto err;
+			}
+
 			next = p + DIV_ROUNDUP(length, sizeof *p);
 			if (next > end) {
 				printf("message too short, "
@@ -659,7 +707,7 @@ wl_connection_demarshal(struct wl_connection *connection,
 			p = next;
 			break;
 		case 'o':
-			id = *p++;
+			id = first;
 			closure->args[i].n = id;
 
 			if (id == 0 && !arg.nullable) {
@@ -671,7 +719,7 @@ wl_connection_demarshal(struct wl_connection *connection,
 			}
 			break;
 		case 'n':
-			id = *p++;
+			id = first;
 			closure->args[i].n = id;
 
 			if (id == 0 && !arg.nullable) {
@@ -692,7 +740,16 @@ wl_connection_demarshal(struct wl_connection *connection,
 
 			break;
 		case 'a':
-			length = *p++;
+			length = first;
+
+			if (is_optional && length != 0) {
+				printf("non-empty optional array, "
+				       "object (%d), message %s(%s)\n",
+				       closure->sender_id, message->name,
+				       message->signature);
+				errno = EINVAL;
+				goto err;
+			}
 
 			next = p + DIV_ROUNDUP(length, sizeof *p);
 			if (next > end) {
diff --git a/src/scanner.c b/src/scanner.c
index 9c14ad3..b41b371 100644
--- a/src/scanner.c
+++ b/src/scanner.c
@@ -76,6 +76,7 @@ struct message {
 	int all_null;
 	int destructor;
 	int since;
+	int arg_since;
 	struct description *description;
 };
 
@@ -94,9 +95,11 @@ struct arg {
 	char *name;
 	enum arg_type type;
 	int nullable;
+	int optional;
 	char *interface_name;
 	struct wl_list link;
 	char *summary;
+	uint32_t default_value;
 };
 
 struct enumeration {
@@ -350,6 +353,7 @@ start_element(void *data, const char *element_name, const char **atts)
 		}
 
 		message->since = ctx->interface->since;
+		message->arg_since = message->since;
 
 		if (strcmp(name, "destroy") == 0 && !message->destructor)
 			fail(ctx, "destroy request should be destructor type");
@@ -361,6 +365,7 @@ start_element(void *data, const char *element_name, const char **atts)
 
 		arg = malloc(sizeof *arg);
 		arg->name = strdup(name);
+		arg->optional = 0;
 
 		if (strcmp(type, "int") == 0)
 			arg->type = INT;
@@ -410,6 +415,33 @@ start_element(void *data, const char *element_name, const char **atts)
 		if (summary)
 			arg->summary = strdup(summary);
 
+		version = ctx->message->since;
+		if (since != NULL) {
+			version = strtol(since, &end, 0);
+			if (errno == EINVAL || end == since || *end != '\0')
+				fail(ctx, "invalid integer\n");
+		}
+		if (version < ctx->message->arg_since)
+			fail(ctx, "since version not increasing\n");
+		ctx->message->arg_since = version;
+
+		if (version > ctx->message->since)
+			arg->optional = 1;
+
+		if (arg->optional && (arg->type == FD))
+		    fail(ctx, "optional FD types not supported\n");
+
+		arg->default_value = 0;
+		if (value) {
+			arg->default_value = strtol(value, &end, 0);
+			if (errno == EINVAL || end == since || *end != '\0')
+				fail(ctx, "invalid integer\n");
+
+			if (arg->default_value != 0 &&
+			    (arg->type != INT && arg->type != UNSIGNED  && arg->type != FIXED))
+				fail(ctx, "non-zero default value only for integers/fixed\n");
+		}
+
 		wl_list_insert(ctx->message->arg_list.prev, &arg->link);
 		ctx->message->arg_count++;
 	} else if (strcmp(element_name, "enum") == 0) {
@@ -1034,6 +1066,12 @@ 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 (a->optional) {
+				printf("#");
+				if (a->default_value != 0)
+					printf("%u", a->default_value);
+			}
+
 			if (is_nullable_type(a) && a->nullable)
 				printf("?");
 
diff --git a/src/wayland-private.h b/src/wayland-private.h
index c4ce6b0..2990704 100644
--- a/src/wayland-private.h
+++ b/src/wayland-private.h
@@ -99,6 +99,8 @@ struct wl_closure {
 struct argument_details {
 	char type;
 	int nullable;
+	int optional;
+	uint32_t default_value;
 };
 
 const char *
-- 
1.8.1.4



More information about the wayland-devel mailing list