improved string handling in the wire protocol

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;

src/client.h

@@ -25,6 +25,8 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
 #include <X11/Xlib.h>
 #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);

src/proto.c

@@ -0,0 +1,72 @@
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+#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;
+}

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_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 6 bytes R[6] remaining length or -1 for failure */
+	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_ */

src/proto_unix.c

@@ -27,9 +27,7 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
 #include <sys/socket.h>
 #include <sys/stat.h>
 #include <sys/un.h>
-#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;
+	int i, idx, res;
-	static int serdev_total_len;
-
-	int i, idx;
 	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);
+		if((res = proto_recv_str(&c->strbuf, req)) == -1) {
-		c->name[sizeof req->data] = 0;
+			logmsg(LOG_ERR, "SET_NAME: failed to receive string\n");
-		logmsg(LOG_INFO, "client name: %s\n", c->name);
+			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);
+			proto_send_str(get_client_socket(c), req->type, dev->name);
-			sendresp(c, req, 0);
-			write(get_client_socket(c), dev->name, req->data[0]);
 		} 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);
+			proto_send_str(get_client_socket(c), req->type, dev->path);
-			sendresp(c, req, 0);
-			write(get_client_socket(c), dev->path, req->data[0]);
 		} 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) {
+		if((res = proto_recv_str(&c->strbuf, req)) == -1) {
-			/* first part */
+			logmsg(LOG_ERR, "SCFG_SERDEV: failed to receive string\n");
-			serdev_end = cfg.serial_dev;
+			break;
-			serdev_total_len = req->data[0];
 		}
-		for(i=0; i<6; i++) {
+		if(res) {
-			if(serdev_end < serdev_end + PATH_MAX - 1) {
+			strncpy(cfg.serial_dev, c->strbuf.buf, sizeof cfg.serial_dev - 1);
-				*serdev_end++ = req->data[i + 1];
+			cfg.serial_dev[sizeof cfg.serial_dev - 1] = 0;
-			}
-			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;
 			cfg_changed();
 		}
 		break;
 
 	case REQ_GCFG_SERDEV:
-		req->data[0] = strlen(cfg.serial_dev);
+		proto_send_str(c->sock, req->type, cfg.serial_dev);
-		sendresp(c, req, 0);
-		write(get_client_socket(c), cfg.serial_dev, req->data[0]);
 		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) {