search

brimworks / lua-zlib

Simple streaming interface to zlib for Lua.
273 stars 111 forks source link

Use lua_Integer instead of lua_Number for checksum calculation #53

Open jbsgh opened 3 years ago

jbsgh commented 3 years ago

We are using Lua on an embedded system with 32 bit. Accordingly, lua_Numbers are 32 bit floats not able to store 32 bit integer-checksums. I've replaced all calls like lua_pushnumber(), lua_tonumber() etc. with their integer counterparts like lua_pushinteger(), lua_tointeger etc. Besides I've added two checks in test.lua to show the correct results.

We use Lua 5.3 and 5.4 only. I didn't check for compatibility issues with older Lua versions that might not support integers correctly. If you merge this PR you may have to add conditions like #if (LUA_VERSION_NUM >= 503) to maintain compatibility with older versions.

brimworks commented 3 years ago

@hishamhm what do you think of dropping support for Lua 5.2?

@jbsgh thanks for the contribution. Let me think about this a bit before I merge it.

brimworks commented 1 year ago

Overall, this looks good, but I'd like to preserve lua 5.2 and luajit support at a minimum. If you can update your PR so that the proper #ifdefs are in place such that it works for these older version of Lua then I'll accept it.

Thanks!

hishamhm commented 1 year ago

Lua 5.1 does have lua_tointeger and lua_pushinteger. It just doesn't have lua_isinteger... Maybe that could be changed back to plain lua_isnumber in this PR (that looks like just a sanity check) and then we'd have support for all Lua versions without ifdefs?

jbsgh commented 1 year ago

lua_isnumber will not work correctly with 32-bit operations, as I stated in my OP.