fix: three crashes found while packaging (#54)
* fix: zero the settings structs so --help does not free garbage `monocoque --help` aborted with `free(): invalid pointer` on Debian forky -- found by installing the released debian-latest .deb on the distro it is built for. gdb put it in monocoquesettingsfree() called straight from main(). The early-exit paths (`--help`, `--version`, bad parameters) jump to cleanup_final before any field of Parameters or MonocoqueSettings is assigned, and cleanup_final calls monocoquesettingsfree() and freeparams(), which free every pointer in both. malloc leaves those pointing at whatever was in the heap block. It is undefined behaviour everywhere; forky's glibc is simply the first to check, which is why the same source looked healthy on Debian stable and both Fedoras. calloc in all three entry points -- monocoque.c, monocoque-cli.c (the `monocoque` binary) and monocoque-gui.c. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(sound): return from sounddev_init so shaker devices survive setup Only 2 of 24 configured sound devices came up -- "Could not initialize Sound Device" for the other 22 -- and no shaker fired, on a rig whose PipeWire graph looked perfect: monocoque streams connected into all six filter chains, every node running. sounddev_init() is declared int and fell off its end without returning, so new_sound_device() read whatever was left in the return register and treated that as the error code. The PulseAudio streams had already connected by then; the device objects were simply freed and never fed telemetry again, which is why the graph looked right and nothing shook. Undefined behaviour, so it varies by build: the same config drives shakers from a locally compiled monocoque and not from the packaged one. Now returns usb_generic_shaker_init()'s own result. Verified against a live Assetto Corsa session on real hardware: 26 devices initialised, no failures, shakers working. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(gui): stop gmonocoque segfaulting on a config without an api key gmonocoque died before drawing a window, in libconfig: #0 config_setting_get_member () from libconfig.so.9 #1 config_setting_lookup () #2 populate_device_list () find_default_config() required sim, api and car to all be present on an entry and skipped the entry if any lookup failed, so it returned -1; populate_device_list passed that -1 straight to config_setting_get_elem, got NULL back, and config_setting_lookup dereferenced it. No real config has all three. conf/monocoque.config -- the example this project's own README points at -- sets sim and car and never mentions api, so this crashed for anyone following the documentation. It stayed hidden in the Flatpak build only until that manifest was given access to the real ~/.config/monocoque; against an empty config there was nothing to look up. A key that isn't there no longer disqualifies a match, a -1 falls back to the first entry (which is what a single-entry config means anyway), and both NULL paths now return with a message instead of dereferencing. Verified by rebuilding the Flatpak and launching gmonocoque against the real config that crashed it: the window opens and stays up. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
committed by
GitHub
co-authored by
Claude Opus 5
parent
501d72fb1e
commit
cd67cb1689
@@ -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 };
|
||||
|
||||
@@ -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);
|
||||
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -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)
|
||||
|
||||
Reference in New Issue
Block a user