From 12ef045fdf5a9e2b4d5618cc3b7ba50ecc0ccf69 Mon Sep 17 00:00:00 2001 From: William Wilgus Date: Sat, 15 Oct 2022 09:08:09 -0400 Subject: [PATCH] move buflib_free invalid handle check to the function allow buflib_free to check for invalid or already freed handles within the function -- remove all the invalid handle guards thru core_free Change-Id: Ibdcbc82760fc93b674c42283fca420d94907df8e --- apps/action.c | 3 +-- apps/debug_menu.c | 3 +-- apps/gui/skin_engine/skin_backdrops.c | 3 +-- apps/gui/skin_engine/skin_parser.c | 4 +--- apps/playback.c | 3 +-- apps/plugin.c | 6 ++---- apps/rbcodec_helpers.c | 5 +---- apps/recorder/pcm_record.c | 3 +-- apps/talk.c | 12 +++++------ firmware/buflib.c | 2 ++ firmware/common/dircache.c | 8 +++----- firmware/common/zip.c | 20 +++++++------------ firmware/font.c | 3 +-- firmware/linuxboot.c | 3 +-- .../mips/ingenic_x1000/installer-x1000.c | 3 +-- firmware/usbstack/usb_storage.c | 3 +-- lib/x1000-installer/src/xf_nandio.c | 5 +---- lib/x1000-installer/src/xf_package.c | 5 +---- 18 files changed, 32 insertions(+), 62 deletions(-) diff --git a/apps/action.c b/apps/action.c index da08e29dbb..8c03ca4d65 100644 --- a/apps/action.c +++ b/apps/action.c @@ -1219,8 +1219,7 @@ int action_set_keymap_handle(int handle, int count) return -1; #else /* free an existing remap */ - if (action_last.key_remap > 0) - action_last.key_remap = core_free(action_last.key_remap); + action_last.key_remap = core_free(action_last.key_remap); /* if clearing the remap, we're done */ if (count <= 0 || handle <= 0) diff --git a/apps/debug_menu.c b/apps/debug_menu.c index b034f25df0..d17668ade5 100644 --- a/apps/debug_menu.c +++ b/apps/debug_menu.c @@ -520,8 +520,7 @@ static int bf_action_cb(int action, struct gui_synclist* list) /* for some reason simplelist doesn't allow adding items here if * info.get_name is given, so use normal list api */ gui_synclist_set_nb_items(list, core_get_num_blocks()); - if (handle > 0) - core_free(handle); + core_free(handle); } action = ACTION_REDRAW; } diff --git a/apps/gui/skin_engine/skin_backdrops.c b/apps/gui/skin_engine/skin_backdrops.c index 146dccb18a..1def0812da 100644 --- a/apps/gui/skin_engine/skin_backdrops.c +++ b/apps/gui/skin_engine/skin_backdrops.c @@ -267,8 +267,7 @@ void skin_backdrop_unload(int backdrop_id) backdrops[backdrop_id].ref_count--; if (backdrops[backdrop_id].ref_count <= 0) { - if (backdrops[backdrop_id].buflib_handle > 0) - core_free(backdrops[backdrop_id].buflib_handle); + core_free(backdrops[backdrop_id].buflib_handle); backdrops[backdrop_id].buffer = NULL; backdrops[backdrop_id].buflib_handle = -1; backdrops[backdrop_id].loaded = false; diff --git a/apps/gui/skin_engine/skin_parser.c b/apps/gui/skin_engine/skin_parser.c index 6021f0647c..26a251bb4b 100644 --- a/apps/gui/skin_engine/skin_parser.c +++ b/apps/gui/skin_engine/skin_parser.c @@ -1829,9 +1829,7 @@ void skin_data_free_buflib_allocs(struct wps_data *wps_data) abort: wps_data->font_ids = PTRTOSKINOFFSET(skin_buffer, NULL); /* Safe if skin_buffer is NULL */ wps_data->images = PTRTOSKINOFFSET(skin_buffer, NULL); - if (wps_data->buflib_handle > 0) - core_free(wps_data->buflib_handle); - wps_data->buflib_handle = -1; + wps_data->buflib_handle = core_free(wps_data->buflib_handle); #endif } diff --git a/apps/playback.c b/apps/playback.c index 7fdb302d71..9b67f2ccc7 100644 --- a/apps/playback.c +++ b/apps/playback.c @@ -3683,8 +3683,7 @@ void audio_hard_stop(void) #ifdef PLAYBACK_VOICE voice_stop(); #endif - if (audiobuf_handle > 0) - audiobuf_handle = core_free(audiobuf_handle); + audiobuf_handle = core_free(audiobuf_handle); } /* Resume playback if paused */ diff --git a/apps/plugin.c b/apps/plugin.c index 888a9e109c..4016040c5a 100644 --- a/apps/plugin.c +++ b/apps/plugin.c @@ -839,8 +839,7 @@ int plugin_load(const char* plugin, const void* parameter) } lc_close(current_plugin_handle); current_plugin_handle = pfn_tsr_exit = NULL; - if (plugin_buffer_handle > 0) - plugin_buffer_handle = core_free(plugin_buffer_handle); + plugin_buffer_handle = core_free(plugin_buffer_handle); } splash(0, ID2P(LANG_WAIT)); @@ -921,8 +920,7 @@ int plugin_load(const char* plugin, const void* parameter) { /* close handle if plugin is no tsr one */ lc_close(current_plugin_handle); current_plugin_handle = NULL; - if (plugin_buffer_handle > 0) - plugin_buffer_handle = core_free(plugin_buffer_handle); + plugin_buffer_handle = core_free(plugin_buffer_handle); } talk_buffer_set_policy(TALK_BUFFER_DEFAULT); diff --git a/apps/rbcodec_helpers.c b/apps/rbcodec_helpers.c index 78e068ded7..b856d7355d 100644 --- a/apps/rbcodec_helpers.c +++ b/apps/rbcodec_helpers.c @@ -87,10 +87,7 @@ void tdspeed_free_buffers(int32_t **buffers, int nbuf) { for (int i = 0; i < nbuf; i++) { - if (handles[i] > 0) - core_free(handles[i]); - - handles[i] = 0; + handles[i] = core_free(handles[i]); buffers[i] = NULL; } } diff --git a/apps/recorder/pcm_record.c b/apps/recorder/pcm_record.c index 0f6e8469ec..d3d45d3e1c 100644 --- a/apps/recorder/pcm_record.c +++ b/apps/recorder/pcm_record.c @@ -1437,8 +1437,7 @@ static void on_close_recording(void) audio_set_output_source(AUDIO_SRC_PLAYBACK); pcm_apply_settings(); - if (pcmrec_handle > 0) - pcmrec_handle = core_free(pcmrec_handle); + pcmrec_handle = core_free(pcmrec_handle); talk_buffer_set_policy(TALK_BUFFER_DEFAULT); send_event(RECORDING_EVENT_STOP, NULL); diff --git a/apps/talk.c b/apps/talk.c index 1be2a041c2..a7af58c273 100644 --- a/apps/talk.c +++ b/apps/talk.c @@ -226,7 +226,7 @@ static int shrink_callback(int handle, unsigned hints, void *start, size_t old_s mutex_lock(&read_buffer_mutex); /* the clip buffer isn't usable without index table */ - if (handle == index_handle && talk_handle > 0) + if (handle == index_handle) talk_handle = core_free(talk_handle); if (h) *h = core_free(handle); @@ -546,8 +546,7 @@ static bool create_clip_buffer(size_t max_size) alloc_err: talk_status = TALK_STATUS_ERR_ALLOC; - if (index_handle > 0) - index_handle = core_free(index_handle); + index_handle = core_free(index_handle); return false; } static inline int load_voicefile_failure(int fd) @@ -621,8 +620,7 @@ static bool load_voicefile_data(int fd) /* just allocate, populate on an as-needed basis later * re-create the clip buffer to ensure clip_ctx is up-to-date */ - if (talk_handle > 0) - talk_handle = core_free(talk_handle); + talk_handle = core_free(talk_handle); if (!create_clip_buffer(voicebuf_size)) return false; @@ -823,8 +821,8 @@ void talk_init(void) voicefile_size = has_voicefile = 0; /* need to free these as their size depends on the voice file, and * this function is called when the talk voice file changes */ - if (index_handle > 0) index_handle = core_free(index_handle); - if (talk_handle > 0) talk_handle = core_free(talk_handle); + index_handle = core_free(index_handle); + talk_handle = core_free(talk_handle); /* don't free thumb handle, it doesn't depend on the actual voice file * and so we can re-use it if it's already allocated in any event */ diff --git a/firmware/buflib.c b/firmware/buflib.c index 7263f1b95d..3130bc960c 100644 --- a/firmware/buflib.c +++ b/firmware/buflib.c @@ -820,6 +820,8 @@ find_block_before(struct buflib_context *ctx, union buflib_data* block, int buflib_free(struct buflib_context *ctx, int handle_num) { + if (handle_num <= 0) /* invalid or already free */ + return handle_num; union buflib_data *handle = ctx->handle_table - handle_num, *freed_block = handle_to_block(ctx, handle_num), *block, *next_block; diff --git a/firmware/common/dircache.c b/firmware/common/dircache.c index 7a84b761a0..8917b3348e 100644 --- a/firmware/common/dircache.c +++ b/firmware/common/dircache.c @@ -1963,8 +1963,7 @@ static int prepare_build(bool *realloced) int handle = reset_buffer(); dircache_unlock(); - if (handle > 0) - core_free(handle); + core_free(handle); handle = alloc_cache(size); @@ -2164,8 +2163,7 @@ static void dircache_suspend_internal(bool freeit) dircache_unlock(); - if (handle > 0) - core_free(handle); + core_free(handle); thread_wait(thread_id); @@ -3179,7 +3177,7 @@ error: dircache_unlock(); error_nolock: - if (rc < 0 && handle > 0) + if (rc < 0) core_free(handle); if (fd >= 0) diff --git a/firmware/common/zip.c b/firmware/common/zip.c index 36b90a9223..22c6226e3b 100644 --- a/firmware/common/zip.c +++ b/firmware/common/zip.c @@ -237,8 +237,7 @@ static int zip_read_ed(struct zip* z) { rv = 0; bail: - if (mem_handle >= 0) - core_free(mem_handle); + core_free(mem_handle); return rv; } @@ -337,10 +336,9 @@ static int zip_read_cd(struct zip* z, bool use_cb) { rv = 0; bail: - if (rv != 0 && cds_handle >= 0) + if (rv != 0) core_free(cds_handle); - if (mem_handle >= 0) - core_free(mem_handle); + core_free(mem_handle); return rv; } @@ -497,8 +495,7 @@ static int zip_read_entries(struct zip* z) { rv = 0; bail: - if (mem_handle >= 0) - core_free(mem_handle); + core_free(mem_handle); return rv; } @@ -754,10 +751,8 @@ struct zip* zip_open(const char* name, bool try_mem) { bail: if (file >= 0) close(file); - if (mem_handle >= 0) - core_free(mem_handle); - if (zip_handle >= 0) - core_free(zip_handle); + core_free(mem_handle); + core_free(zip_handle); return NULL; } @@ -875,8 +870,7 @@ void zip_close(struct zip* z) { z->close(z); - if (z->cds_handle >= 0) - core_free(z->cds_handle); + core_free(z->cds_handle); core_free(z->zip_handle); } diff --git a/firmware/font.c b/firmware/font.c index b8fa1c537f..724cb84d57 100644 --- a/firmware/font.c +++ b/firmware/font.c @@ -604,8 +604,7 @@ void font_unload(int font_id) glyph_cache_save(font_id); close(pf->fd); } - if (handle > 0) - core_free(handle); + core_free(handle); buflib_allocations[font_id] = -1; } diff --git a/firmware/linuxboot.c b/firmware/linuxboot.c index f9732f6ace..419044b5a9 100644 --- a/firmware/linuxboot.c +++ b/firmware/linuxboot.c @@ -221,8 +221,7 @@ int uimage_load(struct uimage_header* uh, size_t* out_size, ret = 0; err: - if(state_h > 0) - core_free(state_h); + core_free(state_h); if(out_h > 0) { if(ret == 0) ret = out_h; diff --git a/firmware/target/mips/ingenic_x1000/installer-x1000.c b/firmware/target/mips/ingenic_x1000/installer-x1000.c index 48850f8a62..ef5bbcd66a 100644 --- a/firmware/target/mips/ingenic_x1000/installer-x1000.c +++ b/firmware/target/mips/ingenic_x1000/installer-x1000.c @@ -178,8 +178,7 @@ static void updater_cleanup(struct updater* u) if(u->tar && mtar_is_open(u->tar)) mtar_close(u->tar); - if(u->buf_hnd >= 0) - core_free(u->buf_hnd); + core_free(u->buf_hnd); if(u->ndrv) { nand_close(u->ndrv); diff --git a/firmware/usbstack/usb_storage.c b/firmware/usbstack/usb_storage.c index eb82f72eae..08b1f0b7e7 100644 --- a/firmware/usbstack/usb_storage.c +++ b/firmware/usbstack/usb_storage.c @@ -481,8 +481,7 @@ void usb_storage_init_connection(void) void usb_storage_disconnect(void) { - if (usb_handle > 0) - usb_handle = core_free(usb_handle); + usb_handle = core_free(usb_handle); } /* called by usb_core_transfer_complete() */ diff --git a/lib/x1000-installer/src/xf_nandio.c b/lib/x1000-installer/src/xf_nandio.c index 29ff9d9120..6dc87bc420 100644 --- a/lib/x1000-installer/src/xf_nandio.c +++ b/lib/x1000-installer/src/xf_nandio.c @@ -75,10 +75,7 @@ int xf_nandio_init(struct xf_nandio* nio) void xf_nandio_destroy(struct xf_nandio* nio) { - if(nio->alloc_handle > 0) { - core_free(nio->alloc_handle); - nio->alloc_handle = 0; - } + nio->alloc_handle = core_free(nio->alloc_handle); if(nio->ndrv) { nand_lock(nio->ndrv); diff --git a/lib/x1000-installer/src/xf_package.c b/lib/x1000-installer/src/xf_package.c index 04b32cdcb0..fb107aef72 100644 --- a/lib/x1000-installer/src/xf_package.c +++ b/lib/x1000-installer/src/xf_package.c @@ -257,8 +257,5 @@ void xf_package_close(struct xf_package* pkg) if(mtar_is_open(pkg->tar)) mtar_close(pkg->tar); - if(pkg->alloc_handle > 0) { - core_free(pkg->alloc_handle); - pkg->alloc_handle = 0; - } + pkg->alloc_handle = core_free(pkg->alloc_handle); }