diff --git a/libretro-common/include/lists/file_list.h b/libretro-common/include/lists/file_list.h index 95b0eb5d2c..67d3875fc9 100644 --- a/libretro-common/include/lists/file_list.h +++ b/libretro-common/include/lists/file_list.h @@ -59,6 +59,16 @@ void *file_list_get_userdata_at_offset(const file_list_t *list, void *file_list_get_actiondata_at_offset(const file_list_t *list, size_t index); +/** + * @brief frees the list + * + * NOTE: This function will also free() the entries actiondata + * and userdata fields if they are non-null. If you store complex + * or non-contiguous data there, make sure you free it's fields + * before calling this function or you might get a memory leak. + * + * @param list + */ void file_list_free(file_list_t *list); bool file_list_append(file_list_t *userdata, const char *path, diff --git a/menu/drivers/xmb.c b/menu/drivers/xmb.c index 6b59b5d009..477d49abfe 100755 --- a/menu/drivers/xmb.c +++ b/menu/drivers/xmb.c @@ -448,21 +448,38 @@ static void xmb_free_node(xmb_node_t *node) free(node); } -/* NOTE: This is faster than memcpy()ing xmb_node_t in most cases - * because most nodes have small (less than 200 bytes) fullpath */ -static xmb_node_t *xmb_copy_node(void *p) +/** + * @brief frees all xmb_node_t in a file_list_t + * + * file_list_t asumes userdata holds a simple structure and + * free()'s it. Can't change this at the time because other + * code depends on this behavior. + * + * @param list + * @param actiondata whether to free actiondata too + */ +static void xmb_free_list_nodes(file_list_t *list, bool actiondata) +{ + unsigned i, size = file_list_get_size(list); + + for (i = 0; i < size; ++i) + { + xmb_free_node((xmb_node_t*)file_list_get_userdata_at_offset(list, i)); + + /* file_list_set_userdata() doesn't accept NULL */ + list->list[i].userdata = NULL; + + if (actiondata) + file_list_free_actiondata(list, i); + } +} + +static xmb_node_t *xmb_copy_node(const xmb_node_t *old_node) { - xmb_node_t *old_node = (xmb_node_t*)p; xmb_node_t *new_node = (xmb_node_t*)malloc(sizeof(*new_node)); - new_node->alpha = old_node->alpha; - new_node->label_alpha = old_node->label_alpha; - new_node->zoom = old_node->zoom; - new_node->x = old_node->x; - new_node->y = old_node->y; - new_node->icon = old_node->icon; - new_node->content_icon = old_node->content_icon; - new_node->fullpath = old_node->fullpath ? strdup(old_node->fullpath) : NULL; + *new_node = *old_node; + new_node->fullpath = old_node->fullpath ? strdup(old_node->fullpath) : NULL; return new_node; } @@ -1969,7 +1986,10 @@ static void xmb_refresh_horizontal_list(xmb_handle_t *xmb) { xmb_context_destroy_horizontal_list(xmb); if (xmb->horizontal_list) + { + xmb_free_list_nodes(xmb->horizontal_list, false); file_list_free(xmb->horizontal_list); + } xmb->horizontal_list = NULL; menu_driver_ctl(RARCH_MENU_CTL_SET_PREVENT_POPULATE, NULL); @@ -3490,7 +3510,10 @@ error: free(xmb->selection_buf_old); xmb->selection_buf_old = NULL; if (xmb->horizontal_list) + { + xmb_free_list_nodes(xmb->horizontal_list, false); file_list_free(xmb->horizontal_list); + } xmb->horizontal_list = NULL; } return NULL; @@ -3503,22 +3526,32 @@ static void xmb_free(void *data) if (xmb) { if (xmb->menu_stack_old) + { + xmb_free_list_nodes(xmb->menu_stack_old, false); file_list_free(xmb->menu_stack_old); - xmb->menu_stack_old = NULL; + } if (xmb->selection_buf_old) + { + xmb_free_list_nodes(xmb->selection_buf_old, false); file_list_free(xmb->selection_buf_old); - xmb->selection_buf_old = NULL; + } + if (xmb->horizontal_list) + { + xmb_free_list_nodes(xmb->horizontal_list, false); file_list_free(xmb->horizontal_list); - xmb->horizontal_list = NULL; + } + + xmb->menu_stack_old = NULL; + xmb->selection_buf_old = NULL; + xmb->horizontal_list = NULL; video_coord_array_free(&xmb->raster_block.carr); video_coord_array_free(&xmb->raster_block2.carr); } font_driver_bind_block(NULL, NULL); - } static void xmb_context_bg_destroy(xmb_handle_t *xmb) @@ -3890,17 +3923,7 @@ static void xmb_list_clear(file_list_t *list) menu_animation_ctl(MENU_ANIMATION_CTL_KILL_BY_TAG, &tag); - for (i = 0; i < size; ++i) - { - xmb_node_t *node = (xmb_node_t*) - menu_entries_get_userdata_at_offset(list, i); - - if (!node) - continue; - - xmb_free_node(node); - list->list[i].userdata = NULL; - } + xmb_free_list_nodes(list, false); } static void xmb_list_deep_copy(const file_list_t *src, file_list_t *dst) @@ -3911,15 +3934,8 @@ static void xmb_list_deep_copy(const file_list_t *src, file_list_t *dst) menu_animation_ctl(MENU_ANIMATION_CTL_KILL_BY_TAG, &tag); - for (i = 0; i < size; ++i) - { - xmb_node_t *node = (xmb_node_t*)menu_entries_get_userdata_at_offset(dst, i); - - xmb_free_node(node); - dst->list[i].userdata = NULL; - file_list_free_actiondata(dst, i); /* this one was allocated by us */ - } - + /* use true here because file_list_copy() doesn't free actiondata */ + xmb_free_list_nodes(dst, true); file_list_copy(src, dst); size = dst->size; @@ -3930,7 +3946,7 @@ static void xmb_list_deep_copy(const file_list_t *src, file_list_t *dst) void *src_adata = (void*)menu_entries_get_actiondata_at_offset(src, i); if (src_udata) - file_list_set_userdata(dst, i, xmb_copy_node(src_udata)); + file_list_set_userdata(dst, i, (xmb_node_t*)xmb_copy_node(src_udata)); if (src_adata) { @@ -4402,7 +4418,7 @@ menu_ctx_driver_t menu_ctx_xmb = { xmb_menu_init_list, xmb_list_insert, NULL, - NULL, + xmb_list_clear, /* free */ xmb_list_clear, xmb_list_cache, xmb_list_push,