client/server: simplify setup packet handling

Instead of writing (and logging errors for) every parameter, one at a
time, send all fixed size data in a single struct, followed by all the
variable length data.

client-protocol.h

@@ -0,0 +1,31 @@
+#pragma once
+
+#include <stdint.h>
+#include <stdbool.h>
+#include <stddef.h>
+
+struct client_argv {
+    uint16_t len;
+    /* char arg[static len]; */
+};
+
+struct client_data {
+    uint8_t maximized:1;
+    uint8_t fullscreen:1;
+    uint8_t hold:1;
+    uint8_t login_shell:1;
+
+    uint16_t cwd_len;
+    uint16_t term_len;
+    uint16_t title_len;
+    uint16_t app_id_len;
+
+    uint16_t argc;
+
+    /* char cwd[stati cwd_len]; */
+    /* char term[static term_len]; */
+    /* char title[static title_len]; */
+    /* char app_id[static app_id_len]; */
+
+    /* struct client_argv argv[static argc]; */
+};

client.c

@@ -15,6 +15,7 @@
 #define LOG_MODULE "foot-client"
 #define LOG_ENABLE_DBG 0
 #include "log.h"
+#include "client-protocol.h"
 #include "version.h"
 #include "xmalloc.h"
 
@@ -150,6 +151,7 @@ main(int argc, char *const *argv)
 
     /* malloc:ed and needs to be in scope of all goto's */
     char *cwd = NULL;
+    struct client_argv *cargv = NULL;
 
     int fd = socket(AF_UNIX, SOCK_STREAM, 0);
     if (fd == -1) {
@@ -205,99 +207,79 @@ main(int argc, char *const *argv)
             buf_len *= 2;
         } while (errno == ERANGE);
     }
-    const uint16_t cwd_len = strlen(cwd) + 1;
-    const uint16_t term_len = strlen(term) + 1;
-    const uint16_t title_len = strlen(title) + 1;
-    const uint16_t app_id_len = strlen(app_id) + 1;
-    uint32_t total_len = 0;
 
-    /* Calculate total length */
+    /* String lengths, including NULL terminator */
-    total_len += sizeof(cwd_len) + cwd_len;
+    const size_t cwd_len = strlen(cwd) + 1;
-    total_len += sizeof(term_len) + term_len;
+    const size_t term_len = strlen(term) + 1;
-    total_len += sizeof(title_len) + title_len;
+    const size_t title_len = strlen(title) + 1;
-    total_len += sizeof(app_id_len) + app_id_len;
+    const size_t app_id_len = strlen(app_id) + 1;
-    total_len += sizeof(uint8_t);  /* maximized */
-    total_len += sizeof(uint8_t);  /* fullscreen */
-    total_len += sizeof(uint8_t);  /* hold */
-    total_len += sizeof(uint8_t);  /* login_shell */
-    total_len += sizeof(argc);
 
-    for (int i = 0; i < argc; i++) {
+    const struct client_data data = {
-        uint16_t len = strlen(argv[i]) + 1;
+        .maximized = maximized,
-        total_len += sizeof(len) + len;
+        .fullscreen = fullscreen,
+        .hold = hold,
+        .login_shell = login_shell,
+        .cwd_len = cwd_len,
+        .term_len = term_len,
+        .title_len = title_len,
+        .app_id_len = app_id_len,
+        .argc = argc,
+    };
+
+    /* Total packet length, not (yet) including argv[] */
+    size_t total_len = (
+        sizeof(data) +
+        argc * sizeof(uint16_t) +
+        cwd_len +
+        term_len +
+        title_len +
+        app_id_len);
+
+    cargv = xmalloc(argc * sizeof(cargv[0]));
+
+    /* Add argv[] size to total packet length */
+    for (size_t i = 0; i < argc; i++) {
+        const size_t arg_len = strlen(argv[i]) + 1;
+
+        if (arg_len >= 1 << (8 * sizeof(cargv[i].len))) {
+            LOG_ERR("argv length overflow");
+            goto err;
+        }
+
+        cargv[i].len = arg_len;
+        total_len += sizeof(cargv[i]) + cargv[i].len;
     }
 
-    LOG_DBG("term-len: %hu, argc: %d, total-len: %u",
+    /* Check for size overflows */
-            term_len, argc, total_len);
+    if (total_len >= 1llu << (8 * sizeof(uint32_t)) ||
-
+        cwd_len >= 1 << (8 * sizeof(data.cwd_len)) ||
-    if (send(fd, &total_len, sizeof(total_len), 0) != sizeof(total_len)) {
+        term_len >= 1 << (8 * sizeof(data.term_len)) ||
-        LOG_ERRNO("failed to send total length to server");
+        title_len >= 1 << (8 * sizeof(data.title_len)) ||
-        goto err;
+        app_id_len >= 1 << (8 * sizeof(data.app_id_len)) ||
-    }
+        argc > data.argc)
-
-    if (send(fd, &cwd_len, sizeof(cwd_len), 0) != sizeof(cwd_len) ||
-        send(fd, cwd, cwd_len, 0) != cwd_len)
     {
-        LOG_ERRNO("failed to send CWD to server");
+        LOG_ERR("size overflow");
         goto err;
     }
 
-    if (send(fd, &term_len, sizeof(term_len), 0) != sizeof(term_len) ||
+    /* Send everything except argv[] */
-        send(fd, term, term_len, 0) != term_len)
+    if (send(fd, &(uint32_t){total_len}, sizeof(uint32_t), 0) != sizeof(uint32_t) ||
-    {
+        send(fd, &data, sizeof(data), 0) != sizeof(data) ||
-        LOG_ERRNO("failed to send TERM to server");
+        send(fd, cwd, cwd_len, 0) != cwd_len ||
-        goto err;
+        send(fd, term, term_len, 0) != term_len ||
-    }
+        send(fd, title, title_len, 0) != title_len ||
-
-    if (send(fd, &title_len, sizeof(title_len), 0) != sizeof(title_len) ||
-        send(fd, title, title_len, 0) != title_len)
-    {
-        LOG_ERRNO("failed to send title to server");
-        goto err;
-    }
-
-    if (send(fd, &app_id_len, sizeof(app_id_len), 0) != sizeof(app_id_len) ||
         send(fd, app_id, app_id_len, 0) != app_id_len)
     {
-        LOG_ERRNO("failed to send app-id to server");
+        LOG_ERRNO("failed to send setup packet to server");
         goto err;
     }
 
-    if (send(fd, &(uint8_t){maximized}, sizeof(uint8_t), 0) != sizeof(uint8_t)) {
+    /* Send argv[] */
-        LOG_ERRNO("failed to send maximized");
+    for (size_t i = 0; i < argc; i++) {
-        goto err;
+        if (send(fd, &cargv[i], sizeof(cargv[i]), 0) != sizeof(cargv[i]) ||
-    }
+            send(fd, argv[i], cargv[i].len, 0) != cargv[i].len)
-
-    if (send(fd, &(uint8_t){fullscreen}, sizeof(uint8_t), 0) != sizeof(uint8_t)) {
-        LOG_ERRNO("failed to send fullscreen");
-        goto err;
-    }
-
-    if (send(fd, &(uint8_t){hold}, sizeof(uint8_t), 0) != sizeof(uint8_t)) {
-        LOG_ERRNO("failed to send hold");
-        goto err;
-    }
-
-    if (send(fd, &(uint8_t){login_shell}, sizeof(uint8_t), 0) != sizeof(uint8_t)) {
-        LOG_ERRNO("failed to send login-shell");
-        goto err;
-    }
-
-    LOG_DBG("argc = %d", argc);
-    if (send(fd, &argc, sizeof(argc), 0) != sizeof(argc)) {
-        LOG_ERRNO("failed to send argc/argv to server");
-        goto err;
-    }
-
-    for (int i = 0; i < argc; i++) {
-        uint16_t len = strlen(argv[i]) + 1;
-
-        LOG_DBG("argv[%d] = %s (%hu)", i, argv[i], len);
-
-        if (send(fd, &len, sizeof(len), 0) != sizeof(len) ||
-            send(fd, argv[i], len, 0) != len)
         {
-            LOG_ERRNO("failed to send argc/argv to server");
+            LOG_ERRNO("failed to send setup packet to server");
             goto err;
         }
     }
@@ -319,6 +301,7 @@ main(int argc, char *const *argv)
         ret = exit_code;
 
 err:
+    free(cargv);
     free(cwd);
     if (fd != -1)
         close(fd);

meson.build

@@ -123,7 +123,7 @@ executable(
   'render.c', 'render.h',
   'search.c', 'search.h',
   'selection.c', 'selection.h',
-  'server.c', 'server.h',
+  'server.c', 'server.h', 'client-protocol.h',
   'shm.c', 'shm.h',
   'sixel.c', 'sixel.h',
   'slave.c', 'slave.h',
@@ -142,7 +142,7 @@ executable(
 
 executable(
   'footclient',
-  'client.c',
+  'client.c', 'client-protocol.h',
   'log.c', 'log.h',
   'macros.h',
   'xmalloc.c', 'xmalloc.h',

server.c

@@ -16,6 +16,7 @@
 #define LOG_ENABLE_DBG 0
 #include "log.h"
 
+#include "client-protocol.h"
 #include "shm.h"
 #include "terminal.h"
 #include "wayland.h"
@@ -202,103 +203,37 @@ fdm_client(struct fdm *fdm, int fd, int events, void *data)
     uint8_t *p = client->buffer.data;
     const uint8_t *end = &client->buffer.data[client->buffer.idx];
 
-    uint16_t cwd_len;
+    const struct client_data *cdata = (const struct client_data *)p;
-    CHECK_BUF(sizeof(cwd_len));
+    CHECK_BUF(sizeof(*cdata));
-    memcpy(&cwd_len, p, sizeof(cwd_len));
+    p += sizeof(*cdata);
-    p += sizeof(cwd_len);
 
-    CHECK_BUF(cwd_len);
+    CHECK_BUF(cdata->cwd_len);
-    const char *cwd = (const char *)p; p += cwd_len;
+    const char *cwd = (const char *)p; p += cdata->cwd_len;
-    LOG_DBG("CWD = %.*s", cwd_len, cwd);
+    LOG_DBG("CWD = %.*s", cdata->cwd_len, cwd);
 
-    if (cwd_len != strlen(cwd) + 1) {
+    CHECK_BUF(cdata->term_len);
-        LOG_ERR("CWD length mismatch: indicated = %u, actual = %zu",
+    const char *term_env = (const char *)p; p += cdata->term_len;
-                cwd_len - 1, strlen(cwd));
+    LOG_DBG("TERM = %.*s", cdata->term_len, term_env);
-        goto shutdown;
+
+    CHECK_BUF(cdata->title_len);
+    const char *title = (const char *)p; p += cdata->title_len;
+    LOG_DBG("title = %.*s", cdata->title_len, title);
+
+    CHECK_BUF(cdata->app_id_len);
+    const char *app_id = (const char *)p; p += cdata->app_id_len;
+    LOG_DBG("app-id = %.*s", cdata->app_id_len, app_id);
+
+    argv = xcalloc(cdata->argc + 1, sizeof(argv[0]));
+
+    for (uint16_t i = 0; i < cdata->argc; i++) {
+        const struct client_argv *arg = (const struct client_argv *)p;
+        CHECK_BUF(sizeof(*arg));
+        p += sizeof(*arg);
+
+        CHECK_BUF(arg->len);
+        argv[i] = (char *)p; p += arg->len;
     }
 
-    uint16_t term_env_len;
-    CHECK_BUF(sizeof(term_env_len));
-    memcpy(&term_env_len, p, sizeof(term_env_len));
-    p += sizeof(term_env_len);
-
-    CHECK_BUF(term_env_len);
-    const char *term_env = (const char *)p; p += term_env_len;
-    LOG_DBG("TERM = %.*s", term_env_len, term_env);
-
-    if (term_env_len != strlen(term_env) + 1) {
-        LOG_ERR("TERM length mismatch: indicated = %u, actual = %zu",
-                term_env_len - 1, strlen(term_env));
-        goto shutdown;
-    }
-
-    uint16_t title_len;
-    CHECK_BUF(sizeof(title_len));
-    memcpy(&title_len, p, sizeof(title_len));
-    p += sizeof(title_len);
-
-    CHECK_BUF(title_len);
-    const char *title = (const char *)p; p += title_len;
-    LOG_DBG("app-id = %.*s", title_len, title);
-
-    if (title_len != strlen(title) + 1) {
-        LOG_ERR("title length mismatch: indicated = %u, actual = %zu",
-                title_len - 1, strlen(title));
-        goto shutdown;
-    }
-
-    uint16_t app_id_len;
-    CHECK_BUF(sizeof(app_id_len));
-    memcpy(&app_id_len, p, sizeof(app_id_len));
-    p += sizeof(app_id_len);
-
-    CHECK_BUF(app_id_len);
-    const char *app_id = (const char *)p; p += app_id_len;
-    LOG_DBG("app-id = %.*s", app_id_len, app_id);
-
-    if (app_id_len != strlen(app_id) + 1) {
-        LOG_ERR("app-id length mismatch: indicated = %u, actual = %zu",
-                app_id_len - 1, strlen(app_id));
-        goto shutdown;
-    }
-
-    CHECK_BUF(sizeof(uint8_t));
-    const uint8_t maximized = *(const uint8_t *)p; p += sizeof(maximized);
-
-    CHECK_BUF(sizeof(uint8_t));
-    const uint8_t fullscreen = *(const uint8_t *)p; p += sizeof(fullscreen);
-
-    CHECK_BUF(sizeof(uint8_t));
-    const uint8_t hold = *(const uint8_t *)p; p += sizeof(hold);
-
-    CHECK_BUF(sizeof(uint8_t));
-    const uint8_t login_shell = *(const uint8_t *)p; p += sizeof(login_shell);
-
-    CHECK_BUF(sizeof(argc));
-    memcpy(&argc, p, sizeof(argc));
-    p += sizeof(argc);
-
-    argv = xcalloc(argc + 1, sizeof(argv[0]));
-    LOG_DBG("argc = %d", argc);
-
-    for (int i = 0; i < argc; i++) {
-        uint16_t len;
-        CHECK_BUF(sizeof(len));
-        memcpy(&len, p, sizeof(len));
-        p += sizeof(len);
-
-        CHECK_BUF(len);
-        argv[i] = (char *)p; p += strlen(argv[i]) + 1;
-        LOG_DBG("argv[%d] = %s", i, argv[i]);
-
-        if (len != strlen(argv[i]) + 1) {
-            LOG_ERR("argv[%d] length mismatch: indicated = %u, actual = %zu",
-                    i, len - 1, strlen(argv[i]));
-            goto shutdown;
-        }
-    }
-    argv[argc] = NULL;
-
 #undef CHECK_BUF
 
     client->conf = *server->conf;
@@ -308,12 +243,12 @@ fdm_client(struct fdm *fdm, int fd, int events, void *data)
         ? xstrdup(title) : xstrdup(server->conf->title);
     client->conf.app_id = strlen(app_id) > 0
         ? xstrdup(app_id) : xstrdup(server->conf->app_id);
-    client->conf.hold_at_exit = hold;
+    client->conf.hold_at_exit = cdata->hold;
-    client->conf.login_shell = login_shell;
+    client->conf.login_shell = cdata->login_shell;
 
-    if (maximized)
+    if (cdata->maximized)
         client->conf.startup_mode = STARTUP_MAXIMIZED;
-    else if (fullscreen)
+    else if (cdata->fullscreen)
         client->conf.startup_mode = STARTUP_FULLSCREEN;
 
     client->term = term_init(