From fa5cde6ce1a7c755c1d8215abdf46e5ec728392c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= <daniel@ekloef.se>
Date: Wed, 23 Jun 2021 13:18:55 +0200
Subject: [PATCH 01/10] server: use config_clone() + config_override_apply()
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

When the connecting client overrides config options, clone the
server’s configuration, and then convert the overridden options to
config overrides, and apply them using config_override_apply().

When destroying the client, free the cloned config using the regular
config_free().
---
 server.c | 72 +++++++++++++++++++++++++++++++++++++++-----------------
 1 file changed, 50 insertions(+), 22 deletions(-)

diff --git a/server.c b/server.c
index 01c2801c..678672ad 100644
--- a/server.c
+++ b/server.c
@@ -120,11 +120,9 @@ instance_destroy(struct terminal_instance *instance, int exit_code)
     /* TODO: clone server conf completely, so that we can just call
      * conf_destroy() here */
     if (instance->conf != NULL) {
-        free(instance->conf->term);
-        free(instance->conf->title);
-        free(instance->conf->app_id);
+        config_free(*instance->conf);
+        free(instance->conf);
     }
-    free(instance->conf);
     free(instance);
 
 }
@@ -296,29 +294,59 @@ fdm_client(struct fdm *fdm, int fd, int events, void *data)
 
     struct config *conf = NULL;
     if (need_to_clone_conf) {
-        conf = xmalloc(sizeof(*conf));
-        *conf = *server->conf;
+        conf = config_clone(server->conf);
 
-        conf->term = strlen(term_env) > 0
-            ? xstrdup(term_env) : xstrdup(server->conf->term);
-        conf->title = strlen(title) > 0
-            ? xstrdup(title) : xstrdup(server->conf->title);
-        conf->app_id = strlen(app_id) > 0
-            ? xstrdup(app_id) : xstrdup(server->conf->app_id);
-        conf->hold_at_exit = cdata.hold;
-        conf->login_shell = cdata.login_shell;
-        conf->no_wait = cdata.no_wait;
+        char buf[1024];
+        config_override_t overrides = tll_init();
 
-        if (cdata.maximized)
-            conf->startup_mode = STARTUP_MAXIMIZED;
-        else if (cdata.fullscreen)
-            conf->startup_mode = STARTUP_FULLSCREEN;
+        if (strlen(term_env) > 0) {
+            snprintf(buf, sizeof(buf), "term=%s", term_env);
+            tll_push_back(overrides, xstrdup(buf));
+        }
+
+        if (strlen(title) > 0) {
+            snprintf(buf, sizeof(buf), "title=%s", title);
+            tll_push_back(overrides, xstrdup(buf));
+        }
+
+        if (strlen(app_id)> 0) {
+            snprintf(buf, sizeof(buf), "app-id=%s", app_id);
+            tll_push_back(overrides, xstrdup(buf));
+        }
+
+        if (cdata.login_shell != server->conf->login_shell)
+            tll_push_back(overrides, xstrdup("login-shell=yes"));
+
+        if (cdata.maximized && server->conf->startup_mode != STARTUP_MAXIMIZED)
+            tll_push_back(overrides, xstrdup("initial-window-mode=maximized"));
+
+        if (cdata.fullscreen && server->conf->startup_mode != STARTUP_FULLSCREEN)
+            tll_push_back(overrides, xstrdup("initial-window-mode=fullscreen"));
 
         if (cdata.width > 0 && cdata.height > 0) {
-            conf->size.type = cdata.size_type;
-            conf->size.width = cdata.width;
-            conf->size.height = cdata.height;
+            switch (cdata.size_type) {
+            case CONF_SIZE_PX:
+                snprintf(buf, sizeof(buf), "initial-window-size-pixels=%ux%u",
+                         cdata.width, cdata.height);
+                tll_push_back(overrides, xstrdup(buf));
+                break;
+
+            case CONF_SIZE_CELLS:
+                snprintf(buf, sizeof(buf), "initial-window-size-chars=%ux%u",
+                         cdata.width, cdata.height);
+                tll_push_back(overrides, xstrdup(buf));
+                break;
+            }
         }
+
+        if (cdata.no_wait != server->conf->no_wait)
+            conf->no_wait = cdata.no_wait;
+
+        if (cdata.hold != server->conf->hold_at_exit)
+            conf->hold_at_exit = cdata.hold;
+
+        config_override_apply(conf, &overrides, false);
+        tll_free_and_free(overrides, free);
     }
 
     *instance = (struct terminal_instance) {

From 136d60606a1e01eb68ed984de63b7a55f8966075 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= <daniel@ekloef.se>
Date: Wed, 23 Jun 2021 14:22:18 +0200
Subject: [PATCH 02/10] client: send overrides for everything that is publicly
 visible in the conf
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Send a generic “overrides” list to the server, containing options in
text, on the format “section.key=value”.

This reduces the size of the base client/server protocol packet, as
well as opens up for a generic -o,--override command line option (not
yet implemented).
---
 client-protocol.h |  29 ++++------
 client.c          | 140 +++++++++++++++++++++++++++-------------------
 meson.build       |   1 +
 server.c          |  80 ++++++--------------------
 4 files changed, 109 insertions(+), 141 deletions(-)

diff --git a/client-protocol.h b/client-protocol.h
index 663199c9..c1291f28 100644
--- a/client-protocol.h
+++ b/client-protocol.h
@@ -4,32 +4,23 @@
 #include <stdbool.h>
 #include <stddef.h>
 
-struct client_argv {
+struct client_string {
     uint16_t len;
-    /* char arg[static len]; */
+    /* char str[static len]; */
 };
 
 struct client_data {
-    unsigned width;
-    unsigned height;
-    uint8_t size_type:1; // Values correspond to enum conf_size_type
-    uint8_t maximized:1;
-    uint8_t fullscreen:1;
-    uint8_t hold:1;
-    uint8_t login_shell:1;
-    uint8_t no_wait:1;
+    bool hold:1;
+    bool no_wait:1;
+    uint8_t reserved:6;
 
     uint16_t cwd_len;
-    uint16_t term_len;
-    uint16_t title_len;
-    uint16_t app_id_len;
-
+    uint16_t override_count;
     uint16_t argc;
 
     /* char cwd[static cwd_len]; */
-    /* char term[static term_len]; */
-    /* char title[static title_len]; */
-    /* char app_id[static app_id_len]; */
+    /* struct client_string overrides[static override_count]; */
+    /* struct client_string argv[static argc]; */
+} __attribute__((packed));
 
-    /* struct client_argv argv[static argc]; */
-};
+_Static_assert(sizeof(struct client_data) == 7, "protocol struct size error");
diff --git a/client.c b/client.c
index 1445146c..9dfeda41 100644
--- a/client.c
+++ b/client.c
@@ -12,6 +12,8 @@
 #include <sys/un.h>
 #include <sys/stat.h>
 
+#include <tllist.h>
+
 #define LOG_MODULE "foot-client"
 #define LOG_ENABLE_DBG 0
 #include "log.h"
@@ -96,22 +98,22 @@ main(int argc, char *const *argv)
         {NULL,                 no_argument,       NULL,   0},
     };
 
-    const char *term = "";
-    const char *title = "";
-    const char *app_id = "";
+    tll(char *) overrides = tll_init();
+
     const char *custom_cwd = NULL;
-    unsigned size_type = 0; // enum conf_size_type (without pulling in tllist/fcft via config.h)
-    unsigned width = 0;
-    unsigned height = 0;
     const char *server_socket_path = NULL;
     enum log_class log_level = LOG_CLASS_INFO;
     enum log_colorize log_colorize = LOG_COLORIZE_AUTO;
-    bool login_shell = false;
-    bool maximized = false;
-    bool fullscreen = false;
     bool hold = false;
     bool no_wait = false;
 
+    char buf[1024];
+
+    /* malloc:ed and needs to be in scope of all goto's */
+    char *_cwd = NULL;
+    struct client_string *coverrides = NULL;
+    struct client_string *cargv = NULL;
+
     while (true) {
         int c = getopt_long(argc, argv, "+t:T:a:w:W:mFLD:s:HNd:l::vh", longopts, NULL);
         if (c == -1)
@@ -119,55 +121,62 @@ main(int argc, char *const *argv)
 
         switch (c) {
         case 't':
-            term = optarg;
+            snprintf(buf, sizeof(buf), "term=%s", optarg);
+            tll_push_back(overrides, xstrdup(buf));
             break;
 
         case 'T':
-            title = optarg;
+            snprintf(buf, sizeof(buf), "title=%s", optarg);
+            tll_push_back(overrides, xstrdup(buf));
             break;
 
         case 'a':
-            app_id = optarg;
+            snprintf(buf, sizeof(buf), "app-id=%s", optarg);
+            tll_push_back(overrides, xstrdup(buf));
             break;
 
         case 'L':
-            login_shell = true;
+            tll_push_back(overrides, xstrdup("login-shell=yes"));
             break;
 
         case 'D': {
             struct stat st;
             if (stat(optarg, &st) < 0 || !(st.st_mode & S_IFDIR)) {
                 fprintf(stderr, "error: %s: not a directory\n", optarg);
-                return ret;
+                goto err;
             }
             custom_cwd = optarg;
             break;
         }
 
-        case 'w':
+        case 'w': {
+            unsigned width, height;
             if (sscanf(optarg, "%ux%u", &width, &height) != 2 || width == 0 || height == 0) {
                 fprintf(stderr, "error: invalid window-size-pixels: %s\n", optarg);
                 return ret;
             }
-            size_type = 0; // CONF_SIZE_PX
+            snprintf(buf, sizeof(buf), "initial-window-size-pixels=%ux%u", width, height);
+            tll_push_back(overrides, xstrdup(buf));
             break;
+        }
 
-        case 'W':
+        case 'W': {
+            unsigned width, height;
             if (sscanf(optarg, "%ux%u", &width, &height) != 2 || width == 0 || height == 0) {
                 fprintf(stderr, "error: invalid window-size-chars: %s\n", optarg);
-                return ret;
+                goto err;
             }
-            size_type = 1; // CONF_SIZE_CELLS
+            snprintf(buf, sizeof(buf), "initial-window-size-chars=%ux%u", width, height);
+            tll_push_back(overrides, xstrdup(buf));
             break;
+        }
 
         case 'm':
-            maximized = true;
-            fullscreen = false;
+            tll_push_back(overrides, xstrdup("initial-window-mode=maximized"));
             break;
 
         case 'F':
-            fullscreen = true;
-            maximized = false;
+            tll_push_back(overrides, xstrdup("initial-window-mode=fullscreen"));
             break;
 
         case 's':
@@ -190,7 +199,7 @@ main(int argc, char *const *argv)
                     "-d,--log-level: %s: argument must be one of %s\n",
                     optarg,
                     log_level_string_hint());
-                return ret;
+                goto err;
             }
             log_level = lvl;
             break;
@@ -205,20 +214,22 @@ main(int argc, char *const *argv)
                 log_colorize = LOG_COLORIZE_ALWAYS;
             else {
                 fprintf(stderr, "%s: argument must be one of 'never', 'always' or 'auto'\n", optarg);
-                return ret;
+                goto err;
             }
             break;
 
         case 'v':
             printf("footclient %s\n", version_and_features());
-            return EXIT_SUCCESS;
+            ret = EXIT_SUCCESS;
+            goto err;
 
         case 'h':
             print_usage(prog_name);
-            return EXIT_SUCCESS;
+            ret = EXIT_SUCCESS;
+            goto err;
 
         case '?':
-            return ret;
+            goto err;
         }
     }
 
@@ -227,10 +238,6 @@ main(int argc, char *const *argv)
 
     log_init(log_colorize, false, LOG_FACILITY_USER, log_level);
 
-    /* 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) {
         LOG_ERRNO("failed to create socket");
@@ -290,33 +297,36 @@ main(int argc, char *const *argv)
 
     /* String lengths, including NULL terminator */
     const size_t cwd_len = strlen(cwd) + 1;
-    const size_t term_len = strlen(term) + 1;
-    const size_t title_len = strlen(title) + 1;
-    const size_t app_id_len = strlen(app_id) + 1;
+    const size_t override_count = tll_length(overrides);
 
     const struct client_data data = {
-        .width = width,
-        .height = height,
-        .size_type = size_type,
-        .maximized = maximized,
-        .fullscreen = fullscreen,
         .hold = hold,
-        .login_shell = login_shell,
         .no_wait = no_wait,
         .cwd_len = cwd_len,
-        .term_len = term_len,
-        .title_len = title_len,
-        .app_id_len = app_id_len,
+        .override_count = override_count,
         .argc = argc,
     };
 
-    /* Total packet length, not (yet) including argv[] */
-    uint64_t total_len = (
-        sizeof(data) +
-        cwd_len +
-        term_len +
-        title_len +
-        app_id_len);
+    /* Total packet length, not (yet) including overrides or argv[] */
+    uint64_t total_len = sizeof(data) + cwd_len;
+
+    /* Add overrides to total packet length */
+    coverrides = xmalloc(override_count * sizeof(coverrides[0]));
+    {
+        size_t i = 0;
+        tll_foreach(overrides, it) {
+            const size_t len = strlen(it->item) + 1;
+
+            if (len >= 1 << (8 * sizeof(coverrides[i].len))) {
+                LOG_ERR("override length overflow");
+                goto err;
+            }
+
+            coverrides[i].len = len;
+            total_len += sizeof(coverrides[i]) + coverrides[i].len;
+            i++;
+        }
+    }
 
     cargv = xmalloc(argc * sizeof(cargv[0]));
 
@@ -336,9 +346,7 @@ main(int argc, char *const *argv)
     /* Check for size overflows */
     if (total_len >= 1llu << (8 * sizeof(uint32_t)) ||
         cwd_len >= 1 << (8 * sizeof(data.cwd_len)) ||
-        term_len >= 1 << (8 * sizeof(data.term_len)) ||
-        title_len >= 1 << (8 * sizeof(data.title_len)) ||
-        app_id_len >= 1 << (8 * sizeof(data.app_id_len)) ||
+        override_count > (size_t)(unsigned int)data.override_count ||
         argc > (int)(unsigned int)data.argc)
     {
         LOG_ERR("size overflow");
@@ -348,21 +356,33 @@ main(int argc, char *const *argv)
     /* Send everything except argv[] */
     if (send(fd, &(uint32_t){total_len}, sizeof(uint32_t), 0) != sizeof(uint32_t) ||
         send(fd, &data, sizeof(data), 0) != sizeof(data) ||
-        send(fd, cwd, cwd_len, 0) != cwd_len ||
-        send(fd, term, term_len, 0) != term_len ||
-        send(fd, title, title_len, 0) != title_len ||
-        send(fd, app_id, app_id_len, 0) != app_id_len)
+        send(fd, cwd, cwd_len, 0) != cwd_len)
     {
         LOG_ERRNO("failed to send setup packet to server");
         goto err;
     }
 
+    /* Send overrides */
+    {
+        size_t i = 0;
+        tll_foreach(overrides, it) {
+            if (send(fd, &coverrides[i], sizeof(coverrides[i]), 0) != sizeof(coverrides[i]) ||
+                send(fd, it->item, coverrides[i].len, 0) != coverrides[i].len)
+            {
+                LOG_ERRNO("failed to send setup packet (overrides) to server");
+                goto err;
+            }
+
+            i++;
+        }
+    }
+
     /* Send argv[] */
     for (size_t i = 0; i < argc; i++) {
         if (send(fd, &cargv[i], sizeof(cargv[i]), 0) != sizeof(cargv[i]) ||
             send(fd, argv[i], cargv[i].len, 0) != cargv[i].len)
         {
-            LOG_ERRNO("failed to send setup packet to server");
+            LOG_ERRNO("failed to send setup packet (argv) to server");
             goto err;
         }
     }
@@ -386,7 +406,9 @@ main(int argc, char *const *argv)
         ret = exit_code;
 
 err:
+    tll_free_and_free(overrides, free);
     free(cargv);
+    free(coverrides);
     free(_cwd);
     if (fd != -1)
         close(fd);
diff --git a/meson.build b/meson.build
index 0c48929a..479f5747 100644
--- a/meson.build
+++ b/meson.build
@@ -211,6 +211,7 @@ executable(
   'macros.h',
   'util.h',
   version,
+  dependencies: [tllist],
   link_with: common,
   install: true)
 
diff --git a/server.c b/server.c
index 678672ad..36bb0ddc 100644
--- a/server.c
+++ b/server.c
@@ -240,6 +240,7 @@ 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];
+    config_override_t overrides = tll_init();
 
     struct client_data cdata;
     CHECK_BUF(sizeof(cdata));
@@ -250,22 +251,23 @@ fdm_client(struct fdm *fdm, int fd, int events, void *data)
     const char *cwd = (const char *)p; p += cdata.cwd_len;
     LOG_DBG("CWD = %.*s", cdata.cwd_len, cwd);
 
-    CHECK_BUF_AND_NULL(cdata.term_len);
-    const char *term_env = (const char *)p; p += cdata.term_len;
-    LOG_DBG("TERM = %.*s", cdata.term_len, term_env);
+    /* Overrides */
+    for (uint16_t i = 0; i < cdata.override_count; i++) {
+        const struct client_string *arg = (const struct client_string *)p;
+        CHECK_BUF(sizeof(*arg));
+        p += sizeof(*arg);
 
-    CHECK_BUF_AND_NULL(cdata.title_len);
-    const char *title = (const char *)p; p += cdata.title_len;
-    LOG_DBG("title = %.*s", cdata.title_len, title);
+        CHECK_BUF_AND_NULL(arg->len);
+        const char *str = (const char *)p;
+        p += arg->len;
 
-    CHECK_BUF_AND_NULL(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);
+        tll_push_back(overrides, xstrdup(str));
+    }
 
+    /* argv */
     argv = xcalloc(cdata.argc + 1, sizeof(argv[0]));
-
     for (uint16_t i = 0; i < cdata.argc; i++) {
-        struct client_argv arg;
+        struct client_string arg;
         CHECK_BUF(sizeof(arg));
         memcpy(&arg, p, sizeof(arg));
         p += sizeof(arg);
@@ -282,63 +284,14 @@ fdm_client(struct fdm *fdm, int fd, int events, void *data)
     struct terminal_instance *instance = malloc(sizeof(struct terminal_instance));
 
     const bool need_to_clone_conf =
-        strlen(term_env) > 0 ||
-        strlen(title) > 0 ||
-        strlen(app_id) > 0 ||
+        tll_length(overrides)> 0 ||
         cdata.hold != server->conf->hold_at_exit ||
-        cdata.login_shell != server->conf->login_shell ||
-        cdata.no_wait != server->conf->no_wait ||
-        (cdata.maximized && server->conf->startup_mode != STARTUP_MAXIMIZED) ||
-        (cdata.fullscreen && server->conf->startup_mode != STARTUP_FULLSCREEN) ||
-        (cdata.width > 0 && cdata.height > 0);
+        cdata.no_wait != server->conf->no_wait;
 
     struct config *conf = NULL;
     if (need_to_clone_conf) {
         conf = config_clone(server->conf);
 
-        char buf[1024];
-        config_override_t overrides = tll_init();
-
-        if (strlen(term_env) > 0) {
-            snprintf(buf, sizeof(buf), "term=%s", term_env);
-            tll_push_back(overrides, xstrdup(buf));
-        }
-
-        if (strlen(title) > 0) {
-            snprintf(buf, sizeof(buf), "title=%s", title);
-            tll_push_back(overrides, xstrdup(buf));
-        }
-
-        if (strlen(app_id)> 0) {
-            snprintf(buf, sizeof(buf), "app-id=%s", app_id);
-            tll_push_back(overrides, xstrdup(buf));
-        }
-
-        if (cdata.login_shell != server->conf->login_shell)
-            tll_push_back(overrides, xstrdup("login-shell=yes"));
-
-        if (cdata.maximized && server->conf->startup_mode != STARTUP_MAXIMIZED)
-            tll_push_back(overrides, xstrdup("initial-window-mode=maximized"));
-
-        if (cdata.fullscreen && server->conf->startup_mode != STARTUP_FULLSCREEN)
-            tll_push_back(overrides, xstrdup("initial-window-mode=fullscreen"));
-
-        if (cdata.width > 0 && cdata.height > 0) {
-            switch (cdata.size_type) {
-            case CONF_SIZE_PX:
-                snprintf(buf, sizeof(buf), "initial-window-size-pixels=%ux%u",
-                         cdata.width, cdata.height);
-                tll_push_back(overrides, xstrdup(buf));
-                break;
-
-            case CONF_SIZE_CELLS:
-                snprintf(buf, sizeof(buf), "initial-window-size-chars=%ux%u",
-                         cdata.width, cdata.height);
-                tll_push_back(overrides, xstrdup(buf));
-                break;
-            }
-        }
-
         if (cdata.no_wait != server->conf->no_wait)
             conf->no_wait = cdata.no_wait;
 
@@ -346,7 +299,6 @@ fdm_client(struct fdm *fdm, int fd, int events, void *data)
             conf->hold_at_exit = cdata.hold;
 
         config_override_apply(conf, &overrides, false);
-        tll_free_and_free(overrides, free);
     }
 
     *instance = (struct terminal_instance) {
@@ -377,6 +329,7 @@ fdm_client(struct fdm *fdm, int fd, int events, void *data)
         instance->client = client;
         client->instance = instance;
         free(argv);
+        tll_free_and_free(overrides, free);
     }
 
     return true;
@@ -385,6 +338,7 @@ shutdown:
     LOG_DBG("client FD=%d: disconnected", client->fd);
 
     free(argv);
+    tll_free_and_free(overrides, free);
     fdm_del(fdm, fd);
     client->fd = -1;
 

From 04d42662c78823fbb2e71c7a481c86293a031c9a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= <daniel@ekloef.se>
Date: Wed, 23 Jun 2021 14:30:02 +0200
Subject: [PATCH 03/10] =?UTF-8?q?server:=20avoid=20=E2=80=9Cmember=20acces?=
 =?UTF-8?q?s=20within=20misaligned=20address=E2=80=9D=20ASAN=20warning?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

---
 server.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/server.c b/server.c
index 36bb0ddc..b8ae5685 100644
--- a/server.c
+++ b/server.c
@@ -253,13 +253,13 @@ fdm_client(struct fdm *fdm, int fd, int events, void *data)
 
     /* Overrides */
     for (uint16_t i = 0; i < cdata.override_count; i++) {
-        const struct client_string *arg = (const struct client_string *)p;
-        CHECK_BUF(sizeof(*arg));
-        p += sizeof(*arg);
+        struct client_string arg;
+        CHECK_BUF(sizeof(arg));
+        memcpy(&arg, p, sizeof(arg)); p += sizeof(arg);
 
-        CHECK_BUF_AND_NULL(arg->len);
+        CHECK_BUF_AND_NULL(arg.len);
         const char *str = (const char *)p;
-        p += arg->len;
+        p += arg.len;
 
         tll_push_back(overrides, xstrdup(str));
     }
@@ -269,8 +269,7 @@ fdm_client(struct fdm *fdm, int fd, int events, void *data)
     for (uint16_t i = 0; i < cdata.argc; i++) {
         struct client_string arg;
         CHECK_BUF(sizeof(arg));
-        memcpy(&arg, p, sizeof(arg));
-        p += sizeof(arg);
+        memcpy(&arg, p, sizeof(arg)); p += sizeof(arg);
 
         CHECK_BUF_AND_NULL(arg.len);
         argv[i] = (char *)p; p += arg.len;

From c6b5ac92997bab13c795543e0fa87d30cc812bc4 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= <daniel@ekloef.se>
Date: Wed, 23 Jun 2021 14:30:29 +0200
Subject: [PATCH 04/10] =?UTF-8?q?main:=20usage:=20add=20=E2=80=98=3D?=
 =?UTF-8?q?=E2=80=99=20between=20--override=20and=20its=20value?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This is how all other long options are documented in the usage.
---
 main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/main.c b/main.c
index df9edfd6..2c392fca 100644
--- a/main.c
+++ b/main.c
@@ -62,7 +62,7 @@ print_usage(const char *prog_name)
         "Options:\n"
         "  -c,--config=PATH                        load configuration from PATH ($XDG_CONFIG_HOME/foot/foot.ini)\n"
         "  -C,--check-config                       verify configuration, exit with 0 if ok, otherwise exit with 1\n"
-        "  -o,--override [section.]key=value       override configuration option\n"
+        "  -o,--override=[section.]key=value       override configuration option\n"
         "  -f,--font=FONT                          comma separated list of fonts in fontconfig format (monospace)\n"
         "  -t,--term=TERM                          value to set the environment variable TERM to (%s)\n"
         "  -T,--title=TITLE                        initial window title (foot)\n"

From 63a64bdca1978814ade32949e4cbf0d9758e6cbe Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= <daniel@ekloef.se>
Date: Wed, 23 Jun 2021 14:34:36 +0200
Subject: [PATCH 05/10] client: implement -o,--override

---
 client.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/client.c b/client.c
index 9dfeda41..9c08a037 100644
--- a/client.c
+++ b/client.c
@@ -63,6 +63,7 @@ print_usage(const char *prog_name)
            "  -s,--server-socket=PATH                 path to the server UNIX domain socket (default=$XDG_RUNTIME_DIR/foot-$WAYLAND_DISPLAY.sock)\n"
            "  -H,--hold                               remain open after child process exits\n"
            "  -N,--no-wait                            detach the client process from the running terminal, exiting immediately\n"
+           "  -o,--override=[section.]key=value       override configuration option\n"
            "  -d,--log-level={info|warning|error}     log level (info)\n"
            "  -l,--log-colorize=[{never|always|auto}] enable/disable colorization of log output on stderr\n"
            "  -v,--version                            show the version number and quit\n");
@@ -91,6 +92,7 @@ main(int argc, char *const *argv)
         {"server-socket",      required_argument, NULL, 's'},
         {"hold",               no_argument,       NULL, 'H'},
         {"no-wait",            no_argument,       NULL, 'N'},
+        {"override",           required_argument, NULL, 'o'},
         {"log-level",          required_argument, NULL, 'd'},
         {"log-colorize",       optional_argument, NULL, 'l'},
         {"version",            no_argument,       NULL, 'v'},
@@ -115,7 +117,7 @@ main(int argc, char *const *argv)
     struct client_string *cargv = NULL;
 
     while (true) {
-        int c = getopt_long(argc, argv, "+t:T:a:w:W:mFLD:s:HNd:l::vh", longopts, NULL);
+        int c = getopt_long(argc, argv, "+t:T:a:w:W:mFLD:s:HNo:d:l::vh", longopts, NULL);
         if (c == -1)
             break;
 
@@ -191,6 +193,10 @@ main(int argc, char *const *argv)
             no_wait = true;
             break;
 
+        case 'o':
+            tll_push_back(overrides, xstrdup(optarg));
+            break;
+
         case 'd': {
             int lvl = log_level_from_string(optarg);
             if (unlikely(lvl < 0)) {

From fcc20456cd7c4685547afab5d74ac4762f4e3787 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= <daniel@ekloef.se>
Date: Wed, 23 Jun 2021 14:34:09 +0200
Subject: [PATCH 06/10] =?UTF-8?q?doc:=20foot.1:=20remove=20trailing=20spac?=
 =?UTF-8?q?e,=20add=20=E2=80=98=3D=E2=80=99=20between=20option=20name=20an?=
 =?UTF-8?q?d=20its=20value?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

---
 doc/foot.1.scd | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/doc/foot.1.scd b/doc/foot.1.scd
index 16d639fa..80203ad9 100644
--- a/doc/foot.1.scd
+++ b/doc/foot.1.scd
@@ -30,9 +30,9 @@ the foot command line
 	Verify configuration and then exit with 0 if ok, otherwise exit
 	with 230 (see *EXIT STATUS*).
 
-*-o*,*--override* [_SECTION_.]_KEY_=_VALUE_
+*-o*,*--override*=[_SECTION_.]_KEY_=_VALUE_
 	Override an option set in the configuration file. If _SECTION_ is not
-	given, defaults to _main_. 
+	given, defaults to _main_.
 
 *-f*,*--font*=_FONT_
 	Comma separated list of fonts to use, in fontconfig format (see

From 8640a9c99a320800d0626ff93b8773ccfca8c83b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= <daniel@ekloef.se>
Date: Wed, 23 Jun 2021 14:34:23 +0200
Subject: [PATCH 07/10] doc: footclient.1: document -o,--override

---
 doc/footclient.1.scd | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/doc/footclient.1.scd b/doc/footclient.1.scd
index a9fb8c69..8cb05316 100644
--- a/doc/footclient.1.scd
+++ b/doc/footclient.1.scd
@@ -65,6 +65,10 @@ terminal has terminated.
 	Detach the client process from the running terminal, exiting
 	immediately.
 
+*-o*,*--override*=[_SECTION_.]_KEY_=_VALUE_
+	Override an option set in the configuration file. If _SECTION_ is not
+	given, defaults to _main_.
+
 *-d*,*--log-level*={*info*,*warning*,*error*}
 	Log level, used both for log output on stderr as well as
 	syslog. Default: _info_.

From 5b9a000b9b795598d559e46c47d6d4211ff22448 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= <daniel@ekloef.se>
Date: Wed, 23 Jun 2021 14:34:50 +0200
Subject: [PATCH 08/10] completions: add -o,--override to footclient

---
 completions/bash/footclient      | 3 ++-
 completions/fish/footclient.fish | 1 +
 completions/zsh/_footclient      | 1 +
 3 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/completions/bash/footclient b/completions/bash/footclient
index 6e065a97..8e0aa017 100644
--- a/completions/bash/footclient
+++ b/completions/bash/footclient
@@ -13,6 +13,7 @@ _footclient()
         "--log-level"
         "--log-colorize"
         "--maximized"
+        "--override"
         "--server-socket"
         "--term"
         "--title"
@@ -61,7 +62,7 @@ _footclient()
         COMPREPLY=( $(compgen -W "error warning info" -- ${cur}) )
     elif [[ ${prev} == '--log-colorize' ]] ; then
         COMPREPLY=( $(compgen -W "never always auto" -- ${cur}) )
-    elif [[ ${prev} =~ ^(--app-id|--help|--title|--version|--window-size-chars|--window-size-pixels|)$ ]] ; then
+    elif [[ ${prev} =~ ^(--app-id|--help|--override|--title|--version|--window-size-chars|--window-size-pixels|)$ ]] ; then
         : # don't autocomplete for these flags
     else
         # complete commands from $PATH
diff --git a/completions/fish/footclient.fish b/completions/fish/footclient.fish
index 826508ef..9b8caa84 100644
--- a/completions/fish/footclient.fish
+++ b/completions/fish/footclient.fish
@@ -11,6 +11,7 @@ complete -c footclient -x -s W -l window-size-chars
 complete -c footclient -F -s s -l server-socket                                                             -d "override the default path to the foot server socket ($XDG_RUNTIME_DIR/foot-$WAYLAND_DISPLAY.sock)"
 complete -c footclient    -s H -l hold                                                                      -d "remain open after child process exits"
 complete -c footclient    -s N -l no-wait                                                                   -d "detach the client process from the running terminal, exiting immediately"
+complete -c footclient -x -s o -l override                                                                  -d "configuration option to override, in form SECTION.KEY=VALUE"
 complete -c footclient -x -s d -l log-level          -a "info warning error"                                -d "log-level (info)"
 complete -c footclient -x -s l -l log-colorize       -a "always never auto"                                 -d "enable or disable colorization of log output on stderr"
 complete -c footclient    -s v -l version                                                                   -d "show the version number and quit"
diff --git a/completions/zsh/_footclient b/completions/zsh/_footclient
index 411d4ec5..3889c488 100644
--- a/completions/zsh/_footclient
+++ b/completions/zsh/_footclient
@@ -14,6 +14,7 @@ _arguments \
     '(-s --server-socket)'{-s,--server-socket}'[override the default path to the foot server socket ($XDG_RUNTIME_DIR/foot-$WAYLAND_DISPLAY.sock)]:server:_files' \
     '(-H --hold)'{-H,--hold}'[remain open after child process exits]' \
     '(-N --no-wait)'{-N,--no-wait}'[detach the client process from the running terminal, exiting immediately]' \
+    '(-o --override)'{-o,--override}'[configuration option to override, in form SECTION.KEY=VALUE]:()' \
     '(-d --log-level)'{-d,--log-level}'[log level (info)]:loglevel:(info warning error)' \
     '(-l --log-colorize)'{-l,--log-colorize}'[enable or disable colorization of log output on stderr]:logcolor:(never always auto)' \
     '(-v --version)'{-v,--version}'[show the version number and quit]' \

From bac3964039a23e7602708408c4d452ec7aa5dbdb Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= <daniel@ekloef.se>
Date: Wed, 23 Jun 2021 14:36:31 +0200
Subject: [PATCH 09/10] changelog: add ref to #600 (-o,--override for
 footclient)

---
 CHANGELOG.md | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index c68ac569..208f0f04 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -64,7 +64,8 @@
 * Support for LS2 and LS3 (locking shift) escape sequences
   (https://codeberg.org/dnkl/foot/issues/581).
 * Support for overriding configuration options on the command line
-  (https://codeberg.org/dnkl/foot/issues/554).
+  (https://codeberg.org/dnkl/foot/issues/554,
+  https://codeberg.org/dnkl/foot/issues/600).
 * `underline-offset` option to `foot.ini`
   (https://codeberg.org/dnkl/foot/issues/490).
 * `csd.button-color` option to `foot.ini`.

From 5fc8275eb2385933a1f068a4476ddfc28afce584 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= <daniel@ekloef.se>
Date: Wed, 23 Jun 2021 15:04:09 +0200
Subject: [PATCH 10/10] client: add and use function push_override()
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

We now track override data (length + malloc:ed string) in a struct,
and push this struct to our overrides linked list.

There’s a new function, push_override() that takes a string,
calculates its length, strdup() it and pushes it to the linked
list. This function also length-checks the string, to ensure we don’t
overflow.

This way, we don’t have to loop the overrides list twice; once when
calculating the total length of all overrides, and once when sending
the overrides to the server.

Now, we can update the total length as we add overrides (i.e while
parsing the command line, instead of afterwards).

This means we only have to loop the list once, and that’s when sending
it. This also means there’s no longer any need to malloc an array
holding the lengths of each override.
---
 client.c | 111 +++++++++++++++++++++++++++++++------------------------
 1 file changed, 63 insertions(+), 48 deletions(-)

diff --git a/client.c b/client.c
index 9c08a037..7249c588 100644
--- a/client.c
+++ b/client.c
@@ -25,6 +25,12 @@
 #include "version.h"
 #include "xmalloc.h"
 
+struct override {
+    size_t len;
+    char *str;
+};
+typedef tll(struct override) override_list_t;
+
 static volatile sig_atomic_t aborted = 0;
 
 static void
@@ -69,6 +75,21 @@ print_usage(const char *prog_name)
            "  -v,--version                            show the version number and quit\n");
 }
 
+static bool NOINLINE
+push_override(override_list_t *overrides, const char *s, uint64_t *total_len)
+{
+    size_t len = strlen(s) + 1;
+    if (len >= 1 << (8 * sizeof(struct client_string))) {
+        LOG_ERR("override length overflow");
+        return false;
+    }
+
+    struct override o = {len, xstrdup(s)};
+    tll_push_back(*overrides, o);
+    *total_len += sizeof(struct client_string) + o.len;
+    return true;
+}
+
 int
 main(int argc, char *const *argv)
 {
@@ -100,20 +121,23 @@ main(int argc, char *const *argv)
         {NULL,                 no_argument,       NULL,   0},
     };
 
-    tll(char *) overrides = tll_init();
-
     const char *custom_cwd = NULL;
     const char *server_socket_path = NULL;
     enum log_class log_level = LOG_CLASS_INFO;
     enum log_colorize log_colorize = LOG_COLORIZE_AUTO;
     bool hold = false;
+
+    /* Used to format overrides */
     bool no_wait = false;
 
     char buf[1024];
 
+    /* Total packet length, not (yet) including overrides or argv[] */
+    uint64_t total_len = 0;
+
     /* malloc:ed and needs to be in scope of all goto's */
     char *_cwd = NULL;
-    struct client_string *coverrides = NULL;
+    override_list_t overrides = tll_init();
     struct client_string *cargv = NULL;
 
     while (true) {
@@ -124,21 +148,25 @@ main(int argc, char *const *argv)
         switch (c) {
         case 't':
             snprintf(buf, sizeof(buf), "term=%s", optarg);
-            tll_push_back(overrides, xstrdup(buf));
+            if (!push_override(&overrides, buf, &total_len))
+                goto err;
             break;
 
         case 'T':
             snprintf(buf, sizeof(buf), "title=%s", optarg);
-            tll_push_back(overrides, xstrdup(buf));
+            if (!push_override(&overrides, buf, &total_len))
+                goto err;
             break;
 
         case 'a':
             snprintf(buf, sizeof(buf), "app-id=%s", optarg);
-            tll_push_back(overrides, xstrdup(buf));
+            if (!push_override(&overrides, buf, &total_len))
+                goto err;
             break;
 
         case 'L':
-            tll_push_back(overrides, xstrdup("login-shell=yes"));
+            if (!push_override(&overrides, "login-shell=yes", &total_len))
+                goto err;
             break;
 
         case 'D': {
@@ -155,10 +183,12 @@ main(int argc, char *const *argv)
             unsigned width, height;
             if (sscanf(optarg, "%ux%u", &width, &height) != 2 || width == 0 || height == 0) {
                 fprintf(stderr, "error: invalid window-size-pixels: %s\n", optarg);
-                return ret;
+                goto err;
             }
+
             snprintf(buf, sizeof(buf), "initial-window-size-pixels=%ux%u", width, height);
-            tll_push_back(overrides, xstrdup(buf));
+            if (!push_override(&overrides, buf, &total_len))
+                goto err;
             break;
         }
 
@@ -168,17 +198,21 @@ main(int argc, char *const *argv)
                 fprintf(stderr, "error: invalid window-size-chars: %s\n", optarg);
                 goto err;
             }
+
             snprintf(buf, sizeof(buf), "initial-window-size-chars=%ux%u", width, height);
-            tll_push_back(overrides, xstrdup(buf));
+            if (!push_override(&overrides, buf, &total_len))
+                goto err;
             break;
         }
 
         case 'm':
-            tll_push_back(overrides, xstrdup("initial-window-mode=maximized"));
+            if (!push_override(&overrides, "initial-window-mode=maximized", &total_len))
+                goto err;
             break;
 
         case 'F':
-            tll_push_back(overrides, xstrdup("initial-window-mode=fullscreen"));
+            if (!push_override(&overrides, "initial-window-mode=fullscreen", &total_len))
+                goto err;
             break;
 
         case 's':
@@ -194,7 +228,8 @@ main(int argc, char *const *argv)
             break;
 
         case 'o':
-            tll_push_back(overrides, xstrdup(optarg));
+            if (!push_override(&overrides, optarg, &total_len))
+                goto err;
             break;
 
         case 'd': {
@@ -313,30 +348,11 @@ main(int argc, char *const *argv)
         .argc = argc,
     };
 
-    /* Total packet length, not (yet) including overrides or argv[] */
-    uint64_t total_len = sizeof(data) + cwd_len;
-
-    /* Add overrides to total packet length */
-    coverrides = xmalloc(override_count * sizeof(coverrides[0]));
-    {
-        size_t i = 0;
-        tll_foreach(overrides, it) {
-            const size_t len = strlen(it->item) + 1;
-
-            if (len >= 1 << (8 * sizeof(coverrides[i].len))) {
-                LOG_ERR("override length overflow");
-                goto err;
-            }
-
-            coverrides[i].len = len;
-            total_len += sizeof(coverrides[i]) + coverrides[i].len;
-            i++;
-        }
-    }
-
-    cargv = xmalloc(argc * sizeof(cargv[0]));
+    /* Total packet length, not (yet) including argv[] */
+    total_len += sizeof(data) + cwd_len;
 
     /* Add argv[] size to total packet length */
+    cargv = xmalloc(argc * sizeof(cargv[0]));
     for (size_t i = 0; i < argc; i++) {
         const size_t arg_len = strlen(argv[i]) + 1;
 
@@ -369,17 +385,14 @@ main(int argc, char *const *argv)
     }
 
     /* Send overrides */
-    {
-        size_t i = 0;
-        tll_foreach(overrides, it) {
-            if (send(fd, &coverrides[i], sizeof(coverrides[i]), 0) != sizeof(coverrides[i]) ||
-                send(fd, it->item, coverrides[i].len, 0) != coverrides[i].len)
-            {
-                LOG_ERRNO("failed to send setup packet (overrides) to server");
-                goto err;
-            }
-
-            i++;
+    tll_foreach(overrides, it) {
+        const struct override *o = &it->item;
+        struct client_string s = {o->len};
+        if (send(fd, &s, sizeof(s), 0) != sizeof(s) ||
+            send(fd, o->str, o->len, 0) != o->len)
+        {
+            LOG_ERRNO("failed to send setup packet (overrides) to server");
+            goto err;
         }
     }
 
@@ -412,9 +425,11 @@ main(int argc, char *const *argv)
         ret = exit_code;
 
 err:
-    tll_free_and_free(overrides, free);
+    tll_foreach(overrides, it) {
+        free(it->item.str);
+        tll_remove(overrides, it);
+    }
     free(cargv);
-    free(coverrides);
     free(_cwd);
     if (fd != -1)
         close(fd);