mirror of
https://github.com/libretro/RetroArch.git
synced 2026-11-09 03:29:43 -10:00
menu/materialui: fix leak + null-deref + needless strdup in update_savestate_thumbnail_path
The function had three problems stacked in a handful of lines:
1. Null-deref: mui->savestate_thumbnail_file_path was dereferenced
(via strdup) BEFORE the 'if (!mui) return;' guard. If data was
ever NULL, the guard would never be reached.
2. Leak: the strdup result was assigned to a const char * and never
freed on any exit path - not when savestate_thumbnail was
disabled, not when the entry label check failed, not on the
success path either. Called frequently (on every selection
change in the savestate list) so the leak was quickly user-
visible.
3. Needless strdup: savestate_thumbnail_file_path is a fixed-size
char[PATH_MAX_LENGTH] embedded in the struct, not a pointer.
Heap allocation is unnecessary; a stack buffer of the same size
does the job and sidesteps both the leak and the OOM failure
mode (where strdup returning NULL silently flipped the
string_is_equal comparison below).
Fix: declare a local 'old_path[PATH_MAX_LENGTH]' AFTER the NULL
guard, strlcpy the current path into it before the field is
cleared, and compare against it further down. No heap allocation,
no leak, no dereference before NULL check.
Thread-safety: unchanged. Menu driver callbacks run on the main
thread; materialui_handle_t state is not shared across threads.
The only writes touch mui->savestate_thumbnail_file_path and
mui->thumbnails.savestate, both already single-thread owned.
This commit is contained in:
1 parent
91eba66b26
commit
93449d3ecc
1 file changed
+13
-3
@@ -2484,12 +2484,22 @@ static void materialui_update_savestate_thumbnail_path(void *data, unsigned i)
|
||||
{
|
||||
settings_t *settings = config_get_ptr();
|
||||
materialui_handle_t *mui = (materialui_handle_t*)data;
|
||||
bool savestate_thumbnail = settings->bools.savestate_thumbnail_enable;
|
||||
const char *current_path = strdup(mui->savestate_thumbnail_file_path);
|
||||
bool savestate_thumbnail;
|
||||
char old_path[PATH_MAX_LENGTH];
|
||||
|
||||
if (!mui)
|
||||
return;
|
||||
|
||||
savestate_thumbnail = settings->bools.savestate_thumbnail_enable;
|
||||
|
||||
/* Snapshot the current path before clearing it, so we can detect
|
||||
* whether the resolved path actually changed and only then reset
|
||||
* the thumbnail. A stack buffer is sufficient (the field it
|
||||
* mirrors is itself a fixed-size char array) and avoids the leak
|
||||
* + potential NULL-deref of the previous strdup-before-NULL-check
|
||||
* construction. */
|
||||
strlcpy(old_path, mui->savestate_thumbnail_file_path, sizeof(old_path));
|
||||
|
||||
mui->savestate_thumbnail_file_path[0] = '\0';
|
||||
|
||||
if (savestate_thumbnail)
|
||||
@@ -2525,7 +2535,7 @@ static void materialui_update_savestate_thumbnail_path(void *data, unsigned i)
|
||||
strlcpy(mui->savestate_thumbnail_file_path, path,
|
||||
sizeof(mui->savestate_thumbnail_file_path));
|
||||
|
||||
if (!string_is_equal(current_path, mui->savestate_thumbnail_file_path))
|
||||
if (!string_is_equal(old_path, mui->savestate_thumbnail_file_path))
|
||||
gfx_thumbnail_reset(&mui->thumbnails.savestate);
|
||||
|
||||
materialui_update_fullscreen_thumbnail_label(mui);
|
||||
|
||||
Reference in new issue
Block a user