-
Notifications
You must be signed in to change notification settings - Fork 3.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ESP32: remove use of luaM_free in file module #2631
Conversation
d2106b7
to
c1cf4dd
Compare
@@ -383,32 +383,27 @@ static int file_stat( lua_State* L ) | |||
// g_read() | |||
static int file_g_read( lua_State* L, int n, int16_t end_char, int fd ) | |||
{ | |||
static char *heap_mem = NULL; | |||
// free leftover memory | |||
if (heap_mem) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This code was added to free memory which was left allocated by a previous call. Standard execution flow doesn't need that, but heap_mem
isn't freed in case lua_pushlstring()
throws a memory exception as noted in #1541 (comment).
If we keep considering this corner case then bufsize
would also need to be declared static
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@devsaurus I added a new function to make a protected call to lua_pushlstring()
and avoid this issue. The new luaX_pushlstring()
returns nonzero instead of throwing an exception, allowing you to free memory. This function could be useful elsewhere.
Please check it out and let me know :-)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Excellent solution!
c1cf4dd
to
3d3452d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good and is ready to merge IMO.
Works toward fixing #2377 for ESP32
dev
branch rather than formaster
.docs/en/*
.modifications in
file_g_read()
to avoid use ofluaM_free()