From 1a38bad756838eb1dd76d3fb3d7df55378f052d2 Mon Sep 17 00:00:00 2001 From: Peter Hutterer Date: Wed, 11 Aug 2021 10:42:00 +1000 Subject: [PATCH] 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 --- src/libei-proto.c | 157 ++++++++++++-------------------------- src/libei-proto.h | 115 ++++++---------------------- src/libei.c | 189 +++++++++++----------------------------------- 3 files changed, 115 insertions(+), 346 deletions(-) diff --git a/src/libei-proto.c b/src/libei-proto.c index 692a44e..338f949 100644 --- a/src/libei-proto.c +++ b/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)); - - bool success = true; + int rc; switch (proto->msg_case) { case SERVER_MESSAGE__MSG_CONNECTED: - *msg = (struct message) { - .type = MESSAGE_CONNECTED, - }; + rc = call(connected, ei); break; case SERVER_MESSAGE__MSG_DISCONNECTED: - *msg = (struct message) { - .type = MESSAGE_DISCONNECTED, - }; + rc = call(disconnected, ei); break; case SERVER_MESSAGE__MSG_SEAT_ADDED: - { - SeatAdded *a = proto->seat_added; - *msg = (struct message) { - .type = MESSAGE_SEAT_ADDED, - .seat_added.seatid = a->seatid, - .seat_added.name = xstrdup(a->name), - .seat_added.capabilities = a->capabilities, - }; - } + rc = call(seat_added, ei, + proto->seat_added->seatid, + proto->seat_added->name, proto->seat_added->capabilities); break; case SERVER_MESSAGE__MSG_SEAT_REMOVED: - { - SeatRemoved *r = proto->seat_removed; - *msg = (struct message) { - .type = MESSAGE_SEAT_REMOVED, - .seat_removed.seatid = r->seatid, - }; - } + rc = call(seat_removed, ei, + proto->seat_removed->seatid); break; case SERVER_MESSAGE__MSG_DEVICE_ADDED: - { - DeviceAdded *a = proto->device_added; - *msg = (struct message) { - .type = MESSAGE_DEVICE_ADDED, - .device_added.deviceid = a->deviceid, - .device_added.name = a->name[0] ? xstrdup(a->name) : NULL, - .device_added.capabilities = a->capabilities, - .device_added.seatid = a->seatid, - }; - } + rc = call(device_added, ei, + proto->device_added->deviceid, + proto->device_added->seatid, + proto->device_added->name, + proto->device_added->capabilities); break; case SERVER_MESSAGE__MSG_DEVICE_ADDED_DONE: - { - DeviceAddedDone *d = proto->device_added_done; - *msg = (struct message) { - .type = MESSAGE_DEVICE_ADDED_DONE, - .device_added_done.deviceid = d->deviceid, - }; - } + rc = call(device_done, ei, + proto->device_added_done->deviceid); break; case SERVER_MESSAGE__MSG_DEVICE_KEYMAP: { - DeviceKeymap *k = proto->device_keymap; - *msg = (struct message) { - .type = MESSAGE_DEVICE_KEYMAP, - .device_keymap.deviceid = k->deviceid, - .device_keymap.keymap_fd = brei_message_take_fd(bmsg), - .device_keymap.keymap_type = k->keymap_type, - .device_keymap.keymap_size = k->keymap_size, - }; + int fd = brei_message_take_fd(bmsg); + rc = call(device_keymap, ei, + proto->device_keymap->deviceid, + proto->device_keymap->keymap_type, + fd, + proto->device_keymap->keymap_size); + xclose(fd); } break; case SERVER_MESSAGE__MSG_DEVICE_REGION: - { - DeviceRegion *r = proto->device_region; - *msg = (struct message) { - .type = MESSAGE_DEVICE_REGION, - .device_region.deviceid = r->deviceid, - .device_region.x = r->offset_x, - .device_region.y = r->offset_y, - .device_region.w = r->width, - .device_region.h = r->height, - .device_region.scale = r->scale, - }; - } + rc = call(device_region, ei, + proto->device_region->deviceid, + proto->device_region->offset_x, + proto->device_region->offset_y, + proto->device_region->width, + proto->device_region->height, + proto->device_region->scale); break; case SERVER_MESSAGE__MSG_DEVICE_REMOVED: - { - DeviceRemoved *r = proto->device_removed; - *msg = (struct message) { - .type = MESSAGE_DEVICE_REMOVED, - .device_removed.deviceid = r->deviceid, - }; - } + rc = call(device_removed, ei, + proto->device_removed->deviceid); break; case SERVER_MESSAGE__MSG_DEVICE_RESUMED: - { - DeviceResumed *r = proto->device_resumed; - *msg = (struct message) { - .type = MESSAGE_DEVICE_RESUMED, - .resumed.deviceid = r->deviceid, - }; - } + rc = call(device_resumed, ei, + proto->device_resumed->deviceid); break; case SERVER_MESSAGE__MSG_DEVICE_SUSPENDED: - { - DeviceSuspended *r = proto->device_suspended; - *msg = (struct message) { - .type = MESSAGE_DEVICE_SUSPENDED, - .suspended.deviceid = r->deviceid, - }; - } + rc = call(device_suspended, ei, + proto->device_suspended->deviceid); break; default: - success = false; + rc = -EBADMSG; break; } - return success ? steal(&msg) : NULL; + return rc < 0 ? rc : (int)bmsg->len; } static inline void diff --git a/src/libei-proto.h b/src/libei-proto.h index 88d3886..26cf03e 100644 --- a/src/libei-proto.h +++ b/src/libei-proto.h @@ -31,99 +31,32 @@ #include "brei-shared.h" #include "libei-private.h" -/* The message type for the wire format */ -enum message_type { - MESSAGE_CONNECTED = 1, - MESSAGE_DISCONNECTED, - MESSAGE_SEAT_ADDED, - MESSAGE_SEAT_REMOVED, - MESSAGE_DEVICE_ADDED, - MESSAGE_DEVICE_ADDED_DONE, - MESSAGE_DEVICE_KEYMAP, - MESSAGE_DEVICE_REGION, - MESSAGE_DEVICE_REMOVED, - MESSAGE_DEVICE_RESUMED, - MESSAGE_DEVICE_SUSPENDED, +/* callbacks invoked during ei_proto_parse_message() */ +struct ei_proto_interface { + int (*connected)(struct ei *ei); + int (*disconnected)(struct ei *ei); + int (*seat_added)(struct ei *ei, uint32_t seatid, + const char *name, uint32_t capabilities); + int (*seat_removed)(struct ei *ei, uint32_t seatid); + int (*device_added)(struct ei *ei, uint32_t deviceid, uint32_t seatid, + const char *name, uint32_t capabilities); + int (*device_removed)(struct ei *ei, uint32_t deviceid); + int (*device_suspended)(struct ei *ei, uint32_t deviceid); + int (*device_resumed)(struct ei *ei, uint32_t deviceid); + 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 * -message_type_to_string(enum message_type type) -{ - switch(type) { - 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_handle_message(struct ei *ei, + const struct ei_proto_interface *interface, + struct brei_message *message); int ei_proto_send_connect(struct ei *ei); diff --git a/src/libei.c b/src/libei.c index 18844cf..b69640c 100644 --- a/src/libei.c +++ b/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, - const char *name, uint32_t capabilities, - uint32_t seatid) +handle_msg_device_added(struct ei *ei, uint32_t deviceid, uint32_t seatid, + const char *name, uint32_t capabilities) { 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 -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 handle_msg_connected(struct ei *ei) { + ei->state = EI_STATE_CONNECTED; + queue_connect_event(ei); + return 0; } -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; - 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 -connection_connected_handle_msg(struct ei *ei, struct message *msg) -{ - int rc = 0; +static const struct ei_proto_interface intf_state_backend = { + /* Everything triggers -EPROTO */ + .connected = NULL, +}; - switch (msg->type) { - case MESSAGE_CONNECTED: - rc = -EPROTO; - 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; - } +static const struct ei_proto_interface intf_state_connecting = { + .connected = handle_msg_connected, + .disconnected = handle_msg_disconnected, +}; - 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); - if (!msg) - return -EBADMSG; + assert(ei->state < ARRAY_LENGTH(interfaces)); + const struct ei_proto_interface *intf = interfaces[ei->state]; - log_debug(ei, "handling message type %s\n", message_type_to_string(msg->type)); - - 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; + return ei_proto_handle_message(ei, intf, bmsg); } static void