diff --git a/src/monocoque/devices/sounddevice.c b/src/monocoque/devices/sounddevice.c index 9210178..5deec6e 100644 --- a/src/monocoque/devices/sounddevice.c +++ b/src/monocoque/devices/sounddevice.c @@ -211,8 +211,16 @@ int sounddev_init(SoundDevice* sounddevice, const char* devname, SoundDeviceSett } - usb_generic_shaker_init(sounddevice, mainloop, context, devname, sds.volume, sds.pan, sds.channels, streamname); - //usb_generic_shaker_init(sounddevice); + // Returned, not discarded: this function is declared int and used to fall + // off its end, so new_sound_device() read whatever happened to be in the + // return register and treated most devices as failures -- "Could not + // initialize Sound Device" for 22 of 24 configured shakers, while their + // PulseAudio streams had in fact connected. The devices were freed and + // never fed telemetry, so the graph looked correct in qpwgraph and nothing + // shook. Being undefined behaviour it varied by build, which is why the + // same config worked against a locally compiled monocoque and not the + // packaged one. + return usb_generic_shaker_init(sounddevice, mainloop, context, devname, sds.volume, sds.pan, sds.channels, streamname); } static const vtable engine_sound_simdevice_vtable = { &sounddev_engine_update, &sounddev_free }; diff --git a/src/monocoque/mgui/uiconfighelper.c b/src/monocoque/mgui/uiconfighelper.c index b9481b8..4b484ab 100644 --- a/src/monocoque/mgui/uiconfighelper.c +++ b/src/monocoque/mgui/uiconfighelper.c @@ -21,6 +21,11 @@ typedef struct pa_devicelist { char description[256]; } pa_devicelist_t; +static int matches_any(const char *value) +{ + return strcmp(value, "default") == 0 || strcmp(value, "all") == 0; +} + int find_default_config(config_setting_t *configs) { int count = config_setting_length(configs); @@ -29,20 +34,20 @@ int find_default_config(config_setting_t *configs) { config_setting_t *entry = config_setting_get_elem(configs, i); - const char *sim; - const char *api; - const char *car; + // A key that isn't in the entry doesn't constrain the match. This used + // to require all three, which no real config has: conf/monocoque.config + // -- the example the README points at -- sets sim and car and never + // mentions api, so every entry was skipped, -1 came back, and the + // caller dereferenced the NULL that produced. + const char *sim = "default"; + const char *api = "default"; + const char *car = "default"; - if (!config_setting_lookup_string(entry, "sim", &sim)) - continue; - if (!config_setting_lookup_string(entry, "api", &api)) - continue; - if (!config_setting_lookup_string(entry, "car", &car)) - continue; + config_setting_lookup_string(entry, "sim", &sim); + config_setting_lookup_string(entry, "api", &api); + config_setting_lookup_string(entry, "car", &car); - if ((strcmp(sim, "default") == 0 || strcmp(sim, "all") == 0) && - (strcmp(api, "default") == 0 || strcmp(api, "all") == 0) && - (strcmp(car, "default") == 0 || strcmp(car, "all") == 0)) + if (matches_any(sim) && matches_any(api) && matches_any(car)) { return i; } @@ -137,11 +142,31 @@ void populate_device_list(ListBox *listbox, config_t* cfg) config = config_lookup(cfg, "configs"); int config_num = find_default_config(config); + if (config_num < 0) + { + // No entry claims to be the default one: list the first, which is what + // a single-entry config means anyway, rather than nothing. + config_num = 0; + } config_setting_t* selectedconfig = config_setting_get_elem(config, config_num); + if (selectedconfig == NULL) + { + fprintf(stderr, "No config entry to list devices for\n"); + return; + } + config_setting_t* config_devices = NULL; config_devices = config_setting_lookup(selectedconfig, "devices"); - + if (config_devices == NULL) + { + // config_setting_lookup dereferences its argument, so reaching here + // with a NULL selectedconfig used to segfault inside libconfig -- + // gmonocoque died on launch, before drawing a window, for anyone whose + // config didn't produce a match above. + fprintf(stderr, "Config entry has no devices section\n"); + return; + } count = config_setting_length(config_devices); diff --git a/src/monocoque/monocoque-cli.c b/src/monocoque/monocoque-cli.c index aadb1f4..797e9bf 100644 --- a/src/monocoque/monocoque-cli.c +++ b/src/monocoque/monocoque-cli.c @@ -88,8 +88,16 @@ int main(int argc, char** argv) return 0; } Parameters* p = NULL; - p = malloc(sizeof(Parameters)); - MonocoqueSettings* ms = malloc(sizeof(MonocoqueSettings));; + // calloc, not malloc: a --help or --version run jumps straight to + // cleanup_final before a single field is set, and the cleanup path frees + // every pointer in both structs. Uninitialised heap made that a free() of + // whatever junk was there -- confirmed on Debian forky, where the + // released .deb aborted with `free(): invalid pointer` in + // monocoquesettingsfree() on `monocoque --help`. Older glibc happened not + // to notice, which is the only reason this looked fine on stable and + // Fedora. + p = calloc(1, sizeof(Parameters)); + MonocoqueSettings* ms = calloc(1, sizeof(MonocoqueSettings)); ConfigError ppe = getParameters(argc, argv, p); if (ppe == E_SUCCESS_AND_EXIT || ppe == E_SOMETHING_BAD) diff --git a/src/monocoque/monocoque-gui.c b/src/monocoque/monocoque-gui.c index a2d5769..29949a8 100644 --- a/src/monocoque/monocoque-gui.c +++ b/src/monocoque/monocoque-gui.c @@ -91,7 +91,10 @@ int monocoque_initialize(int argc, char** argv) } p = NULL; - p = malloc(sizeof(Parameters)); + // calloc, not malloc: freeparams() walks every pointer in this struct + // on the early-exit paths, before getParameters has set them. See the + // same change in monocoque-cli.c. + p = calloc(1, sizeof(Parameters)); p->config_dirpath = NULL; p->config_filepath = NULL; p->log_filename_str = NULL; diff --git a/src/monocoque/monocoque.c b/src/monocoque/monocoque.c index b75447c..e27bfef 100644 --- a/src/monocoque/monocoque.c +++ b/src/monocoque/monocoque.c @@ -84,8 +84,16 @@ int main(int argc, char** argv) return 0; } Parameters* p = NULL; - p = malloc(sizeof(Parameters)); - MonocoqueSettings* ms = malloc(sizeof(MonocoqueSettings));; + // calloc, not malloc: a --help or --version run jumps straight to + // cleanup_final before a single field is set, and the cleanup path frees + // every pointer in both structs. Uninitialised heap made that a free() of + // whatever junk was there -- confirmed on Debian forky, where the + // released .deb aborted with `free(): invalid pointer` in + // monocoquesettingsfree() on `monocoque --help`. Older glibc happened not + // to notice, which is the only reason this looked fine on stable and + // Fedora. + p = calloc(1, sizeof(Parameters)); + MonocoqueSettings* ms = calloc(1, sizeof(MonocoqueSettings)); ConfigError ppe = getParameters(argc, argv, p); if (ppe == E_SUCCESS_AND_EXIT || ppe == E_SOMETHING_BAD)