Wireshark · Wireshark-dev: Re: [Wireshark-dev] Changes to internal Lua classes implementation (Pinfo, TvbRange, etc.)

[Graham Bloice <graham.bloice@xxxxxxxxxxxxx>]

On 3 October 2016 at 00:51, Peter Wu <peter@xxxxxxxxxxxxx> wrote:

Hi,

To fix bug 12968 (__gc being called for tables, resulting in lua_error
while exiting), I modified the way how classes are registered in
https://code.wireshark.org/review/18026

In these changes I also reduced the use of macros, following Joãos
concerns. Macros like WSLUA_META should most likely become typedefs.
Below I will give a motivation for the changes (feedback welcome!).


Consider class Address with function Address.ip and metamethods like
address.__tostring. Previously you could have these strange invocations:

    -- note: "Address" is the global class, "address" is an instance.
    tostring(Address)   -- invokes metamethod __tostring (error!)
    address.ip("foo")   -- invokes "static" class function
    tostring(FileHandler)   -- error
    filehandler.new(...)    -- huh?

In the proposed change, there will be different method tables and
metatables for the class ("Address") and its instances ("address").
Further modification could disable the above badness, allowing just:

    tostring(address)
    Address.ip("foo")
    tostring(filehandler)
    FileHandler.new(...)

This might break some dissectors that use non-documented invocations,
but enables use of separate __call functions for example:

    field = Field("x")  -- __call in metatable of class Field
    fields = field()    -- __call in metatable of instance of Field

Also changed is that attributes are no longer visible on the class, only
its instances:

    filehandler.read_open = x   -- ok, sets callback attribute
    -- previously failed in the setter callback out because "self" is
    -- not a FileHandler instance. Now it will already fail while
    -- looking up the attribute (in the __newindex metamethod).
    FileHandler.read_open = x



Distinct metatables are good, but what about different method tables?
The proposal will also disable invocations like Tvb.len(tvb) assuming
that nobody wants to do it (tvb:len() is saner).

Any objections with removing __setters/__getters/__methods?
I doubt that dissectors use this. Users can read the WSDG and study the
source code to discover available functions.


Future work, in order (assuming the current proposal):
 - grep for WSLUA_REGISTER_ATTRIBUTES, change their ClassName_register
   functions to use the new wslua_class definition mechanism, including
   setting the "attrs" member.
 - Remove wslua_reg_attributes and macros.
 - grep for WSLUA_REGISTER_META and WSLUA_REGISTER_CLASS to change the
   remaining ClassName_register files.

If you feel that the Lua core code needs some love, help is appreciated
in these tasks :-)
--
Kind regards,
Peter Wu

As a non-lua programmer most of that goes straight over my head, but I'm intrigued\concerned about methods that are class based (i.e. similar to C++ class static methods ??) and instance based that differ only by the case of the initial method letter.

Is this normal for Lua so won't be a surprise for users, or is it likely to surprise\confuse users and cause lots of issues and support queries?

--

Graham Bloice