Remove luaL_buffer from file_g_read() (#1541)

* remove luaL_buffer from file_g_read()
- avoid memory leak when function gets terminated by lua_error
- skip scanning for end_char when reading until EOF
* attempt to free memory in any case

app/modules/file.c

@@ -2,12 +2,17 @@
 
 #include "module.h"
 #include "lauxlib.h"
+#include "lmem.h"
 #include "platform.h"
 
 #include "c_types.h"
 #include "vfs.h"
 #include "c_string.h"
 
+#include <alloca.h>
+
+#define FILE_READ_CHUNK 1024
+
 static int file_fd = 0;
 static int rtc_cb_ref = LUA_NOREF;
 
@@ -255,36 +260,55 @@ static int file_rename( lua_State* L )
 // g_read()
 static int file_g_read( lua_State* L, int n, int16_t end_char )
 {
-  if(n <= 0 || n > LUAL_BUFFERSIZE)
-    n = LUAL_BUFFERSIZE;
+  static char *heap_mem = NULL;
+  // free leftover memory
+  if (heap_mem)
+    luaM_free(L, heap_mem);
+
+  if(n <= 0)
+    n = FILE_READ_CHUNK;
+
   if(end_char < 0 || end_char >255)
     end_char = EOF;
 
-  luaL_Buffer b;
   if(!file_fd)
     return luaL_error(L, "open a file first");
 
-  luaL_buffinit(L, &b);
-  char *p = luaL_prepbuffer(&b);
+  char *p;
   int i;
 
+  if (n > LUAL_BUFFERSIZE) {
+    // get buffer from heap
+    p = heap_mem = luaM_malloc(L, n);
+  } else {
+    // small chunks go onto the stack
+    p = alloca(n);
+  }
+
   n = vfs_read(file_fd, p, n);
-  for (i = 0; i < n; ++i)
+  // bypass search if no end character provided
+  for (i = end_char != EOF ? 0 : n; i < n; ++i)
     if (p[i] == end_char)
     {
       ++i;
       break;
     }
 
-  if(i==0){
-    luaL_pushresult(&b);  /* close buffer */
-    return (lua_objlen(L, -1) > 0);  /* check whether read something */
+  if (i == 0) {
+    if (heap_mem) {
+      luaM_free(L, heap_mem);
+      heap_mem = NULL;
+    }
+    return 0;
   }
 
   vfs_lseek(file_fd, -(n - i), VFS_SEEK_CUR);
-  luaL_addsize(&b, i);
-  luaL_pushresult(&b);  /* close buffer */
-  return 1;  /* read at least an `eol' */ 
+  lua_pushlstring(L, p, i);
+  if (heap_mem) {
+    luaM_free(L, heap_mem);
+    heap_mem = NULL;
+  }
+  return 1;
 }
 
 // Lua: read()
@@ -293,15 +317,12 @@ static int file_g_read( lua_State* L, int n, int16_t end_char )
 // file.read('q') will read until 'q' or EOF is reached. 
 static int file_read( lua_State* L )
 {
-  unsigned need_len = LUAL_BUFFERSIZE;
+  unsigned need_len = FILE_READ_CHUNK;
   int16_t end_char = EOF;
   size_t el;
   if( lua_type( L, 1 ) == LUA_TNUMBER )
   {
     need_len = ( unsigned )luaL_checkinteger( L, 1 );
-    if( need_len > LUAL_BUFFERSIZE ){
-      need_len = LUAL_BUFFERSIZE;
-    }
   }
   else if(lua_isstring(L, 1))
   {
@@ -318,7 +339,7 @@ static int file_read( lua_State* L )
 // Lua: readline()
 static int file_readline( lua_State* L )
 {
-  return file_g_read(L, LUAL_BUFFERSIZE, '\n');
+  return file_g_read(L, FILE_READ_CHUNK, '\n');
 }
 
 // Lua: write("string")

docs/en/modules/file.md

@@ -298,14 +298,18 @@ end
 
 Read content from the open file.
 
+!!! note
+
+    The function temporarily allocates 2 * (number of requested bytes) on the heap for buffering and processing the read data. Default chunk size (`FILE_READ_CHUNK`) is 1024 bytes and is regarded to be safe. Pushing this by 4x or more can cause heap overflows depending on the application. Consider this when selecting a value for parameter `n_or_char`.
+
 #### Syntax
-`file.read([n_or_str])`
+`file.read([n_or_char])`
 
 #### Parameters
-- `n_or_str`:
-	- if nothing passed in, read up to `LUAL_BUFFERSIZE` bytes (default 1024) or the entire file (whichever is smaller)
-	- if passed a number n, then read the file until the lesser of `n` bytes, `LUAL_BUFFERSIZE` bytes, or EOF is reached. Specifying a number larger than the buffer size will read the buffer size.
-	- if passed a string `str`, then read until `str` appears next in the file, `LUAL_BUFFERSIZE` bytes have been read, or EOF is reached
+- `n_or_char`:
+	- if nothing passed in, then read up to `FILE_READ_CHUNK` bytes or the entire file (whichever is smaller).
+	- if passed a number `n`, then read up to `n` bytes or the entire file (whichever is smaller).
+	- if passed a string containing the single character `char`, then read until `char` appears next in the file, `FILE_READ_CHUNK` bytes have been read, or EOF is reached.
 
 #### Returns
 File content as a string, or nil when EOF
@@ -331,7 +335,7 @@ end
 
 ## file.readline()
 
-Read the next line from the open file. Lines are defined as zero or more bytes ending with a EOL ('\n') byte. If the next line is longer than `LUAL_BUFFERSIZE`, this function only returns the first `LUAL_BUFFERSIZE` bytes (this is 1024 bytes by default).
+Read the next line from the open file. Lines are defined as zero or more bytes ending with a EOL ('\n') byte. If the next line is longer than 1024, this function only returns the first 1024 bytes.
 
 #### Syntax
 `file.readline()`