free memory

[glory-man]

It seems like code in this repo has an error with free memory resources

https://github.com/lvgl/lv_lib_png/blob/7ff6168764893d658375a9c3173337506ff4eb1b/lv_png.c#L162 https://github.com/lvgl/lv_lib_png/blob/bf1531afe07c9f861107559e29ab8a2d83e4715a/lv_png.c#L204

It uses free() while png_data and img_data are allocated in lodepng.c with lodepng_malloc(), lodepng_realloc().

[kisvegabor]

Isn't it resolve to free here?

[glory-man]

Isn't it resolve to free here?

lodepng.c uses lodepng_malloc() and lodepng_realloc() to allocate memory, which could be implemented independed from lodepng (especially if LODEPNG_COMPILE_ALLOCATORS not defined). And users which use this lib with RTOS can use OS allocators. You can also use lvmem allocators, which can use heap/memory independend from system malloc(). And this is strange if lodepng use lv_mem_alloc() to allocate memory, but lv_png use system free() to free it. Potential memory leakage. It will be better to use lodepng_free() instead of free().

[glory-man]

This problem may occur even if I will use latest branch of lvgl with lv_png as extra lib. For example, if I define in my project LODEPNG_NO_COMPILE_ALLOCATORS-macro and will create my own independent allocators (lodepng_malloc(), lodepng_realloc(), lodepng_free()) that won't use lv_mem_alloc()/realloc() but will use some memory region. In this case lodepng will use my allocators to allocate memory for png_data and dsc->img_data, but in lv_png this memory will not be freed because it uses lv_mem_free(), which has no information about where the memory was allocated. I think in this case also will be better to use lodepng_free() instead of lv_mem_free() (I mean lvgl-branch).

[kisvegabor]

Got it.

Although, this lib is not required for v8 and v7 is already EOL, I'd be be happy to merge a PR if you send one.

[glory-man]

I didn't test this changes - but it should work: https://github.com/lvgl/lv_lib_png/pull/29 - PR for release/v7 branch https://github.com/lvgl/lv_lib_png/pull/30 - PR for master branch

[kisvegabor]

Both are merged, thanks!