Fix potential createStopWatch when given nil crash#3272
Fix potential createStopWatch when given nil crash#3272vadi2 merged 2 commits intoMudlet:release-4.4from
Conversation
Also "name as string or autostart as boolean are optional, got nil!" sounds pretty confusing to me, so I've edited out the optional part, like with other messages. If it's optional, then don't complain that you got nil...
|
Hey there! Thanks for helping Mudlet improve. 🌟 Test versionsYou can directly test the changes here:
No need to install anything - just unzip and run. |
SlySven
left a comment
There was a problem hiding this comment.
Also "name as string or autostart as boolean are optional, got nil!" sounds pretty confusing to me, so I've edited out the optional part, like with other messages. If it's optional, then don't complain that you got nil...
If the only argument provided to a function is a nil then I believed Lua would collapse / discard that argument - so then n would become zero and that extra test would never be reached - or have I got that rong?
I agree that missing an argument specified in the printf format string to the lua_pushfstring(...) error messages is a bug and needs fixing...
Yes, that is not the case for the C API - it differentiates between 'nill' and 'none'. It's the Lua side of the API that will discard it. |
SlySven
left a comment
There was a problem hiding this comment.
Can you document in the comment that the C API doesn't ignore a nil explicitly passed as an argument - I wasn't aware of that behaviour (c.f. the Lua side that does for trailing arguments of that type) and will need to watch for that in other cases where we (I) check for the argument type with a lua_type(lua_State*, int) call...
|
Sure thing! |
* Fix createStopWatch(nil) potentially crashing Mudlet Also "name as string or autostart as boolean are optional, got nil!" sounds pretty confusing to me, so I've edited out the optional part, like with other messages. If it's optional, then don't complain that you got nil... * Add note per review
| // note that 'nil' will still count towards the stack's gettop amount | ||
| } else { | ||
| lua_pushfstring(L, "createStopWatch: bad argument #%d type (name as string or autostart as boolean are optional, got %s!)", luaL_typename(L, s)); | ||
| lua_pushfstring(L, "createStopWatch: bad argument #%d type (name as string or autostart as boolean are optional, got %s!)", s, luaL_typename(L, s)); |
There was a problem hiding this comment.
Ouch! 🌳 🥄 award for me there and below for not providing that int...
Brief overview of PR changes/additions
Fix createStopWatch crash because not all variables were passed to the printf-like function.
Motivation for adding to Mudlet
Mudlet should never crash!
Other info (issues closed, discussion etc)
Also "name as string or autostart as boolean are optional, got nil!" sounds pretty confusing to me, so I've edited out the optional part, like with other messages. If it's optional, then don't complain that you got nil...