From 4118b1e52a2f6efc50ea64a396faa67e82295597 Mon Sep 17 00:00:00 2001 From: Xiang Xiao Date: Mon, 1 Mar 2021 06:59:14 -0800 Subject: [PATCH] fs: avoid the unnecessary memory allocation (#2098) * fix(fs): don't allocate lv_fs_file_t in lv_fs_open avoid the unnecessary allocation * fix(fs): apply the similar file open/close change to dir open/close avoid the unnecessary allocation --- src/lv_draw/lv_img_decoder.c | 44 ++++++++++----------- src/lv_font/lv_font_loader.c | 37 ++++++++---------- src/lv_misc/lv_fs.c | 75 ++++++++++++------------------------ src/lv_misc/lv_fs.h | 5 +-- 4 files changed, 66 insertions(+), 95 deletions(-) diff --git a/src/lv_draw/lv_img_decoder.c b/src/lv_draw/lv_img_decoder.c index 831bfe732..391235cf4 100644 --- a/src/lv_draw/lv_img_decoder.c +++ b/src/lv_draw/lv_img_decoder.c @@ -22,7 +22,7 @@ * TYPEDEFS **********************/ typedef struct { - lv_fs_file_t * f; + lv_fs_file_t f; lv_color_t * palette; lv_opa_t * opa; } lv_img_decoder_built_in_data_t; @@ -282,13 +282,12 @@ lv_res_t lv_img_decoder_built_in_info(lv_img_decoder_t * decoder, const void * s /*Support only "*.bin" files*/ if(strcmp(lv_fs_get_ext(src), "bin")) return LV_RES_INV; - lv_fs_res_t res; - lv_fs_file_t * f; - uint32_t rn; - f = lv_fs_open(src, LV_FS_MODE_RD); - if(f) { - res = lv_fs_read(f, header, sizeof(lv_img_header_t), &rn); - lv_fs_close(f); + lv_fs_file_t f; + lv_fs_res_t res = lv_fs_open(&f, src, LV_FS_MODE_RD); + if(res == LV_FS_RES_OK) { + uint32_t rn; + res = lv_fs_read(&f, header, sizeof(lv_img_header_t), &rn); + lv_fs_close(&f); if(res != LV_FS_RES_OK || rn != sizeof(lv_img_header_t)) { LV_LOG_WARN("Image get info get read file header"); return LV_RES_INV; @@ -326,8 +325,9 @@ lv_res_t lv_img_decoder_built_in_open(lv_img_decoder_t * decoder, lv_img_decoder /*Support only "*.bin" files*/ if(strcmp(lv_fs_get_ext(dsc->src), "bin")) return LV_RES_INV; - lv_fs_file_t * f = lv_fs_open(dsc->src, LV_FS_MODE_RD); - if(f == NULL) { + lv_fs_file_t f; + lv_fs_res_t res = lv_fs_open(&f, dsc->src, LV_FS_MODE_RD); + if(res != LV_FS_RES_OK) { LV_LOG_WARN("Built-in image decoder can't open the file"); return LV_RES_INV; } @@ -338,14 +338,14 @@ lv_res_t lv_img_decoder_built_in_open(lv_img_decoder_t * decoder, lv_img_decoder LV_ASSERT_MALLOC(dsc->user_data); if(dsc->user_data == NULL) { LV_LOG_ERROR("img_decoder_built_in_open: out of memory"); - lv_fs_close(f); + lv_fs_close(&f); return LV_RES_INV; } lv_memset_00(dsc->user_data, sizeof(lv_img_decoder_built_in_data_t)); } lv_img_decoder_built_in_data_t * user_data = dsc->user_data; - user_data->f = f; + lv_memcpy_small(&user_data->f, &f, sizeof(f)); } else if(dsc->src_type == LV_IMG_SRC_VARIABLE) { /*The variables should have valid data*/ @@ -398,11 +398,11 @@ lv_res_t lv_img_decoder_built_in_open(lv_img_decoder_t * decoder, lv_img_decoder if(dsc->src_type == LV_IMG_SRC_FILE) { /*Read the palette from file*/ - lv_fs_seek(user_data->f, 4, LV_FS_SEEK_SET); /*Skip the header*/ + lv_fs_seek(&user_data->f, 4, LV_FS_SEEK_SET); /*Skip the header*/ lv_color32_t cur_color; uint32_t i; for(i = 0; i < palette_size; i++) { - lv_fs_read(user_data->f, &cur_color, sizeof(lv_color32_t), NULL); + lv_fs_read(&user_data->f, &cur_color, sizeof(lv_color32_t), NULL); user_data->palette[i] = lv_color_make(cur_color.ch.red, cur_color.ch.green, cur_color.ch.blue); user_data->opa[i] = cur_color.ch.alpha; } @@ -488,8 +488,8 @@ void lv_img_decoder_built_in_close(lv_img_decoder_t * decoder, lv_img_decoder_ds lv_img_decoder_built_in_data_t * user_data = dsc->user_data; if(user_data) { - if(user_data->f) { - lv_fs_close(user_data->f); + if(dsc->src_type == LV_IMG_SRC_FILE) { + lv_fs_close(&user_data->f); } if(user_data->palette) lv_mem_free(user_data->palette); if(user_data->opa) lv_mem_free(user_data->opa); @@ -512,14 +512,14 @@ static lv_res_t lv_img_decoder_built_in_line_true_color(lv_img_decoder_dsc_t * d uint32_t pos = ((y * dsc->header.w + x) * px_size) >> 3; pos += 4; /*Skip the header*/ - res = lv_fs_seek(user_data->f, pos, LV_FS_SEEK_SET); + res = lv_fs_seek(&user_data->f, pos, LV_FS_SEEK_SET); if(res != LV_FS_RES_OK) { LV_LOG_WARN("Built-in image decoder seek failed"); return LV_RES_INV; } uint32_t btr = len * (px_size >> 3); uint32_t br = 0; - res = lv_fs_read(user_data->f, buf, btr, &br); + res = lv_fs_read(&user_data->f, buf, btr, &br); if(res != LV_FS_RES_OK || btr != br) { LV_LOG_WARN("Built-in image decoder read failed"); return LV_RES_INV; @@ -598,8 +598,8 @@ static lv_res_t lv_img_decoder_built_in_line_alpha(lv_img_decoder_dsc_t * dsc, l data_tmp = img_dsc->data + ofs; } else { - lv_fs_seek(user_data->f, ofs + 4, LV_FS_SEEK_SET); /*+4 to skip the header*/ - lv_fs_read(user_data->f, fs_buf, w, NULL); + lv_fs_seek(&user_data->f, ofs + 4, LV_FS_SEEK_SET); /*+4 to skip the header*/ + lv_fs_read(&user_data->f, fs_buf, w, NULL); data_tmp = fs_buf; } @@ -665,8 +665,8 @@ static lv_res_t lv_img_decoder_built_in_line_indexed(lv_img_decoder_dsc_t * dsc, data_tmp = img_dsc->data + ofs; } else { - lv_fs_seek(user_data->f, ofs + 4, LV_FS_SEEK_SET); /*+4 to skip the header*/ - lv_fs_read(user_data->f, fs_buf, w, NULL); + lv_fs_seek(&user_data->f, ofs + 4, LV_FS_SEEK_SET); /*+4 to skip the header*/ + lv_fs_read(&user_data->f, fs_buf, w, NULL); data_tmp = fs_buf; } diff --git a/src/lv_font/lv_font_loader.c b/src/lv_font/lv_font_loader.c index fac9bc47f..861e0cc48 100644 --- a/src/lv_font/lv_font_loader.c +++ b/src/lv_font/lv_font_loader.c @@ -85,30 +85,27 @@ static unsigned int read_bits(bit_iterator_t * it, int n_bits, lv_fs_res_t * res */ lv_font_t * lv_font_load(const char * font_name) { - bool success = false; + lv_fs_file_t file; + lv_fs_res_t res = lv_fs_open(&file, font_name, LV_FS_MODE_RD); + if(res != LV_FS_RES_OK) + return NULL; lv_font_t * font = lv_mem_alloc(sizeof(lv_font_t)); - memset(font, 0, sizeof(lv_font_t)); - - lv_fs_file_t * file; - file = lv_fs_open(font_name, LV_FS_MODE_RD); - - if(file) { - success = lvgl_load_font(file, font); + if(font) { + memset(font, 0, sizeof(lv_font_t)); + if(!lvgl_load_font(&file, font)) { + LV_LOG_WARN("Error loading font file: %s\n", font_name); + /* + * When `lvgl_load_font` fails it can leak some pointers. + * All non-null pointers can be assumed as allocated and + * `lv_font_free` should free them correctly. + */ + lv_font_free(font); + font = NULL; + } } - if(!success) { - LV_LOG_WARN("Error loading font file: %s\n", font_name); - /* - * When `lvgl_load_font` fails it can leak some pointers. - * All non-null pointers can be assumed as allocated and - * `lv_font_free` should free them correctly. - */ - lv_font_free(font); - font = NULL; - } - - lv_fs_close(file); + lv_fs_close(&file); return font; } diff --git a/src/lv_misc/lv_fs.c b/src/lv_misc/lv_fs.c index 61d870bbf..014516cbd 100644 --- a/src/lv_misc/lv_fs.c +++ b/src/lv_misc/lv_fs.c @@ -62,11 +62,11 @@ bool lv_fs_is_ready(char letter) return drv->ready_cb(drv); } -void * lv_fs_open(const char * path, lv_fs_mode_t mode) +lv_fs_res_t lv_fs_open(lv_fs_file_t * file_p, const char * path, lv_fs_mode_t mode) { if(path == NULL) { LV_LOG_WARN("Can't open file: path is NULL"); - return NULL; + return LV_FS_RES_INV_PARAM; } char letter = path[0]; @@ -74,39 +74,32 @@ void * lv_fs_open(const char * path, lv_fs_mode_t mode) if(drv == NULL) { LV_LOG_WARN("Can't open file (%s): unknown driver letter", path); - return NULL; + return LV_FS_RES_NOT_EX; } if(drv->ready_cb) { if(drv->ready_cb(drv) == false) { LV_LOG_WARN("Can't open file (%s): driver not ready", path); - return NULL; + return LV_FS_RES_HW_ERR; } } if(drv->open_cb == NULL) { LV_LOG_WARN("Can't open file (%s): open function not exists", path); - return NULL; + return LV_FS_RES_NOT_IMP; } - lv_fs_file_t * file_p = lv_mem_alloc(sizeof(lv_fs_file_t)); - if(file_p == NULL) { - LV_LOG_WARN("Can't open file (%s): out of memory", path); - return NULL; + const char * real_path = lv_fs_get_real_path(path); + void *file_d = drv->open_cb(drv, real_path, mode); + + if(file_d == NULL || file_d == (void*)(-1)) { + return LV_FS_RES_UNKNOWN; } file_p->drv = drv; - file_p->file_d = NULL; + file_p->file_d = file_d; - const char * real_path = lv_fs_get_real_path(path); - file_p->file_d = drv->open_cb(drv, real_path, mode); - - if(file_p->file_d == NULL || file_p->file_d == (void*)(-1)) { - lv_mem_free(file_p); - return NULL; - } - - return file_p; + return LV_FS_RES_OK; } lv_fs_res_t lv_fs_close(lv_fs_file_t * file_p) @@ -121,7 +114,8 @@ lv_fs_res_t lv_fs_close(lv_fs_file_t * file_p) lv_fs_res_t res = file_p->drv->close_cb(file_p->drv, file_p->file_d); - lv_mem_free(file_p); /*Clean up*/ + file_p->file_d = NULL; + file_p->drv = NULL; return res; } @@ -192,54 +186,36 @@ lv_fs_res_t lv_fs_tell(lv_fs_file_t * file_p, uint32_t * pos) lv_fs_res_t lv_fs_dir_open(lv_fs_dir_t * rddir_p, const char * path) { - rddir_p->drv = NULL; - rddir_p->dir_d = NULL; - if(path == NULL) return LV_FS_RES_INV_PARAM; char letter = path[0]; + lv_fs_drv_t * drv = lv_fs_get_drv(letter); - rddir_p->drv = lv_fs_get_drv(letter); - - if(rddir_p->drv == NULL) { + if(drv == NULL) { return LV_FS_RES_NOT_EX; } - if(rddir_p->drv->ready_cb != NULL) { - if(rddir_p->drv->ready_cb(rddir_p->drv) == false) { - rddir_p->drv = NULL; + if(drv->ready_cb) { + if(drv->ready_cb(drv) == false) { return LV_FS_RES_HW_ERR; } } - if(rddir_p->drv->dir_open_cb == NULL) { - rddir_p->drv = NULL; + if(drv->dir_open_cb == NULL) { return LV_FS_RES_NOT_IMP; } const char * real_path = lv_fs_get_real_path(path); + void *dir_d = drv->dir_open_cb(drv, real_path); - if(rddir_p->drv->rddir_size == 0) { /*Is dir_d zero size?*/ - /*Pass dir_d's address to dir_open_cb, so the implementor can allocate memory byself*/ - return rddir_p->drv->dir_open_cb(rddir_p->drv, &rddir_p->dir_d, real_path); + if(dir_d == NULL || dir_d == (void*)(-1)) { + return LV_FS_RES_UNKNOWN; } - rddir_p->dir_d = lv_mem_alloc(rddir_p->drv->rddir_size); - LV_ASSERT_MALLOC(rddir_p->dir_d); - if(rddir_p->dir_d == NULL) { - rddir_p->drv = NULL; - return LV_FS_RES_OUT_OF_MEM; /* Out of memory */ - } + rddir_p->drv = drv; + rddir_p->dir_d = dir_d; - lv_fs_res_t res = rddir_p->drv->dir_open_cb(rddir_p->drv, rddir_p->dir_d, real_path); - - if(res != LV_FS_RES_OK) { - lv_mem_free(rddir_p->dir_d); - rddir_p->dir_d = NULL; - rddir_p->drv = NULL; - } - - return res; + return LV_FS_RES_OK; } lv_fs_res_t lv_fs_dir_read(lv_fs_dir_t * rddir_p, char * fn) @@ -271,7 +247,6 @@ lv_fs_res_t lv_fs_dir_close(lv_fs_dir_t * rddir_p) lv_fs_res_t res = rddir_p->drv->dir_close_cb(rddir_p->drv, rddir_p->dir_d); - lv_mem_free(rddir_p->dir_d); /*Clean up*/ rddir_p->dir_d = NULL; rddir_p->drv = NULL; diff --git a/src/lv_misc/lv_fs.h b/src/lv_misc/lv_fs.h index add234681..580b2c11c 100644 --- a/src/lv_misc/lv_fs.h +++ b/src/lv_misc/lv_fs.h @@ -70,7 +70,6 @@ typedef uint8_t lv_fs_whence_t; typedef struct _lv_fs_drv_t { char letter; - uint16_t rddir_size; bool (*ready_cb)(struct _lv_fs_drv_t * drv); void * (*open_cb)(struct _lv_fs_drv_t * drv, const char * path, lv_fs_mode_t mode); @@ -80,7 +79,7 @@ typedef struct _lv_fs_drv_t { lv_fs_res_t (*seek_cb)(struct _lv_fs_drv_t * drv, void * file_p, uint32_t pos, lv_fs_whence_t whence); lv_fs_res_t (*tell_cb)(struct _lv_fs_drv_t * drv, void * file_p, uint32_t * pos_p); - lv_fs_res_t (*dir_open_cb)(struct _lv_fs_drv_t * drv, void * rddir_p, const char * path); + void * (*dir_open_cb)(struct _lv_fs_drv_t * drv, const char * path); lv_fs_res_t (*dir_read_cb)(struct _lv_fs_drv_t * drv, void * rddir_p, char * fn); lv_fs_res_t (*dir_close_cb)(struct _lv_fs_drv_t * drv, void * rddir_p); @@ -145,7 +144,7 @@ bool lv_fs_is_ready(char letter); * @param mode read: FS_MODE_RD, write: FS_MODE_WR, both: FS_MODE_RD | FS_MODE_WR * @return LV_FS_RES_OK or any error from lv_fs_res_t enum */ -void * lv_fs_open(const char * path, lv_fs_mode_t mode); +lv_fs_res_t lv_fs_open(lv_fs_file_t * file_p, const char * path, lv_fs_mode_t mode); /** * Close an already opened file