proto: drop the Packet message, replace with 4 byte prefix

We need some sort of length field to be able to know how long the next
message is. But for simplicity, we might as well just write that
explicitly on the wire instead of wrapping our messages into yet another
message. This makes the wire format slightly simpler since the first 4
bytes are now always the length, without the previous 0x0d prefix
caused by the protobuf encoding.

0x0d == (field number << 3) | wire_type == 1 << 3 | 5
(see https://protobuf.dev/programming-guides/encoding/#structure)

proto/ei.proto

@@ -376,13 +376,3 @@ message ServerMessage {
     Frame frame = 32;
   }
 }
-
-
-/* Protobuf-C only works correctly if the length of the next message is
- * known, so we need to prefix *every* message with our packed size.
- * This message is used by server and client but it's an implementation
- * detail, not a part of the actual protocol.
- */
-message Packet {
-  fixed32 length = 1;
-}

src/brei-shared.c

@@ -47,49 +47,6 @@ brei_message_take_fd(struct brei_message *m)
 	return fd;
 }
 
-static void packet_cleanup(Packet **f) {
-	if (*f)
-		packet__free_unpacked(*f, NULL);
-}
-#define _cleanup_packet_ _cleanup_(packet_cleanup)
-
-
-/**
- * The BREI (i.e. the EI protocol) is using packets to separate the messages.
- * This helper takes the data of length msglength and returns the position
- * of the next actual message.
- *
- * @param msglen Length of data
- * @param consumed The number of bytes consumed to advance to the next
- * message
- * @return The position of the next message
- */
-static const char *
-brei_next_message(const char *data, size_t *msglen, size_t *consumed)
-{
-	/* Every message is prefixed by a fixed-length packet message which
-	 * contains the length of the next message. packets are always the
-	 * same length, so we only need to calculate the size once.
-	 */
-	static size_t packetlen = 0;
-
-	if (packetlen == 0) {
-		Packet f = PACKET__INIT;
-		f.length = 0xffff;
-		packetlen = packet__get_packed_size(&f);
-		assert(packetlen >= 5);
-	}
-
-	_cleanup_packet_ Packet *packet = packet__unpack(NULL, packetlen, (const unsigned char *)data);
-	if (!packet)
-		return NULL;
-
-	*msglen = packet->length;
-	*consumed = packetlen;
-
-	return data + packetlen;
-}
-
 int
 brei_dispatch(int fd,
 	      int (*callback)(struct brei_message *m, void *user_data),
@@ -115,9 +72,8 @@ brei_dispatch(int fd,
 		if (len == 0)
 			break;
 
-		size_t headerbytes = 0;
-		size_t msglen = 0;
-		const char *msgdata = brei_next_message(data, &msglen, &headerbytes);
+		uint32_t msglen = *(uint32_t*)data;
+		const char *msgdata = data + sizeof(msglen);
 
 		assert(len >= msglen);
 
@@ -143,7 +99,7 @@ brei_dispatch(int fd,
 			rc = consumed;
 			goto error;
 		}
-		idx += consumed + headerbytes;
+		idx += consumed + sizeof(msglen);
 	}
 
 	rc = 0;
@@ -165,43 +121,6 @@ brei_drain_fd(int fd)
 #ifdef _enable_tests_
 #include "util-munit.h"
 
-MUNIT_TEST(test_proto_next_message)
-{
-	char data[64];
-
-	/* Invalid packet, rest can be random */
-	memset(data, 0xab, sizeof(data));
-	data[0] = 0xaa;
-
-	size_t msglen = 0xab;
-	size_t consumed = 0xbc;
-
-	const char *rval = brei_next_message(data, &msglen, &consumed);
-	munit_assert_ptr_null(rval);
-	munit_assert_int(msglen, ==, 0xab);
-	munit_assert_int(consumed, ==, 0xbc);
-
-	/* Now try a valid one */
-	Packet f = PACKET__INIT;
-	f.length = 0xcd;
-	size_t packetlen = packet__get_packed_size(&f);
-
-	unsigned char buf[packetlen * 4];
-	for (int i = 0; i < 4; i++)
-		packet__pack(&f, buf + packetlen * i);
-
-	const char *ptr = (char*)buf;
-	for (int i = 0; i < 4; i++) {
-		const char *next_packet = brei_next_message(ptr, &msglen, &consumed);
-		munit_assert_ptr_equal(next_packet, buf + (i + 1) * packetlen);
-		munit_assert_int(consumed, ==, packetlen);
-		munit_assert_int(msglen, ==, 0xcd);
-		ptr += consumed;
-	}
-
-	return MUNIT_OK;
-}
-
 static int
 brei_dispatch_cb(struct brei_message *msg,
 		 void *user_data)
@@ -216,18 +135,14 @@ brei_dispatch_cb(struct brei_message *msg,
 static inline void
 send_data(int fd, const char *data, size_t data_size)
 {
-	Packet f = PACKET__INIT;
-	f.length = 1;
-	size_t packetlen = packet__get_packed_size(&f);
-
 	/* note: data is null-terminated, we copy all of it but only use
 	   datalen to check truncation works */
 	unsigned char buf[1024] = {0};
-	f.length = data_size;
-	packet__pack(&f, buf);
-	memcpy(buf + packetlen, data, strlen(data)); /* intentionally strlen */
-	int rc = xsend(fd, buf, packetlen + data_size);
-	munit_assert_int(rc, ==, packetlen + data_size);
+	*(uint32_t*)buf = data_size;
+	memcpy(buf + 4, data, strlen(data)); /* intentionally strlen */
+
+	int rc = xsend(fd, buf, 4 + data_size);
+	munit_assert_int(rc, ==, 4 + data_size);
 }
 
 MUNIT_TEST(test_brei_dispatch)

src/libei-proto.c

@@ -277,13 +277,9 @@ static int
 ei_proto_send_msg(struct ei *ei, const ClientMessage *msg)
 {
 	size_t msglen = client_message__get_packed_size(msg);
-	Packet packet = PACKET__INIT;
-	packet.length = msglen;
-	size_t packetlen = packet__get_packed_size(&packet);
-
-	uint8_t buf[packetlen + msglen];
-	packet__pack(&packet, buf);
-	client_message__pack(msg, buf + packetlen);
+	uint8_t buf[4 + msglen];
+	*(uint32_t*)buf = msglen;
+	client_message__pack(msg, buf + 4);
 
 	int rc = min(0, xsend(source_get_fd(ei->source), buf, sizeof(buf)));
 
@@ -296,13 +292,9 @@ _unused_ static int
 ei_proto_send_msg_with_fds(struct ei *ei, const ClientMessage *msg, int *fds)
 {
 	size_t msglen = client_message__get_packed_size(msg);
-	Packet packet = PACKET__INIT;
-	packet.length = msglen;
-	size_t packetlen = packet__get_packed_size(&packet);
-
-	uint8_t buf[packetlen + msglen];
-	packet__pack(&packet, buf);
-	client_message__pack(msg, buf + packetlen);
+	uint8_t buf[4 + msglen];
+	*(uint32_t*)buf = msglen;
+	client_message__pack(msg, buf + 4);
 
 	int rc = min(0, xsend_with_fd(source_get_fd(ei->source), buf, sizeof(buf), fds));
 	log_wire_message(ei, msg, rc);

src/libeis-proto.c

@@ -97,13 +97,9 @@ eis_proto_send_msg(struct eis_client *client, const ServerMessage *msg)
 	log_wire_message(eis_client_get_context(client), msg);
 
 	size_t msglen = server_message__get_packed_size(msg);
-	Packet packet = PACKET__INIT;
-	packet.length = msglen;
-	size_t packetlen = packet__get_packed_size(&packet);
-
-	uint8_t buf[packetlen + msglen];
-	packet__pack(&packet, buf);
-	server_message__pack(msg, buf + packetlen);
+	uint8_t buf[4 + msglen];
+	*(uint32_t*)buf = msglen;
+	server_message__pack(msg, buf + 4);
 	return min(0, xsend(source_get_fd(client->source), buf, sizeof(buf)));
 }
 
@@ -113,13 +109,9 @@ eis_proto_send_msg_with_fds(struct eis_client *client, const ServerMessage *msg,
 	log_wire_message(eis_client_get_context(client), msg);
 
 	size_t msglen = server_message__get_packed_size(msg);
-	Packet packet = PACKET__INIT;
-	packet.length = msglen;
-	size_t packetlen = packet__get_packed_size(&packet);
-
-	uint8_t buf[packetlen + msglen];
-	packet__pack(&packet, buf);
-	server_message__pack(msg, buf + packetlen);
+	uint8_t buf[4 + msglen];
+	*(uint32_t*)buf = msglen;
+	server_message__pack(msg, buf + 4);
 
 	return min(0, xsend_with_fd(source_get_fd(client->source), buf, sizeof(buf), fds));
 }

src/libreis.c

@@ -59,13 +59,9 @@ static int
 send_msg(int fd, const ClientMessage *msg)
 {
 	size_t msglen = client_message__get_packed_size(msg);
-	Packet packet = PACKET__INIT;
-	packet.length = msglen;
-	size_t packetlen = packet__get_packed_size(&packet);
-
-	uint8_t buf[packetlen + msglen];
-	packet__pack(&packet, buf);
-	client_message__pack(msg, buf + packetlen);
+	uint8_t buf[4 + msglen];
+	*(uint32_t*)buf = msglen;
+	client_message__pack(msg, buf + 4);
 	return min(0, xsend(fd, buf, sizeof(buf)));
 }