Re: Please comment on these Lua to C library bindings that I wrote

[Jerome Vuarand <jerome.vuarand@...>]

2012/11/20 Richard W.M. Jones <rjones@redhat.com>:
> This seems something of a problem with Lua itself.  How would you
> express, say, the exact size in a bytes of a disk volume?  2^53 is
> 8 petabytes, which is easily in range of what is available in high end
> storage now.  I have access to a larger disk array at work.  Not to
> mention that virtual, sparsely allocated disks can be larger even on
> my laptop.
>
> Also we use 64 bit ints as bitmasks in various places.

Well, as in any language when you want to use numbers larger than the
largest integer type, you have to use a specialized library. There are
several bignum libraries around, or you can roll your own if you have
special needs.

>> It all depends on the threading model of the library you're binding.
>> You should ensure that your callbacks are called in order and don't
>> overlap, as the Lua interpreter cannot be used simultaneously by
>> several threads. For some libraries (FUSE for example) that's an init
>> option.
>
> In libguestfs we assume that each handle is only used in a single
> thread, and callbacks are always delivered in the same thread that is
> running the function that generates them.  So I guess we should be OK?

That sounds OK.

>> > - How do you print "any" type, from C?
>>
>> There's no built-in framework for that, because for non-basic types
>> there is no trivial way to do it. There are plenty of libraries
>> around, in Lua or in C (keep in mind that if you got a lua_State
>> pointer you can call Lua code from C very easily).
>
> Is there a particular library I should be looking at?  My Google-fu
> isn't turning up anything at the moment ...
>
> The only thing I want to print is the exception which is thrown by the
> callback, which cannot be handled in any other way except printing it
> (callbacks aren't supposed to be throwing exceptions).

Well the easy solution is to use the tostring function (not
lua_tostring). It accepts any Lua value, and gives you a string. Most
thrown errors are already strings anyway, and if you throw custom
objects you can give them a __tostring metamethod that tostring will
invoke.

>> That by itself should work. How do you determine it doesn't? Do you
>> get an error?
>
> Just making that single change causes the bindings to throw errors
> like:
>
> /usr/bin/lua: /home/rjones/d/libguestfs/lua/tests/027-create-multiple.lua:26: attempt to index global 'g1' (a userdata value)
> stack traceback:
>       /home/rjones/d/libguestfs/lua/tests/027-create-multiple.lua:26: in main chunk
>       [C]: ?

The g1 variable is nil. This means either Guestfs.create returned nil,
or one of the calls on lines 22 or 23 reset g1 to nil (g1 is a global
variable, so any code can access it). By looking at lua_guestfs_create
I don't see how it could return nil. You should use a debugger and
make sure your latest version of lua_guestfs_create is properly
called, and then see why and how it returns nil.

And as for the "single change", it has a big impact. Whether you pass
a valid string or NULL, luaL_register will create a new table on the
stack or not. This means the stack content on all subsequent Lua API
calls is different. I suggest you add a comment on every line of your
luaopen_* function, where you list the content of the stack. For
example:

  luaL_newmetatable (L, LUA_GUESTFS_HANDLE); /* stack: mt */
  luaL_register (L, NULL /* "guestfs" ? XXX */, handle_methods); /* stack: mt */

  /* Set __index field of metatable to point to itself. */
  lua_pushvalue (L, -1); /* stack: mt, mt */
  lua_setfield (L, -1, "__index"); /* stack: mt */

  /* Globals in the Guestfs.* namespace. */
  lua_pushliteral (L, "event_all"); /* stack: mt, k */
  push_string_list (L, (char **) event_all); /* stack: mt, k, v */
  lua_settable (L, -3); /* stack: mt */

  /* Add _COPYRIGHT, etc. fields to the metatable. */
  lua_pushliteral (L, "_COPYRIGHT"); /* stack: mt, k */
  lua_pushliteral (L, "Copyright (C) 2009-2012 Red Hat Inc."); /*
stack: mt, k, v */
  lua_settable (L, -3); /* stack: mt */

  lua_pushliteral (L, "_DESCRIPTION"); /* stack: mt, k */
  lua_pushliteral (L, "Lua binding to libguestfs"); /* stack: mt, k, v */
  lua_settable (L, -3); /* stack: mt */

  lua_pushliteral (L, "_VERSION"); /* stack: mt, k */
  make_version_string (v, sizeof v);
  lua_pushlstring (L, v, strlen (v)); /* stack: mt, k, v */
  lua_settable (L, -3); /* stack: mt */

  /* Expose metatable to lua as "Guestfs". */
  lua_setglobal (L, "Guestfs"); /* stack: <nothing> */

  return 1; /* /!\ you return 1 value, but you have none on the stack /!\ */
}

I'll let you update all the comments when luaL_register is passed a
string rather than NULL.