Syslinux - Nov 2016 - [PATCH 0/2] improve Lua API for files and initramfs objects

From: Paul Emmerich <p.emmerich at first-colo.net>

Hi,

the new API for initramfs and files in master lacked the ability to build
initramfs objects from files loaded via HTTP/TFTP in Lua. The documentation
indicated that it should be possible (and I believe I did that in an older
version). I implemented a few new functions to handle files/initramfs
objects better.

Changes:
* NEW: file:data() returns file contents as a string
* NEW: initramfs:load_file(filename, source, mode, do_mkdir)
       loads a file from source and add it an initramfs object
* CHANGED: initramfs:add_file(filename, data, mode, do_mkdir)
       data can now also be a file loaded by loadfile(name)

I've also changed the order of the do_mkdirs and mode arguments and changed
the default of do_mkdirs to true (since that is more useful). This change
is not backwards compatible, but it shouldn't be a big problem since the
new API in the current master branch is not backwards compatible with the
latest release anyways.

Let me know what you think.

 Paul

Paul Emmerich (2):
  lua: improve handling of files/initramfs
  docs: document Lua syslinux module changes

 com32/lua/doc/syslinux.asc | 15 ++++++++++---
 com32/lua/src/syslinux.c   | 52 +++++++++++++++++++++++++++++++++++++---------
 2 files changed, 54 insertions(+), 13 deletions(-)

-- 
2.2.1

From: Paul Emmerich <p.emmerich at first-colo.net>

* add initramfs:load_file(), a wrapper for initramfs_load_file
* initramfs:add_file() now accepts files loaded via sl.loadfile()
* initramfs:add_file() sets mkdirs by default now
* add file:data() returning file contents as a string
---
 com32/lua/src/syslinux.c | 52 ++++++++++++++++++++++++++++++++++++++----------
 1 file changed, 42 insertions(+), 10 deletions(-)

diff --git a/com32/lua/src/syslinux.c b/com32/lua/src/syslinux.c
index 9a2998b..108081d 100644
--- a/com32/lua/src/syslinux.c
+++ b/com32/lua/src/syslinux.c
@@ -280,6 +280,15 @@ static int sl_filename(lua_State * L)
     return 1;
 }
 
+static int sl_filedata(lua_State * L)
+{
+    const syslinux_file *file = luaL_checkudata(L, 1, SYSLINUX_FILE);
+
+    lua_pushlstring(L, file->data, file->size);
+
+    return 1;
+}
+
 static int sl_initramfs_init(lua_State * L)
 {
     struct initramfs *ir = lua_newuserdata (L, sizeof (*ir));
@@ -303,20 +312,41 @@ static int sl_initramfs_load_archive(lua_State * L)
 static int sl_initramfs_add_file(lua_State * L)
 {
     const char *filename = luaL_checkstring(L, 2);
-    size_t file_len;
-    const char *file_data = luaL_optlstring (L, 3, NULL, &file_len);
     void *data = NULL;
-
-    if (file_len) {
-        data = malloc (file_len);
+    size_t data_len = 0;
+    const char *string_data;
+    const syslinux_file *file_data;
+
+    if (lua_isstring(L, 3)) {
+        /* called with file contents as string */
+        string_data = lua_tolstring(L, 3, &data_len);
+        data = malloc (data_len);
         if (!data) return luaL_error (L, "Out of memory");
-        memcpy (data, file_data, file_len);
-    }
+        memcpy(data, string_data, data_len);
+    } else if (lua_isuserdata(L, 3)) {
+        /* called with a SYSLINUX_FILE object */
+        file_data = luaL_checkudata(L, 3, SYSLINUX_FILE);
+        data = file_data->data;
+        data_len = file_data->size;
+    } /* else: nil, just create an empty file */
+
     if (initramfs_add_file(luaL_checkudata(L, 1, SYSLINUX_INITRAMFS),
-                           data, file_len, file_len, filename,
-                           luaL_optint (L, 4, 0), luaL_optint (L, 5, 0755)))
+                           data, data_len, data_len, filename,
+                           luaL_optint (L, 5, 1), luaL_optint (L, 4, 0755)))
         return luaL_error (L, "Adding file %s to initramfs failed",
filename);
-    lua_settop (L, 1);
+    lua_settop(L, 1);
+    return 1;
+}
+
+static int sl_initramfs_load_file(lua_State * L)
+{
+    const char *dst_filename = luaL_checkstring(L, 2);
+    const char *src_filename = luaL_checkstring(L, 3);
+    if (initramfs_load_file(luaL_checkudata(L, 1, SYSLINUX_INITRAMFS),
+                           src_filename, dst_filename,
+                           luaL_optint(L, 5, 1), luaL_optint(L, 4, 0755)))
+        return luaL_error (L, "Adding file %s to initramfs failed",
dst_filename);
+    lua_settop(L, 1);
     return 1;
 }
 
@@ -461,6 +491,7 @@ static const luaL_Reg file_methods[] = {
     {"__gc", sl_unloadfile},
     {"name", sl_filename},
     {"size", sl_filesize},
+    {"data", sl_filedata},
     {NULL, NULL}
 };
 
@@ -468,6 +499,7 @@ static const luaL_Reg initramfs_methods[] = {
     {"__gc", sl_initramfs_purge},
     {"load", sl_initramfs_load_archive},
     {"add_file", sl_initramfs_add_file},
+    {"load_file", sl_initramfs_load_file},
     {"size", sl_initramfs_size},
     {NULL, NULL}
 };
-- 
2.2.1

From: Paul Emmerich <p.emmerich at first-colo.net>

---
 com32/lua/doc/syslinux.asc | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/com32/lua/doc/syslinux.asc b/com32/lua/doc/syslinux.asc
index 6475d42..92ad7aa 100644
--- a/com32/lua/doc/syslinux.asc
+++ b/com32/lua/doc/syslinux.asc
@@ -90,6 +90,9 @@ Return the size of a loaded _file_.
 name():::
 Return the name of a loaded _file_.
 
+data():::
+Return the contents of a loaded _file_ as a string.
+
 initramfs()::
 Return an empty _initramfs_ object.  Its methods are:
 
@@ -97,11 +100,17 @@ load(filename):::
 Load contents of +filename+ into an _initramfs_ and return the
 extended object.
 
-add_file(filename[,data[,do_mkdir[,mode]]]):::
+add_file(filename[,data[,mode[,do_mkdir]]]):::
 Add +filename+ of +mode+ containing +data+ to an _initramfs_ and
 return the extended object.
-If +do_mkdir+, create parent directories, too.  +mode+ defaults
-to 7*64+5*8+5 (which is 0755 in octal).
++data+ can either be a string or a _file_ object.
+If +do_mkdir+, create parent directories, too. +do_mkdir+ defaults
+to true. +mode+ defaults to 7*64+5*8+5 (which is 0755 in octal).
+
+load_file(filename, source, mode, do_mkdir):::
+Load a file from +source+, add it as +filename+ of +mode+ to
+an _initramfs_ and return the extended object.
++mode+ and +do_mkdir+ are the same as in _add_file_.
 
 size():::
 Returns the current size of an _initramfs_.
-- 
2.2.1

"p.emmerich--- via Syslinux" <syslinux at zytor.com> writes:
> the new API for initramfs and files in master lacked the ability to build
> initramfs objects from files loaded via HTTP/TFTP in Lua.
Hi,

Standard Lua file I/O should work for that.  For example, I use

local function readAll (file)
   local f = io.open ("/extlinux/" .. file)
   if f == nil then return nil end
   local content = f:read ("*a")
   f:close ()
   return content
end

for reading configuration files from disk.  Does it not work with HTTP/
TFTP for you?  Using that, load_file could be implemented in Lua as:

function load_file (initrd, name, mkdir, mode)
  local content = readAll (name)
  initrd:add_file (name, content, mkdir, mode)
end

All in all, I can't see the need for new bindings for this task, but
maybe I miss something.  At the same time, file:data() might
occasionally come useful.
> I've also changed the order of the do_mkdirs and mode arguments and
changed
> the default of do_mkdirs to true (since that is more useful). This change
> is not backwards compatible, but it shouldn't be a big problem since
the
> new API in the current master branch is not backwards compatible with the
> latest release anyways.
I've got no strong feelings about the argument order or backward
compatibility in the Lua bindings, though we shouldn't deliberately
break it more than necessary without adding some value in the process.
-- 
Regards,
Feri

Hi,
> Ferenc W?gner <wferi at niif.hu>:
> for reading configuration files from disk.  Does it not work with HTTP/
> TFTP for you?  Using that, load_file could be implemented in Lua as:
that does work, yes. It just looked like a file object should have a some way
to get the contents. I'm actually not using it in our scripts.

> All in all, I can't see the need for new bindings for this task, but
> maybe I miss something.  At the same time, file:data() might
> occasionally come useful.
initramfs:load_file() is basically a memory optimization over your solution.
It calls initramfs_load_file() directly, this loads the file exactly once into
memory.

Going through a Lua string loads the file, then copies it into a (interned) Lua
string,
then copies this string again in initramfs:add_file(). This doubles the peak
memory usage
(might even triple it if you are not freeing the file before passing it into
add_file)

This is a concern for our setup: we load relatively large wimboot images and
support relatively small VMs at the same time.


 Paul