Neopallium / lua-zmq

Lua zeromq2 binding
http://github.com/Neopallium/lua-zmq
MIT License
153 stars 36 forks source link

Global variable accesses in ffi interface #7

Closed agladysh closed 13 years ago

agladysh commented 13 years ago

By analyzing luac -l output, I found one variable that is regularly accessed via {S,G}ETGLOBAL in lua-zmq FFI bindings — ud_obj:

    31  [88]    SETGLOBAL   4 -8    ; ud_obj
    35  [89]    GETGLOBAL   6 -8    ; ud_obj
    39  [94]    GETGLOBAL   5 -8    ; ud_obj
    8   [161]   SETGLOBAL   3 -2    ; ud_obj
    12  [162]   GETGLOBAL   5 -2    ; ud_obj
    20  [166]   GETGLOBAL   4 -2    ; ud_obj

Number in square brackets is line number in zmq.dump_ffi() output.

Note that this may be a false positive if you prepend some kind of header with local ud_obj declaration before loading this code.

Also, a small nitpicking observation: there is a lot of non-aliased global variables used (see full dump below). It is a better style to alias all this stuff to locals on the top of the file — this really helps if code is being run inside sandboxed environment. (Not that I plan to do so with zmq right now, but anyway.)

$ luajit2 -lzmq -e 'io.stdout:write(zmq:dump_ffi())' | tee zmq_ffi_dump.lua | luac -l -o /dev/null - | grep ETGLOBAL
    1   [2] GETGLOBAL   0 -1    ; pcall
    2   [2] GETGLOBAL   1 -2    ; require
    9   [9] GETGLOBAL   5 -4    ; bit
    11  [10]    GETGLOBAL   6 -6    ; debug
    13  [11]    GETGLOBAL   7 -6    ; debug
    23  [48]    GETGLOBAL   13 -8   ; setmetatable
    107 [390]   GETGLOBAL   37 -16  ; pairs
    111 [391]   GETGLOBAL   42 -17  ; type
    124 [396]   GETGLOBAL   37 -16  ; pairs
    228 [632]   GETGLOBAL   44 -20  ; zmq
    231 [633]   GETGLOBAL   44 -20  ; zmq
    234 [634]   GETGLOBAL   44 -20  ; zmq
    237 [635]   GETGLOBAL   44 -20  ; zmq
    240 [636]   GETGLOBAL   44 -20  ; zmq
    243 [637]   GETGLOBAL   44 -20  ; zmq
    246 [638]   GETGLOBAL   44 -20  ; zmq
    249 [639]   GETGLOBAL   44 -20  ; zmq
    252 [640]   GETGLOBAL   44 -20  ; zmq
    255 [641]   GETGLOBAL   44 -20  ; zmq
    258 [642]   GETGLOBAL   44 -20  ; zmq
    261 [643]   GETGLOBAL   44 -20  ; zmq
    264 [644]   GETGLOBAL   44 -20  ; zmq
    267 [645]   GETGLOBAL   44 -20  ; zmq
    270 [646]   GETGLOBAL   44 -20  ; zmq
    273 [647]   GETGLOBAL   44 -20  ; zmq
    276 [648]   GETGLOBAL   44 -20  ; zmq
    279 [649]   GETGLOBAL   44 -20  ; zmq
    284 [653]   GETGLOBAL   46 -16  ; pairs
    12  [56]    GETGLOBAL   3 -3    ; error
    16  [56]    GETGLOBAL   7 -7    ; type
    23  [84]    SETGLOBAL   6 -7    ; type
    31  [88]    SETGLOBAL   4 -8    ; ud_obj
    35  [89]    GETGLOBAL   6 -8    ; ud_obj
    39  [94]    GETGLOBAL   5 -8    ; ud_obj
    13  [106]   GETGLOBAL   5 -5    ; tonumber
    23  [120]   SETGLOBAL   6 -7    ; type
    24  [124]   GETGLOBAL   4 -8    ; tonumber
    12  [147]   GETGLOBAL   3 -3    ; error
    16  [147]   GETGLOBAL   7 -7    ; type
    8   [161]   SETGLOBAL   3 -2    ; ud_obj
    12  [162]   GETGLOBAL   5 -2    ; ud_obj
    20  [166]   GETGLOBAL   4 -2    ; ud_obj
    3   [266]   GETGLOBAL   1 -2    ; setmetatable
    3   [298]   GETGLOBAL   1 -2    ; setmetatable
    3   [330]   GETGLOBAL   1 -2    ; setmetatable
    3   [362]   GETGLOBAL   1 -2    ; setmetatable
    24  [503]   GETGLOBAL   5 -5    ; error
    23  [540]   GETGLOBAL   4 -5    ; error
    11  [670]   GETGLOBAL   8 -3    ; tostring
    50  [715]   GETGLOBAL   7 -8    ; tonumber
    40  [900]   GETGLOBAL   6 -10   ; tonumber
    1   [984]   GETGLOBAL   2 -1    ; type
    17  [990]   GETGLOBAL   3 -6    ; error
agladysh commented 13 years ago

Also, I find these accesses to zmq constant fields a bit strange — given that require 'zmq' is nowhere to be found in this code. This probably works due to some implementation details in your FFI bindings generator, but I would at least leave a comment there, describing what is going on.

Neopallium commented 13 years ago

Fixed incorrect use of global for ud_obj variable. Also localized some of the other global access that needed it. Most of those global accesses are only during the loading of the zmq FFI bindings and there is no need to localize them at the top of the file.

The FFI bindings over-write part of the normal Lua C API bindings, so those constants are define in the C code before the FFI bindings are loaded.

agladysh commented 13 years ago

Thank you, fix confirmed!