Similar issue with 'luaL_newmetatable' in I/O standard library

[Jure Bagić]

In regards to commit 'bc4bbce' that fixes the issue with 'luaL_newmetatable'
in auxiliary library.

The creation of file metatable in Input/output standard library faces the same
problem in case of OOM.

liolib.c:797 the function 'createmeta'

static void createmeta (lua_State *L) {
  luaL_newmetatable(L, LUA_FILEHANDLE);  /* metatable for file handles */
  luaL_setfuncs(L, metameth, 0);  /* add metamethods to new metatable */
  luaL_newlibtable(L, meth);  /* create method table */
  luaL_setfuncs(L, meth, 0);  /* add file methods to method table */
  lua_setfield(L, -2, "__index");  /* metatable.__index = method table */
  lua_pop(L, 1);  /* pop metatable */
}

(After 'luaL_newmetatable' the table is already in the registry; the proper re-'require'
of I/O library skips the creation of the file metatable.)

Would be nice to make 'luaL_newmetatable' a macro over something like
'luaL_newmetatable[from|init]'. Where the 3rd parameter is the array of
'luaL_Reg' (and 4th parameter is the 'nup'). Then this function would create
and initialize the table in case it is not found in the registry, and after all that,
if no errors, it would set it into the registry.

If you want empty metatable, you either not use this function or call it with 3rd
parameter (the array) as NULL or even better, you call the 'luaL_newmetatable'
macro. This should not break backwards compatibility. I think :).

The biggest hassle in this new function would be to get the size hint for
the table without adding additional parameter to the function (which
would be quite verbose) or the double iteration of 'luaL_Reg' array.

Even better solution is to change the preallocated memory message
to an empty string to further avoid the out of memory errors and
variable names should not be longer than one character (or two
characters for premium PCs).

-- Jure

[Jure Bagić]

Additionally, shouldn't 'lua_setupvalue' have 'api_checkpop' and not 'api_checknelems'?

And in 'lua_concat'

  lua_lock(L);
  api_checknelems(L, n);
  if (n > 0) {
+   api_checkpop(L, n - 1);
    luaV_concat(L, n);
    luaC_checkGC(L);
  }
  ...

Or am I missing something?

-- Jure

[Roberto Ierusalimschy]

> In regards to commit 'bc4bbce' that fixes the issue with 'luaL_newmetatable'
> in auxiliary library.
>

> The creation of *file* metatable in Input/output standard library faces the

> same
> problem in case of OOM.
>
> liolib.c:797 the function 'createmeta'
>
> static void createmeta (lua_State *L) {
> luaL_newmetatable(L, LUA_FILEHANDLE); /* metatable for file handles */
> luaL_setfuncs(L, metameth, 0); /* add metamethods to new metatable */
> luaL_newlibtable(L, meth); /* create method table */
> luaL_setfuncs(L, meth, 0); /* add file methods to method table */
> lua_setfield(L, -2, "__index"); /* metatable.__index = method table */
> lua_pop(L, 1); /* pop metatable */
> }
>
> (After 'luaL_newmetatable' the table is already in the registry; the proper
> re-'require'
> of I/O library skips the creation of the file metatable.)

I don't see the problem here. If the table is already in the registry,
it doesn't need to be created again. In case of a re-'require', the
initialization (luaL_setfuncs) will be tried again, and the require will
only complete if that initialization goes without errors.

The problematic uses seem to be only when the result of luaL_newmetatable
is used to conditionally initialize the metatable.

-- Roberto

[Jure Bagić]

> The problematic uses seem to be only when the result of luaL_newmetatable
> is used to conditionally initialize the metatable.

Yeah I see that now, soz and thanks for explaining.

-- Jure

[Roberto Ierusalimschy]

That's correct, both api_checknelems should be api_checkpop. Thanks for
the feedback.

-- Roberto