proto: swap the ei message parsing for an interface struct

This gets rid of one layer of indirection with the struct message in
between - we can now call from the proto parser into the handling code
directly.

Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>

src/libei-proto.c

@@ -32,156 +32,97 @@
 #include "libei-proto.h"
 #include "brei-shared.h"
 
-void
-message_free(struct message *msg)
-{
-	switch (msg->type) {
-	case MESSAGE_SEAT_ADDED:
-		free(msg->seat_added.name);
-		break;
-	case MESSAGE_DEVICE_KEYMAP:
-		xclose(msg->device_keymap.keymap_fd);
-		break;
-	case MESSAGE_DEVICE_ADDED:
-		free(msg->device_added.name);
-		break;
-	default:
-		break;
-	}
-	free(msg);
-}
-
 static void servermessage_cleanup(ServerMessage **m) {
 	if (*m)
 		server_message__free_unpacked(*m, NULL);
 }
 #define _cleanup_servermessage_ _cleanup_(servermessage_cleanup)
 
-struct message *
+
-ei_proto_parse_message(struct brei_message *bmsg, size_t *consumed)
+int
+ei_proto_handle_message(struct ei *ei,
+		       const struct ei_proto_interface *interface,
+		       struct brei_message *bmsg)
 {
 	_cleanup_servermessage_ ServerMessage *proto = server_message__unpack(NULL,
 									      bmsg->len,
 									      (const unsigned char*)bmsg->data);
 	if (!proto)
-		return NULL;
+		return -EAGAIN;
 
-	*consumed = bmsg->len;
+#define call(field, ...) \
+	({ \
+	int r = (interface->field == NULL) ? -EPROTO : interface->field(__VA_ARGS__); \
+	log_debug(ei, "message type '" #field "': %d\n", r); \
+	r; \
+	})
 
-	_cleanup_message_ struct message *msg = xalloc(sizeof(*msg));
+	int rc;
-
-	bool success = true;
 	switch (proto->msg_case) {
 	case SERVER_MESSAGE__MSG_CONNECTED:
-		*msg = (struct message) {
+		rc = call(connected, ei);
-			.type = MESSAGE_CONNECTED,
-		};
 		break;
 	case SERVER_MESSAGE__MSG_DISCONNECTED:
-		*msg = (struct message) {
+		rc = call(disconnected, ei);
-			.type = MESSAGE_DISCONNECTED,
-		};
 		break;
 	case SERVER_MESSAGE__MSG_SEAT_ADDED:
-		{
+		rc = call(seat_added, ei,
-			SeatAdded *a = proto->seat_added;
+			  proto->seat_added->seatid,
-			*msg = (struct message) {
+			  proto->seat_added->name, proto->seat_added->capabilities);
-				.type = MESSAGE_SEAT_ADDED,
-				.seat_added.seatid = a->seatid,
-				.seat_added.name = xstrdup(a->name),
-				.seat_added.capabilities = a->capabilities,
-			};
-		}
 		break;
 	case SERVER_MESSAGE__MSG_SEAT_REMOVED:
-		{
+		rc = call(seat_removed, ei,
-			SeatRemoved *r = proto->seat_removed;
+			  proto->seat_removed->seatid);
-			*msg = (struct message) {
-				.type = MESSAGE_SEAT_REMOVED,
-				.seat_removed.seatid = r->seatid,
-			};
-		}
 		break;
 	case SERVER_MESSAGE__MSG_DEVICE_ADDED:
-		{
+		rc = call(device_added, ei,
-			DeviceAdded *a = proto->device_added;
+			  proto->device_added->deviceid,
-			*msg = (struct message) {
+			  proto->device_added->seatid,
-				.type = MESSAGE_DEVICE_ADDED,
+			  proto->device_added->name,
-				.device_added.deviceid = a->deviceid,
+			  proto->device_added->capabilities);
-				.device_added.name = a->name[0] ? xstrdup(a->name) : NULL,
-				.device_added.capabilities = a->capabilities,
-				.device_added.seatid = a->seatid,
-			};
-		}
 		break;
 	case SERVER_MESSAGE__MSG_DEVICE_ADDED_DONE:
-		{
+		rc = call(device_done, ei,
-			DeviceAddedDone *d = proto->device_added_done;
+			  proto->device_added_done->deviceid);
-			*msg = (struct message) {
-				.type = MESSAGE_DEVICE_ADDED_DONE,
-				.device_added_done.deviceid = d->deviceid,
-			};
-		}
 		break;
 	case SERVER_MESSAGE__MSG_DEVICE_KEYMAP:
 		{
-			DeviceKeymap *k = proto->device_keymap;
+		int fd = brei_message_take_fd(bmsg);
-			*msg = (struct message) {
+		rc = call(device_keymap, ei,
-				.type = MESSAGE_DEVICE_KEYMAP,
+			  proto->device_keymap->deviceid,
-				.device_keymap.deviceid = k->deviceid,
+			  proto->device_keymap->keymap_type,
-				.device_keymap.keymap_fd = brei_message_take_fd(bmsg),
+			  fd,
-				.device_keymap.keymap_type = k->keymap_type,
+			  proto->device_keymap->keymap_size);
-				.device_keymap.keymap_size = k->keymap_size,
+		xclose(fd);
-			};
 		}
 		break;
 	case SERVER_MESSAGE__MSG_DEVICE_REGION:
-		{
+		rc = call(device_region, ei,
-			DeviceRegion *r = proto->device_region;
+			  proto->device_region->deviceid,
-			*msg = (struct message) {
+			  proto->device_region->offset_x,
-				.type = MESSAGE_DEVICE_REGION,
+			  proto->device_region->offset_y,
-				.device_region.deviceid = r->deviceid,
+			  proto->device_region->width,
-				.device_region.x = r->offset_x,
+			  proto->device_region->height,
-				.device_region.y = r->offset_y,
+			  proto->device_region->scale);
-				.device_region.w = r->width,
-				.device_region.h = r->height,
-				.device_region.scale = r->scale,
-			};
-		}
 		break;
 	case SERVER_MESSAGE__MSG_DEVICE_REMOVED:
-		{
+		rc = call(device_removed, ei,
-			DeviceRemoved *r = proto->device_removed;
+			  proto->device_removed->deviceid);
-			*msg = (struct message) {
-				.type = MESSAGE_DEVICE_REMOVED,
-				.device_removed.deviceid = r->deviceid,
-			};
-		}
 		break;
 	case SERVER_MESSAGE__MSG_DEVICE_RESUMED:
-		{
+		rc = call(device_resumed, ei,
-			DeviceResumed *r = proto->device_resumed;
+			  proto->device_resumed->deviceid);
-			*msg = (struct message) {
-				.type = MESSAGE_DEVICE_RESUMED,
-				.resumed.deviceid = r->deviceid,
-			};
-		}
 		break;
 	case SERVER_MESSAGE__MSG_DEVICE_SUSPENDED:
-		{
+		rc = call(device_suspended, ei,
-			DeviceSuspended *r = proto->device_suspended;
+			  proto->device_suspended->deviceid);
-			*msg = (struct message) {
-				.type = MESSAGE_DEVICE_SUSPENDED,
-				.suspended.deviceid = r->deviceid,
-			};
-		}
 		break;
 	default:
-		success = false;
+		rc = -EBADMSG;
 		break;
 	}
 
-	return success ? steal(&msg) : NULL;
+	return rc < 0 ? rc : (int)bmsg->len;
 }
 
 static inline void

src/libei-proto.h

@@ -31,99 +31,32 @@
 #include "brei-shared.h"
 #include "libei-private.h"
 
-/* The message type for the wire format */
+/* callbacks invoked during ei_proto_parse_message() */
-enum message_type {
+struct ei_proto_interface {
-	MESSAGE_CONNECTED = 1,
+	int (*connected)(struct ei *ei);
-	MESSAGE_DISCONNECTED,
+	int (*disconnected)(struct ei *ei);
-	MESSAGE_SEAT_ADDED,
+	int (*seat_added)(struct ei *ei, uint32_t seatid,
-	MESSAGE_SEAT_REMOVED,
+			  const char *name, uint32_t capabilities);
-	MESSAGE_DEVICE_ADDED,
+	int (*seat_removed)(struct ei *ei, uint32_t seatid);
-	MESSAGE_DEVICE_ADDED_DONE,
+	int (*device_added)(struct ei *ei, uint32_t deviceid, uint32_t seatid,
-	MESSAGE_DEVICE_KEYMAP,
+			    const char *name, uint32_t capabilities);
-	MESSAGE_DEVICE_REGION,
+	int (*device_removed)(struct ei *ei, uint32_t deviceid);
-	MESSAGE_DEVICE_REMOVED,
+	int (*device_suspended)(struct ei *ei, uint32_t deviceid);
-	MESSAGE_DEVICE_RESUMED,
+	int (*device_resumed)(struct ei *ei, uint32_t deviceid);
-	MESSAGE_DEVICE_SUSPENDED,
+	int (*device_done)(struct ei *ei, uint32_t deviceid);
+	int (*device_region)(struct ei *ei, uint32_t deviceid,
+			     uint32_t x, uint32_t y, uint32_t w, uint32_t h,
+			     double scale);
+	int (*device_keymap)(struct ei *ei, uint32_t deviceid,
+			     enum ei_keymap_type keymap_type,
+			     int keymap_fd,
+			     size_t keymap_size);
 };
 
-_unused_ static inline const char *
+int
-message_type_to_string(enum message_type type)
+ei_proto_handle_message(struct ei *ei,
-{
+		       const struct ei_proto_interface *interface,
-	switch(type) {
+		       struct brei_message *message);
-	CASE_RETURN_STRING(MESSAGE_CONNECTED);
-	CASE_RETURN_STRING(MESSAGE_DISCONNECTED);
-	CASE_RETURN_STRING(MESSAGE_SEAT_ADDED);
-	CASE_RETURN_STRING(MESSAGE_SEAT_REMOVED);
-	CASE_RETURN_STRING(MESSAGE_DEVICE_ADDED);
-	CASE_RETURN_STRING(MESSAGE_DEVICE_ADDED_DONE);
-	CASE_RETURN_STRING(MESSAGE_DEVICE_KEYMAP);
-	CASE_RETURN_STRING(MESSAGE_DEVICE_REGION);
-	CASE_RETURN_STRING(MESSAGE_DEVICE_REMOVED);
-	CASE_RETURN_STRING(MESSAGE_DEVICE_RESUMED);
-	CASE_RETURN_STRING(MESSAGE_DEVICE_SUSPENDED);
-	}
-	assert(!"Unimplemented message type");
-}
-
-struct message {
-	enum message_type type;
-	union {
-		struct message_connected {
-			uint8_t pad; /* no data */
-		} connected;
-		struct message_disconnected {
-			uint8_t pad; /* no data */
-		} disconnected;
-		struct message_seat_added {
-			uint32_t seatid;
-			char *name;
-			uint32_t capabilities;
-		} seat_added;
-		struct message_seat_removed {
-			uint32_t seatid;
-		} seat_removed;
-		struct message_device_added {
-			uint32_t deviceid;
-			char *name;
-			uint32_t capabilities;
-			uint32_t seatid;
-		} device_added;
-		struct message_device_keymap {
-			uint32_t deviceid;
-			enum ei_keymap_type keymap_type;
-			int keymap_fd;
-			size_t keymap_size;
-		} device_keymap;
-		struct message_device_added_done {
-			uint32_t deviceid;
-		} device_added_done;
-		struct message_device_region {
-			uint32_t deviceid;
-			uint32_t x;
-			uint32_t y;
-			uint32_t w;
-			uint32_t h;
-			double scale;
-		} device_region;
-		struct message_device_removed {
-			uint32_t deviceid;
-		} device_removed;
-		struct message_device_resumed {
-			uint32_t deviceid;
-		} resumed;
-		struct message_device_suspended {
-			uint32_t deviceid;
-		} suspended;
-	};
-};
-
-void message_free(struct message *msg);
-
-DEFINE_TRIVIAL_CLEANUP_FUNC(struct message*, message_free);
-#define _cleanup_message_ _cleanup_(message_freep)
-
-struct message *
-ei_proto_parse_message(struct brei_message *message, size_t *consumed);
 
 int
 ei_proto_send_connect(struct ei *ei);

src/libei.c

@@ -522,9 +522,8 @@ handle_msg_seat_removed(struct ei *ei, uint32_t seatid)
 }
 
 static int
-handle_msg_device_added(struct ei *ei, uint32_t deviceid,
+handle_msg_device_added(struct ei *ei, uint32_t deviceid, uint32_t seatid,
-			const char *name, uint32_t capabilities,
+			const char *name, uint32_t capabilities)
-			uint32_t seatid)
 {
 	struct ei_seat *seat = ei_find_seat(ei, seatid);
 
@@ -641,7 +640,7 @@ handle_msg_device_removed(struct ei *ei, uint32_t deviceid)
 }
 
 static int
-handle_msg_resumed(struct ei *ei, uint32_t deviceid)
+handle_msg_device_resumed(struct ei *ei, uint32_t deviceid)
 {
 	log_debug(ei, "Resumed device %#x\n", deviceid);
 
@@ -655,7 +654,7 @@ handle_msg_resumed(struct ei *ei, uint32_t deviceid)
 }
 
 static int
-handle_msg_suspended(struct ei *ei, uint32_t deviceid)
+handle_msg_device_suspended(struct ei *ei, uint32_t deviceid)
 {
 	log_debug(ei, "Suspended device %d\n", deviceid);
 
@@ -811,161 +810,57 @@ ei_peek_event(struct ei *ei)
 	return ei_event_ref(e);
 }
 
-static int
+static int handle_msg_connected(struct ei *ei) {
-connection_new_handle_msg(struct ei *ei, struct message *msg)
-{
-	int rc = 0;
-
-	switch (msg->type) {
-	case MESSAGE_CONNECTED:
-	case MESSAGE_DISCONNECTED:
-	case MESSAGE_SEAT_ADDED:
-	case MESSAGE_SEAT_REMOVED:
-	case MESSAGE_DEVICE_ADDED:
-	case MESSAGE_DEVICE_ADDED_DONE:
-	case MESSAGE_DEVICE_KEYMAP:
-	case MESSAGE_DEVICE_REGION:
-	case MESSAGE_DEVICE_REMOVED:
-	case MESSAGE_DEVICE_RESUMED:
-	case MESSAGE_DEVICE_SUSPENDED:
-		rc = -EPROTO;
-		break;
-	}
-
-	return rc;
-}
-
-static int
-connection_connecting_handle_msg(struct ei *ei, struct message *msg)
-{
-	int rc = 0;
-
-	switch (msg->type) {
-	case MESSAGE_CONNECTED:
 	ei->state = EI_STATE_CONNECTED;
 	queue_connect_event(ei);
-		break;
+	return 0;
-	case MESSAGE_DISCONNECTED:
-		rc = -ECANCELED;
-		break;
-	case MESSAGE_SEAT_ADDED:
-	case MESSAGE_SEAT_REMOVED:
-	case MESSAGE_DEVICE_ADDED:
-	case MESSAGE_DEVICE_ADDED_DONE:
-	case MESSAGE_DEVICE_KEYMAP:
-	case MESSAGE_DEVICE_REGION:
-	case MESSAGE_DEVICE_REMOVED:
-	case MESSAGE_DEVICE_RESUMED:
-	case MESSAGE_DEVICE_SUSPENDED:
-		rc = -EPROTO;
-		break;
 }
 
-	return rc;
+static int handle_msg_disconnected(struct ei *ei) {
+	return -ECANCELED;
 }
 
-static int
+static const struct ei_proto_interface intf_state_backend = {
-connection_connected_handle_msg(struct ei *ei, struct message *msg)
+	/* Everything triggers -EPROTO */
-{
+	.connected = NULL,
-	int rc = 0;
+};
 
-	switch (msg->type) {
+static const struct ei_proto_interface intf_state_connecting = {
-	case MESSAGE_CONNECTED:
+	.connected = handle_msg_connected,
-		rc = -EPROTO;
+	.disconnected = handle_msg_disconnected,
-		break;
+};
-	case MESSAGE_DISCONNECTED:
-		rc = -ECANCELED;
-		break;
-	case MESSAGE_SEAT_ADDED:
-		rc = handle_msg_seat_added(ei,
-					   msg->seat_added.seatid,
-					   msg->seat_added.name,
-					   msg->seat_added.capabilities);
-		break;
-	case MESSAGE_SEAT_REMOVED:
-		rc = handle_msg_seat_removed(ei, msg->seat_removed.seatid);
-		break;
-	case MESSAGE_DEVICE_ADDED:
-		rc = handle_msg_device_added(ei,
-					     msg->device_added.deviceid,
-					     msg->device_added.name,
-					     msg->device_added.capabilities,
-					     msg->device_added.seatid);
-		break;
-	case MESSAGE_DEVICE_ADDED_DONE:
-		rc = handle_msg_device_added_done(ei, msg->device_added_done.deviceid);
-		break;
-	case MESSAGE_DEVICE_KEYMAP:
-		rc = handle_msg_device_keymap(ei,
-					     msg->device_keymap.deviceid,
-					     msg->device_keymap.keymap_type,
-					     msg->device_keymap.keymap_fd,
-					     msg->device_keymap.keymap_size);
-		break;
-	case MESSAGE_DEVICE_REGION:
-		rc = handle_msg_device_region(ei, msg->device_region.deviceid,
-						  msg->device_region.x, msg->device_region.y,
-						  msg->device_region.w, msg->device_region.h,
-						  msg->device_region.scale);
-		break;
-	case MESSAGE_DEVICE_REMOVED:
-		rc = handle_msg_device_removed(ei, msg->device_removed.deviceid);
-		break;
-	case MESSAGE_DEVICE_RESUMED:
-		rc = handle_msg_resumed(ei, msg->resumed.deviceid);
-		break;
-	case MESSAGE_DEVICE_SUSPENDED:
-		rc = handle_msg_suspended(ei, msg->resumed.deviceid);
-		break;
-	}
 
-	return rc;
+static const struct ei_proto_interface intf_state_connected = {
-}
+	.disconnected = handle_msg_disconnected,
+	.seat_added = handle_msg_seat_added,
+	.seat_removed = handle_msg_seat_removed,
+	.device_added = handle_msg_device_added,
+	.device_removed = handle_msg_device_removed,
+	.device_resumed = handle_msg_device_resumed,
+	.device_suspended = handle_msg_device_suspended,
+	.device_region = handle_msg_device_region,
+	.device_keymap = handle_msg_device_keymap,
+	.device_done = handle_msg_device_added_done,
+};
+
+static const struct ei_proto_interface *interfaces[] = {
+	[EI_STATE_NEW] = NULL,
+	[EI_STATE_BACKEND] = &intf_state_backend,
+	[EI_STATE_CONNECTING] = &intf_state_connecting,
+	[EI_STATE_CONNECTED] = &intf_state_connected,
+	[EI_STATE_DISCONNECTING] = NULL,
+	[EI_STATE_DISCONNECTED] = NULL,
+};
 
 static int
 connection_message_callback(struct brei_message *bmsg, void *userdata)
 {
 	struct ei *ei = userdata;
-	size_t consumed;
 
-	_cleanup_message_ struct message *msg = ei_proto_parse_message(bmsg, &consumed);
+	assert(ei->state < ARRAY_LENGTH(interfaces));
-	if (!msg)
+	const struct ei_proto_interface *intf = interfaces[ei->state];
-		return -EBADMSG;
 
-	log_debug(ei, "handling message type %s\n", message_type_to_string(msg->type));
+	return ei_proto_handle_message(ei, intf, bmsg);
-
-	int rc = 0;
-	switch (ei->state) {
-		case EI_STATE_NEW:
-			abort();
-		case EI_STATE_BACKEND:
-			rc = connection_new_handle_msg(ei, msg);
-			break;
-		case EI_STATE_CONNECTING:
-			rc = connection_connecting_handle_msg(ei, msg);
-			break;
-		case EI_STATE_CONNECTED:
-			rc = connection_connected_handle_msg(ei, msg);
-			break;
-		case EI_STATE_DISCONNECTING:
-		case EI_STATE_DISCONNECTED:
-#if 0
-			/* FIXME: this shouldn't happen, but right now we
-			 * may end up calling ei_disconnect() on an error
-			 * which cleans most things up, leaving us with the
-			 * message processing once we have actually cleaned
-			 * up. Needs a bigger rework than currently
-			 * possible.
-			 */
-			assert(!"Protocol error: message received while disconnecting\n");
-#endif
-			break;
-	}
-
-	if (rc < 0)
-		return rc;
-	else
-		return consumed;
 }
 
 static void