src: don't crash on incomplete user-edited config file

If a user created a custom config file and we added new fields in a
newer version, `eg25-manager` will crash by assuming all config files
are complete and up-to-date.

Moreover, creating a custom config, even to change only one option,
required copying a complete default config file and then editing the
relevant field(s). This could lead to issues when upgrading, as default
values of fields unrelated to the user change could be modified and not
applied to the user config.

This patch addresses both these issues by:
* making sure at least one config file exists
* requiring only the "default" config file (the one under `/usr/share`)
  to include all the required fields
* trying to use user config for each field, falling back to the default
  config file if the field isn't present in the user config
* exiting with a meaningful error message in case the default config
  file is missing a required field or section

That way, it will be possible to have a minimal user config file
containing only the field(s) needing a different value than the default
one, falling back to the values in the default config file.

Fixes #23

src/at.c

@@ -5,6 +5,7 @@
  */
 
 #include "at.h"
+#include "config.h"
 #include "suspend.h"
 #include "gpio.h"
 #include "gnss.h"
@@ -307,67 +308,61 @@ static void parse_commands_list(toml_array_t *array, GArray **cmds)
             continue;
 
         value = toml_string_in(table, "cmd");
-        if (value.ok) {
-            cmd->cmd = g_strdup(value.u.s);
-            free(value.u.s);
-        }
+        if (value.ok)
+            cmd->cmd = value.u.s;
 
         value = toml_string_in(table, "subcmd");
-        if (value.ok) {
-            cmd->subcmd = g_strdup(value.u.s);
-            free(value.u.s);
-        }
+        if (value.ok)
+            cmd->subcmd = value.u.s;
 
         value = toml_string_in(table, "value");
-        if (value.ok) {
-            cmd->value = g_strdup(value.u.s);
-            free(value.u.s);
-        }
+        if (value.ok)
+            cmd->value = value.u.s;
 
         value = toml_string_in(table, "expect");
-        if (value.ok) {
-            cmd->expected = g_strdup(value.u.s);
-            free(value.u.s);
-        }
+        if (value.ok)
+            cmd->expected = value.u.s;
     }
 }
 
-int at_init(struct EG25Manager *manager, toml_table_t *config)
+int at_init(struct EG25Manager *manager, toml_table_t *config[])
 {
-    toml_array_t *commands;
-    toml_datum_t uart_port;
+    toml_array_t *commands = NULL;
+    gchar *uart_port = NULL;
+    toml_table_t *at_config[EG25_CONFIG_COUNT];
 
-    uart_port = toml_string_in(config, "uart");
-    if (!uart_port.ok)
+    for (int i = 0; i < EG25_CONFIG_COUNT; i++)
+        at_config[i] = config[i] ? toml_table_in(config[i], "at") : NULL;
+
+    if (!at_config[EG25_CONFIG_SYS])
+        g_error("Default config file lacks the 'at' section!");
+
+    if (!config_get_string(at_config, "uart", &uart_port))
         g_error("Configuration file lacks UART port definition");
 
-    manager->at_fd = configure_serial(uart_port.u.s);
+    manager->at_fd = configure_serial(uart_port);
     if (manager->at_fd < 0) {
-        g_critical("Unable to configure %s", uart_port.u.s);
-        free(uart_port.u.s);
+        g_critical("Unable to configure %s", uart_port);
+        g_free(uart_port);
         return 1;
     }
-    free(uart_port.u.s);
+    g_free(uart_port);
 
     manager->at_source = g_unix_fd_add(manager->at_fd, G_IO_IN, modem_response, manager);
 
-    commands = toml_array_in(config, "configure");
-    if (!commands)
+    if (!config_get_array(at_config, "configure", &commands))
         g_error("Configuration file lacks initial AT commands list");
     parse_commands_list(commands, &configure_commands);
 
-    commands = toml_array_in(config, "suspend");
-    if (!commands)
+    if (!config_get_array(at_config, "suspend", &commands))
         g_error("Configuration file lacks suspend AT commands list");
     parse_commands_list(commands, &suspend_commands);
 
-    commands = toml_array_in(config, "resume");
-    if (!commands)
+    if (!config_get_array(at_config, "resume", &commands))
         g_error("Configuration file lacks resume AT commands list");
     parse_commands_list(commands, &resume_commands);
 
-    commands = toml_array_in(config, "reset");
-    if (!commands)
+    if (!config_get_array(at_config, "reset", &commands))
         g_error("Configuration file lacks reset AT commands list");
     parse_commands_list(commands, &reset_commands);
 

src/at.h

@@ -17,7 +17,7 @@ typedef struct AtCommand {
     int retries;
 } AtCommand;
 
-int at_init(struct EG25Manager *manager, toml_table_t *config);
+int at_init(struct EG25Manager *manager, toml_table_t *config[]);
 void at_destroy(struct EG25Manager *manager);
 
 void at_process_result(struct EG25Manager *manager,

src/gnss.c

@@ -4,6 +4,7 @@
  * SPDX-License-Identifier: GPL-3.0-or-later
  */
 
+#include "config.h"
 #include "gnss.h"
 #include "manager.h"
 #include "at.h"
@@ -58,38 +59,34 @@ gboolean gnss_upload_assistance_data(struct EG25Manager *manager)
     return FALSE;
 }
 
-void gnss_init(struct EG25Manager *manager, toml_table_t *config)
+void gnss_init(struct EG25Manager *manager, toml_table_t *config[])
 {
-    toml_datum_t enabled;
-    toml_datum_t url;
-    toml_datum_t file;
+    toml_table_t *gnss_config[EG25_CONFIG_COUNT];
     g_autoptr (GError) error = NULL;
 
+    for (int i = 0; i < EG25_CONFIG_COUNT; i++)
+        gnss_config[i] = config[i] ? toml_table_in(config[i], "gnss") : NULL;
+
+    if (!gnss_config[EG25_CONFIG_SYS])
+        g_error("Default config file lacks the 'gnss' section!");
+
     /*
      * GNSS assistance is an optional feature, you can disable it
      * if you want in the configuration file.
      * In case the configuration is missing, we assume GNSS assistance
      * to be disabled.
      */
-    enabled = toml_bool_in(config, "enabled");
-    manager->gnss_assistance_enabled = FALSE;
-    if (enabled.ok)
-        manager->gnss_assistance_enabled = enabled.u.b;
+    config_get_bool(gnss_config, "enabled", &manager->gnss_assistance_enabled);
 
     if (!manager->gnss_assistance_enabled) {
         g_message("GNSS assistance is disabled!");
         return;
     }
 
-    url = toml_string_in(config, "url");
-    if (url.ok)
-        manager->gnss_assistance_url = url.u.s;
-    else
+    if (!config_get_string(gnss_config, "url", &manager->gnss_assistance_url))
         g_error("GNSS assistance server URL is missing from config file");
-    file = toml_string_in(config, "file");
-    if (file.ok)
-        manager->gnss_assistance_file = file.u.s;
-    else
+
+    if (!config_get_string(gnss_config, "file", &manager->gnss_assistance_file))
         g_error("GNSS assistance file name is missing from config file");
 
     /* Create temporary file to store assistance data */
@@ -105,6 +102,8 @@ void gnss_init(struct EG25Manager *manager, toml_table_t *config)
 
 void gnss_destroy(struct EG25Manager *manager)
 {
+    g_free(manager->gnss_assistance_url);
+    g_free(manager->gnss_assistance_file);
     close(manager->gnss_assistance_fd);
 }
 
@@ -512,4 +511,3 @@ void gnss_step(struct EG25Manager *manager)
         break;
     }
 }
-

src/gnss.h

@@ -12,6 +12,6 @@
 
 #include "manager.h"
 
-void gnss_init(struct EG25Manager *manager, toml_table_t *config);
+void gnss_init(struct EG25Manager *manager, toml_table_t *config[]);
 void gnss_destroy(struct EG25Manager *manager);
 gboolean gnss_upload_assistance_data(struct EG25Manager *manager);

src/gpio.c

@@ -4,6 +4,7 @@
  * SPDX-License-Identifier: GPL-3.0-or-later
  */
 
+#include "config.h"
 #include "gpio.h"
 
 #include <unistd.h>
@@ -13,8 +14,6 @@
 
 #define MAX_GPIOCHIP_LINES 352
 
-#define GPIO_IDX_INVAL 0xffff
-
 enum {
     GPIO_OUT_DTR = 0,
     GPIO_OUT_PWRKEY,
@@ -24,12 +23,23 @@ enum {
     GPIO_OUT_COUNT
 };
 
-
 enum {
     GPIO_IN_STATUS = 0,
     GPIO_IN_COUNT
 };
 
+static char *gpio_out_names[] = {
+    "dtr",
+    "pwrkey",
+    "reset",
+    "apready",
+    "disable",
+};
+
+static char *gpio_in_names[] = {
+    "status",
+};
+
 int gpio_sequence_poweron(struct EG25Manager *manager)
 {
     gpiod_line_set_value(manager->gpio_out[GPIO_OUT_PWRKEY], 1);
@@ -89,24 +99,17 @@ int gpio_sequence_sleep(struct EG25Manager *manager)
     return 0;
 }
 
-static guint get_config_gpio(toml_table_t *config, const char *id)
-{
-    toml_datum_t value = toml_int_in(config, id);
-    guint gpio;
-
-    if (!value.ok)
-        return GPIO_IDX_INVAL;
-
-    gpio = (guint)value.u.i;
-
-    return gpio;
-}
-
-int gpio_init(struct EG25Manager *manager, toml_table_t *config)
+int gpio_init(struct EG25Manager *manager, toml_table_t *config[])
 {
     int i, ret;
-    guint gpio_out_idx[GPIO_OUT_COUNT];
-    guint gpio_in_idx[GPIO_IN_COUNT];
+    guint offset, chipidx, gpio_idx;
+    toml_table_t *gpio_config[EG25_CONFIG_COUNT];
+
+    for (i = 0; i < EG25_CONFIG_COUNT; i++)
+        gpio_config[i] = config[i] ? toml_table_in(config[i], "gpio") : NULL;
+
+    if (!gpio_config[EG25_CONFIG_SYS])
+        g_error("Default config file lacks the 'gpio' section!");
 
     manager->gpiochip[0] = gpiod_chip_open_by_label(GPIO_CHIP1_LABEL);
     if (!manager->gpiochip[0]) {
@@ -120,21 +123,15 @@ int gpio_init(struct EG25Manager *manager, toml_table_t *config)
         return 1;
     }
 
-    gpio_out_idx[GPIO_OUT_DTR] = get_config_gpio(config, "dtr");
-    gpio_out_idx[GPIO_OUT_PWRKEY] = get_config_gpio(config, "pwrkey");
-    gpio_out_idx[GPIO_OUT_RESET] = get_config_gpio(config, "reset");
-    gpio_out_idx[GPIO_OUT_APREADY] = get_config_gpio(config, "apready");
-    gpio_out_idx[GPIO_OUT_DISABLE] = get_config_gpio(config, "disable");
-    gpio_in_idx[GPIO_IN_STATUS] = get_config_gpio(config, "status");
-
     for (i = 0; i < GPIO_OUT_COUNT; i++) {
-        guint offset, chipidx;
+        if (!config_get_uint(gpio_config, gpio_out_names[i], &gpio_idx))
+            g_error("Unable to get config for output GPIO '%s'", gpio_out_names[i]);
 
-        if (gpio_out_idx[i] < MAX_GPIOCHIP_LINES) {
-            offset = gpio_out_idx[i];
+        if (gpio_idx < MAX_GPIOCHIP_LINES) {
+            offset = gpio_idx;
             chipidx = 0;
         } else {
-            offset = gpio_out_idx[i] - MAX_GPIOCHIP_LINES;
+            offset = gpio_idx - MAX_GPIOCHIP_LINES;
             chipidx = 1;
         }
 
@@ -152,16 +149,14 @@ int gpio_init(struct EG25Manager *manager, toml_table_t *config)
     }
 
     for (i = 0; i < GPIO_IN_COUNT; i++) {
-        guint offset, chipidx;
-
-        if (gpio_in_idx[i] == GPIO_IDX_INVAL)
+        if (!config_get_uint(gpio_config, gpio_in_names[i], &gpio_idx))
             continue;
 
-        if (gpio_in_idx[i] < MAX_GPIOCHIP_LINES) {
-            offset = gpio_in_idx[i];
+        if (gpio_idx < MAX_GPIOCHIP_LINES) {
+            offset = gpio_idx;
             chipidx = 0;
         } else {
-            offset = gpio_in_idx[i] - MAX_GPIOCHIP_LINES;
+            offset = gpio_idx - MAX_GPIOCHIP_LINES;
             chipidx = 1;
         }
 

src/gpio.h

@@ -8,7 +8,7 @@
 
 #include "manager.h"
 
-int gpio_init(struct EG25Manager *state, toml_table_t *config);
+int gpio_init(struct EG25Manager *state, toml_table_t *config[]);
 void gpio_destroy(struct EG25Manager *state);
 
 int gpio_sequence_poweron(struct EG25Manager *state);

src/manager.c

@@ -5,6 +5,7 @@
  */
 
 #include "at.h"
+#include "config.h"
 #include "gpio.h"
 #include "manager.h"
 
@@ -33,6 +34,8 @@
 #define EG25_DATADIR "/usr/share/eg25-manager"
 #endif
 
+#define POWERON_DELAY_US 100000UL
+
 static gboolean quit_app(struct EG25Manager *manager)
 {
     int i;
@@ -225,7 +228,7 @@ void modem_resume_post(struct EG25Manager *manager)
     at_sequence_resume(manager);
 }
 
-static toml_table_t *parse_config_file(char *config_file)
+static toml_table_t *parse_config_file(char *config_file, gboolean force_default)
 {
     toml_table_t *toml_config;
     gchar *compatible;
@@ -249,28 +252,26 @@ static toml_table_t *parse_config_file(char *config_file)
         } while (pos < len);
 
         for (pos = 0; pos < compat->len; pos++) {
-            g_autofree gchar *filename = g_strdup_printf(EG25_CONFDIR "/%s.toml", (gchar *)g_ptr_array_index(compat, pos));
+            g_autofree gchar *filename = NULL;
+            if (force_default)
+                filename = g_strdup_printf(EG25_DATADIR "/%s.toml", (gchar *)g_ptr_array_index(compat, pos));
+            else
+                filename = g_strdup_printf(EG25_CONFDIR "/%s.toml", (gchar *)g_ptr_array_index(compat, pos));
+
             if (access(filename, F_OK) == 0) {
                 g_message("Opening config file: %s", filename);
                 f = fopen(filename, "r");
                 break;
             }
         }
-
-        if (!f) {
-            for (pos = 0; pos < compat->len; pos++) {
-                g_autofree gchar *filename = g_strdup_printf(EG25_DATADIR "/%s.toml", (gchar *)g_ptr_array_index(compat, pos));
-                if (access(filename, F_OK) == 0) {
-                    g_message("Opening config file: %s", filename);
-                    f = fopen(filename, "r");
-                    break;
-                }
-            }
-        }
     }
 
-    if (!f)
-        g_error("unable to find a suitable config file!");
+    if (!f) {
+        if (force_default)
+            g_error("unable to find a suitable config file!");
+        else
+            return NULL;
+    }
 
     toml_config = toml_parse_file(f, error, sizeof(error));
     if (!toml_config)
@@ -285,9 +286,8 @@ int main(int argc, char *argv[])
     g_autoptr(GError) err = NULL;
     struct EG25Manager manager;
     gchar *config_file = NULL;
-    toml_table_t *toml_config;
-    toml_table_t *toml_manager;
-    toml_datum_t toml_value;
+    toml_table_t *toml_config[EG25_CONFIG_COUNT];
+    toml_table_t *manager_config[EG25_CONFIG_COUNT];
     const GOptionEntry options[] = {
         { "config", 'c', 0, G_OPTION_ARG_STRING, &config_file, "Config file to use.", NULL },
         { NULL, 0, 0, G_OPTION_ARG_NONE, NULL, NULL, NULL }
@@ -295,6 +295,7 @@ int main(int argc, char *argv[])
 
     memset(&manager, 0, sizeof(manager));
     manager.at_fd = -1;
+    manager.poweron_delay = POWERON_DELAY_US;
     manager.suspend_delay_fd = -1;
     manager.suspend_block_fd = -1;
 
@@ -307,43 +308,46 @@ int main(int argc, char *argv[])
 
     manager.loop = g_main_loop_new(NULL, FALSE);
 
-    toml_config = parse_config_file(config_file);
+    toml_config[EG25_CONFIG_SYS] = parse_config_file(NULL, TRUE);
+    toml_config[EG25_CONFIG_USER] = parse_config_file(config_file, FALSE);
 
-    toml_manager = toml_table_in(toml_config, "manager");
-    if (toml_manager) {
-        toml_value = toml_bool_in(toml_manager, "need_libusb");
-        if (toml_value.ok)
-            manager.use_libusb = toml_value.u.b;
-
-        toml_value = toml_int_in(toml_manager, "usb_vid");
-        if (toml_value.ok)
-            manager.usb_vid = toml_value.u.i;
-
-        toml_value = toml_int_in(toml_manager, "usb_pid");
-        if (toml_value.ok)
-            manager.usb_pid = toml_value.u.i;
-
-        toml_value = toml_int_in(toml_manager, "poweron_delay");
-        if (toml_value.ok) {
-            if (toml_value.u.i >= 0 && toml_value.u.i <= G_MAXULONG) {
-                // Safe to cast into gulong
-                manager.poweron_delay = (gulong) toml_value.u.i;
-            } else {
-                // Changed from initialized default value but not in range
-                g_message("Configured poweron_delay out of range, using default");
-            }
-        }
+    /*
+     * We need at least one valid config file, and assuming it's
+     * EG25_CONFIG_SYS will make the rest easier to implement
+     */
+    if (!toml_config[EG25_CONFIG_SYS] && toml_config[EG25_CONFIG_USER]) {
+        toml_config[EG25_CONFIG_SYS] = toml_config[EG25_CONFIG_USER];
+        toml_config[EG25_CONFIG_USER] = NULL;
     }
 
-    at_init(&manager, toml_table_in(toml_config, "at"));
-    gpio_init(&manager, toml_table_in(toml_config, "gpio"));
+    if (!toml_config[EG25_CONFIG_SYS])
+        g_error("Unable to parse config file!");
+
+    for (int i = 0; i < EG25_CONFIG_COUNT; i++)
+        manager_config[i] = toml_config[i] ? toml_table_in(toml_config[i], "manager") : NULL;
+
+    if (!manager_config[EG25_CONFIG_SYS])
+        g_error("Default config file lacks the 'manager' section!");
+
+    config_get_bool(manager_config, "need_libusb", &manager.use_libusb);
+    config_get_uint(manager_config, "usb_vid", &manager.usb_vid);
+    config_get_uint(manager_config, "usb_pid", &manager.usb_pid);
+    config_get_uint(manager_config, "poweron_delay", &manager.poweron_delay);
+
+    at_init(&manager, toml_config);
+    gpio_init(&manager, toml_config);
 #ifdef HAVE_MMGLIB
-    mm_iface_init(&manager, toml_table_in(toml_config, "mm-iface"));
+    mm_iface_init(&manager, toml_config);
 #endif
-    ofono_iface_init(&manager);
-    suspend_init(&manager, toml_table_in(toml_config, "suspend"));
-    udev_init(&manager, toml_table_in(toml_config, "udev"));
-    gnss_init(&manager, toml_table_in(toml_config, "gnss"));
+    ofono_iface_init(&manager, toml_config);
+    suspend_init(&manager, toml_config);
+    udev_init(&manager, toml_config);
+    gnss_init(&manager, toml_config);
+
+    for (int i = 0; i < EG25_CONFIG_COUNT; i++) {
+        if (toml_config[i])
+            toml_free(toml_config[i]);
+    }
 
     g_idle_add(G_SOURCE_FUNC(modem_start), &manager);
 

src/manager.h

@@ -74,7 +74,7 @@ struct EG25Manager {
     gboolean use_libusb;
     guint usb_vid;
     guint usb_pid;
-    gulong poweron_delay;
+    guint poweron_delay;
 
     int at_fd;
     guint at_source;

src/mm-iface.c

@@ -209,7 +209,7 @@ static void mm_vanished_cb(GDBusConnection    *connection,
     mm_iface_clean(manager);
 }
 
-void mm_iface_init(struct EG25Manager *manager, toml_table_t *config)
+void mm_iface_init(struct EG25Manager *manager, toml_table_t *config[])
 {
     manager->mm_watch = g_bus_watch_name(G_BUS_TYPE_SYSTEM, MM_DBUS_SERVICE,
                                          G_BUS_NAME_WATCHER_FLAGS_AUTO_START,

src/mm-iface.h

@@ -8,5 +8,5 @@
 
 #include "manager.h"
 
-void mm_iface_init(struct EG25Manager *data, toml_table_t *config);
+void mm_iface_init(struct EG25Manager *data, toml_table_t *config[]);
 void mm_iface_destroy(struct EG25Manager *data);

src/ofono-iface.c

@@ -128,7 +128,7 @@ static void ofono_vanished_cb(GDBusConnection    *connection,
     }
 }
 
-void ofono_iface_init(struct EG25Manager *manager)
+void ofono_iface_init(struct EG25Manager *manager, toml_table_t *config[])
 {
     manager->ofono_watch = g_bus_watch_name(G_BUS_TYPE_SYSTEM, OFONO_SERVICE,
                                             G_BUS_NAME_WATCHER_FLAGS_AUTO_START,

src/ofono-iface.h

@@ -8,5 +8,5 @@
 
 #include "manager.h"
 
-void ofono_iface_init(struct EG25Manager *data);
+void ofono_iface_init(struct EG25Manager *data, toml_table_t *config[]);
 void ofono_iface_destroy(struct EG25Manager *data);

src/suspend.c

@@ -9,6 +9,7 @@
  * SPDX-License-Identifier: GPL-3.0-or-later
  */
 
+#include "config.h"
 #include "manager.h"
 
 #include <gio/gunixfdlist.h>
@@ -238,18 +239,26 @@ static void on_proxy_acquired(GObject *object,
     }
 }
 
-void suspend_init(struct EG25Manager *manager, toml_table_t *config)
+void suspend_init(struct EG25Manager *manager, toml_table_t *config[])
 {
-    toml_datum_t timeout_value;
+    toml_table_t *suspend_config[EG25_CONFIG_COUNT];
 
-    if (config) {
-        timeout_value = toml_int_in(config, "boot_timeout");
-        if (timeout_value.ok)
-            manager->modem_boot_timeout = (guint)timeout_value.u.i;
+    for (int i = 0; i < EG25_CONFIG_COUNT; i++)
+        suspend_config[i] = config[i] ? toml_table_in(config[i], "suspend") : NULL;
 
-        timeout_value = toml_int_in(config, "recovery_timeout");
-        if (timeout_value.ok)
-            manager->modem_recovery_timeout = (guint)timeout_value.u.i;
+    /* 
+     * The `suspend` section is optional in both the user and system config files,
+     * so let's make sure suspend_config[EG25_CONFIG_SYS] is valid if one of the
+     * files has it.
+     */
+    if (suspend_config[EG25_CONFIG_USER] && !suspend_config[EG25_CONFIG_SYS]) {
+        suspend_config[EG25_CONFIG_SYS] = suspend_config[EG25_CONFIG_USER];
+        suspend_config[EG25_CONFIG_USER] = NULL;
+    }
+
+    if (suspend_config[EG25_CONFIG_SYS]) {
+        config_get_uint(suspend_config, "boot_timeout", &manager->modem_boot_timeout);
+        config_get_uint(suspend_config, "recovery_timeout", &manager->modem_recovery_timeout);
     }
 
     if (manager->modem_boot_timeout == 0)

src/suspend.h

@@ -8,7 +8,7 @@
 
 #include "manager.h"
 
-void suspend_init (struct EG25Manager *data, toml_table_t *config);
+void suspend_init (struct EG25Manager *data, toml_table_t *config[]);
 void suspend_destroy (struct EG25Manager *data);
 
 void suspend_inhibit (struct EG25Manager *data, gboolean inhibit, gboolean block);

src/udev.c

@@ -25,7 +25,7 @@ static void udev_event_cb(GUdevClient *client, gchar *action, GUdevDevice *devic
     }
 }
 
-void udev_init (struct EG25Manager *manager, toml_table_t *config)
+void udev_init (struct EG25Manager *manager, toml_table_t *config[])
 {
     const char * const subsystems[] = { "usb", NULL };
 

src/udev.h

@@ -8,5 +8,5 @@
 
 #include "manager.h"
 
-void udev_init (struct EG25Manager *data, toml_table_t *config);
+void udev_init (struct EG25Manager *data, toml_table_t *config[]);
 void udev_destroy (struct EG25Manager *data);