From a6e9f4b2cae7c04e75210d02f903770b2c3f8069 Mon Sep 17 00:00:00 2001 From: John Tsiombikas Date: Wed, 23 Mar 2022 22:16:53 +0200 Subject: [PATCH] improved string handling in the wire protocol --- src/client.c | 13 +++++++-- src/client.h | 8 +++++- src/proto.c | 72 ++++++++++++++++++++++++++++++++++++++++++++++++ src/proto.h | 50 +++++++++++++++++++++++++-------- src/proto_unix.c | 60 +++++++++++++++------------------------- 5 files changed, 151 insertions(+), 52 deletions(-) create mode 100644 src/proto.c diff --git a/src/client.c b/src/client.c index cd0ec24..5e711b6 100644 --- a/src/client.c +++ b/src/client.c @@ -85,7 +85,7 @@ void remove_client(struct client *client) if(iter == client) { client_list = iter->next; - free(iter); + free_client(iter); iter = client_list; if(!iter) { if(cfg.led == LED_AUTO) { @@ -99,13 +99,22 @@ void remove_client(struct client *client) if(iter->next == client) { struct client *tmp = iter->next; iter->next = tmp->next; - free(tmp); + free_client(tmp); } else { iter = iter->next; } } } +void free_client(struct client *client) +{ + if(client) { + free(client->name); + free(client->strbuf.buf); + free(client); + } +} + int get_client_type(struct client *client) { return client->type; diff --git a/src/client.h b/src/client.h index 5237ec9..96021bc 100644 --- a/src/client.h +++ b/src/client.h @@ -25,6 +25,8 @@ along with this program. If not, see . #include #endif +#include "proto.h" + /* client types */ enum { CLIENT_X11, /* through the magellan X11 protocol */ @@ -53,17 +55,21 @@ struct client { float sens; /* sensitivity */ struct device *dev; - char name[32]; /* client name (not unique) */ + char *name; /* client name (not unique) */ unsigned int evmask; /* event selection mask */ char reqbuf[64]; int reqbytes; + /* protocol buffer for handling reception of strings in multiple packets */ + struct reqresp_strbuf strbuf; + struct client *next; }; struct client *add_client(int type, void *cdata); void remove_client(struct client *client); +void free_client(struct client *client); int get_client_type(struct client *client); int get_client_socket(struct client *client); diff --git a/src/proto.c b/src/proto.c new file mode 100644 index 0000000..2d6709b --- /dev/null +++ b/src/proto.c @@ -0,0 +1,72 @@ +#include +#include +#include +#define DEF_PROTO_REQ_NAMES +#include "proto.h" + + +int proto_send_str(int fd, int req, const char *str) +{ + int len; + struct reqresp rr = {0}; + + if(fd == -1 || !str) { + return -1; + } + + len = strlen(str); + rr.type = req; + rr.data[6] = len; + + while(len > 0) { + memcpy(rr.data, str, len > REQSTR_CHUNK_SIZE ? REQSTR_CHUNK_SIZE : len); + write(fd, &rr, sizeof rr); + str += REQSTR_CHUNK_SIZE; + len -= REQSTR_CHUNK_SIZE; + rr.data[6] = len | REQSTR_CONT_BIT; + } + return 0; +} + +int proto_recv_str(struct reqresp_strbuf *sbuf, struct reqresp *rr) +{ + int len; + + if(rr->data[6] < 0) return -1; + len = REQSTR_REMLEN(rr); + + if(REQSTR_FIRST(rr)) { + /* first packet, allocate buffer */ + free(sbuf->buf); + sbuf->expect = len; + sbuf->size = sbuf->expect + 1; + if(!(sbuf->buf = malloc(sbuf->size))) { + return -1; + } + sbuf->endp = sbuf->buf; + } + + if(!sbuf->size || !sbuf->buf || !sbuf->endp) { + return -1; + } + if(sbuf->endp < sbuf->buf || sbuf->endp >= sbuf->buf + sbuf->size) { + return -1; + } + if(sbuf->expect > sbuf->size) return -1; + if(len != sbuf->expect) return -1; + + if(len > REQSTR_CHUNK_SIZE) { + len = REQSTR_CHUNK_SIZE; + } + memcpy(sbuf->endp, rr->data, len); + sbuf->endp += len; + sbuf->expect -= len; + + if(sbuf->expect < 0) return -1; + + if(!sbuf->expect) { + *sbuf->endp = 0; + return 1; + } + return 0; +} diff --git a/src/proto.h b/src/proto.h index 4480e25..8418114 100644 --- a/src/proto.h +++ b/src/proto.h @@ -21,25 +21,34 @@ struct reqresp { int32_t data[7]; }; +struct reqresp_strbuf { + char *buf, *endp; + int size; + int expect; +}; + #define REQ_TAG 0x7faa0000 #define REQ_BASE 0x1000 -/* REQ_S* are set, REQ_G* are get requests. +/* REQ_S* are set requests, REQ_G* are get requests. * Quick-reference for request-response data in the comments next to each * request: Q[n] defines request data item n, R[n] defines response data item n * * status responses are 0 for success, non-zero for failure + * + * "remaining length" fields in string transfers have 16 valid bits. Bit 16 is + * used as a flag: 0 for the first packet, 1 for continuation packets. */ enum { /* per-client settings */ - REQ_SET_NAME = REQ_BASE,/* set client name: Q[0-5] next 6 bytes Q[6] remaining length - R[6] status */ + REQ_SET_NAME = REQ_BASE,/* set client name: Q[0-5] next 24 bytes Q[6] remaining length - R[6] status */ REQ_SET_SENS, /* set client sensitivity: Q[0] float - R[6] status */ REQ_GET_SENS, /* get client sensitivity: R[0] float R[6] status */ REQ_SET_EVMASK, /* set event mask: Q[0] mask - R[6] status */ REQ_GET_EVMASK, /* get event mask: R[0] mask R[6] status */ /* device queries */ - REQ_DEV_NAME = 0x2000, /* get device name: R[0-5] next 6 bytes R[6] remaining length or -1 for failure */ + REQ_DEV_NAME = 0x2000, /* get device name: R[0-5] next 24 bytes R[6] remaining length or -1 for failure */ REQ_DEV_PATH, /* get device path: same as above */ REQ_DEV_NAXES, /* get number of axes: R[0] num axes R[6] status */ REQ_DEV_NBUTTONS, /* get number of buttons: same as above */ @@ -70,8 +79,8 @@ enum { REQ_GCFG_LED, /* get LED state: R[0] state R[6] status */ REQ_SCFG_GRAB, /* set device grabbing: Q[0] state - R[6] status */ REQ_GCFG_GRAB, /* get device grabbing: R[0] state R[6] status */ - REQ_SCFG_SERDEV, /* set serial device path: Q[0-5] next 6 bytes Q[6] remaining length - R[6] status */ - REQ_GCFG_SERDEV, /* get serial device path: R[0-5] next 6 bytes R[6] remaining length or -1 for failure */ + REQ_SCFG_SERDEV, /* set serial device path: Q[0-5] next 24 bytes Q[6] remaining length - R[6] status */ + REQ_GCFG_SERDEV, /* get serial device path: R[0-5] next 24 bytes R[6] remaining length or -1 for failure */ /* TODO ... more */ REQ_CFG_SAVE = 0x3ffe, /* save config file: R[6] status */ REQ_CFG_RESTORE, /* load config from file: R[6] status */ @@ -111,23 +120,31 @@ enum { }; +#define REQSTR_CHUNK_SIZE 24 +#define REQSTR_CONT_BIT 0x10000 +#define REQSTR_FIRST(rr) (((rr)->data[6] & REQSTR_CONT_BIT) == 0) +#define REQSTR_REMLEN(rr) ((rr)->data[6] & 0xffff) + +int proto_send_str(int fd, int req, const char *str); +int proto_recv_str(struct reqresp_strbuf *sbuf, struct reqresp *rr); + #ifdef DEF_PROTO_REQ_NAMES -static const char *reqnames_1000[] = { +const char *reqnames_1000[] = { "SET_NAME", "SET_SENS", "GET_SENS", "SET_EVMASK", - "GET_EVMASK", + "GET_EVMASK" }; -static const char *reqnames_2000[] = { +const char *reqnames_2000[] = { "DEV_NAME", "DEV_PATH", "DEV_NAXES", "DEV_NBUTTONS", "DEV_USBID", - "DEV_TYPE", + "DEV_TYPE" }; -static const char *reqnames_3000[] = { +const char *reqnames_3000[] = { "SCFG_SENS", "GCFG_SENS", "SCFG_SENS_AXIS", @@ -151,8 +168,19 @@ static const char *reqnames_3000[] = { "SCFG_GRAB", "GCFG_GRAB", "SCFG_SERDEV", - "GCFG_SERDEV", + "GCFG_SERDEV" }; + +const int reqnames_1000_size = sizeof reqnames_1000 / sizeof *reqnames_1000; +const int reqnames_2000_size = sizeof reqnames_2000 / sizeof *reqnames_2000; +const int reqnames_3000_size = sizeof reqnames_3000 / sizeof *reqnames_3000; +#else +extern const char *reqnames_1000[]; +extern const char *reqnames_2000[]; +extern const char *reqnames_3000[]; +extern const int reqnames_1000_size; +extern const int reqnames_2000_size; +extern const int reqnames_3000_size; #endif /* DEF_PROTO_REQ_NAMES */ #endif /* PROTO_H_ */ diff --git a/src/proto_unix.c b/src/proto_unix.c index 86b028a..67f5154 100644 --- a/src/proto_unix.c +++ b/src/proto_unix.c @@ -27,9 +27,7 @@ along with this program. If not, see . #include #include #include -#define DEF_PROTO_REQ_NAMES #include "proto.h" -#undef DEF_PROTO_REQ_NAMES #include "proto_unix.h" #include "spnavd.h" #ifdef USE_X11 @@ -268,10 +266,7 @@ static int sendresp(struct client *c, struct reqresp *rr, int status) static int handle_request(struct client *c, struct reqresp *req) { - static char *serdev_end; - static int serdev_total_len; - - int i, idx; + int i, idx, res; float fval, fvec[6]; struct device *dev; const char *str = 0; @@ -281,9 +276,15 @@ static int handle_request(struct client *c, struct reqresp *req) switch(req->type & 0xffff) { case REQ_SET_NAME: - memcpy(c->name, req->data, sizeof req->data); - c->name[sizeof req->data] = 0; - logmsg(LOG_INFO, "client name: %s\n", c->name); + if((res = proto_recv_str(&c->strbuf, req)) == -1) { + logmsg(LOG_ERR, "SET_NAME: failed to receive string\n"); + break; + } + if(res) { + c->name = c->strbuf.buf; + c->strbuf.buf = 0; + logmsg(LOG_INFO, "client name: %s\n", c->name); + } break; case REQ_SET_SENS: @@ -315,9 +316,7 @@ static int handle_request(struct client *c, struct reqresp *req) case REQ_DEV_NAME: if((dev = get_client_device(c))) { - req->data[0] = strlen(dev->name); - sendresp(c, req, 0); - write(get_client_socket(c), dev->name, req->data[0]); + proto_send_str(get_client_socket(c), req->type, dev->name); } else { sendresp(c, req, -1); } @@ -325,9 +324,7 @@ static int handle_request(struct client *c, struct reqresp *req) case REQ_DEV_PATH: if((dev = get_client_device(c))) { - req->data[0] = strlen(dev->name); - sendresp(c, req, 0); - write(get_client_socket(c), dev->path, req->data[0]); + proto_send_str(get_client_socket(c), req->type, dev->path); } else { sendresp(c, req, -1); } @@ -583,32 +580,19 @@ static int handle_request(struct client *c, struct reqresp *req) break; case REQ_SCFG_SERDEV: - if(!serdev_end) { - /* first part */ - serdev_end = cfg.serial_dev; - serdev_total_len = req->data[0]; + if((res = proto_recv_str(&c->strbuf, req)) == -1) { + logmsg(LOG_ERR, "SCFG_SERDEV: failed to receive string\n"); + break; } - for(i=0; i<6; i++) { - if(serdev_end < serdev_end + PATH_MAX - 1) { - *serdev_end++ = req->data[i + 1]; - } - req->data[0]--; - } - if(req->data[0] <= 0) { - *serdev_end = 0; - if(strlen(cfg.serial_dev) != serdev_total_len) { - logmsg(LOG_WARNING, "config SCFG_SERDEV, expected %d bytes, got %d\n", serdev_total_len, strlen(cfg.serial_dev)); - } - serdev_end = 0; - serdev_total_len = 0; + if(res) { + strncpy(cfg.serial_dev, c->strbuf.buf, sizeof cfg.serial_dev - 1); + cfg.serial_dev[sizeof cfg.serial_dev - 1] = 0; cfg_changed(); } break; case REQ_GCFG_SERDEV: - req->data[0] = strlen(cfg.serial_dev); - sendresp(c, req, 0); - write(get_client_socket(c), cfg.serial_dev, req->data[0]); + proto_send_str(c->sock, req->type, cfg.serial_dev); break; case REQ_CFG_SAVE: @@ -645,13 +629,13 @@ static const char *reqstr(int req) req &= 0xffff; - if(req >= 0x1000 && req < 0x1000 + sizeof reqnames_1000 / sizeof *reqnames_1000) { + if(req >= 0x1000 && req < 0x1000 + reqnames_1000_size) { return reqnames_1000[req - 0x1000]; } - if(req >= 0x2000 && req < 0x2000 + sizeof reqnames_2000 / sizeof *reqnames_2000) { + if(req >= 0x2000 && req < 0x2000 + reqnames_2000_size) { return reqnames_2000[req - 0x2000]; } - if(req >= 0x3000 && req < 0x3000 + sizeof reqnames_3000 / sizeof *reqnames_3000) { + if(req >= 0x3000 && req < 0x3000 + reqnames_3000_size) { return reqnames_3000[req - 0x3000]; } switch(req) {