From 2f25257a3e1ed8a91328bd1e33cdb30710865628 Mon Sep 17 00:00:00 2001 From: Petr Mikheev Date: Sun, 19 Sep 2021 14:38:27 +0200 Subject: [PATCH] Move LuaState::makeReadOnly(sol::table) out of the class because it doesn't need to access LuaState internals. --- apps/openmw/mwlua/camerabindings.cpp | 2 +- apps/openmw/mwlua/inputbindings.cpp | 10 +++++----- apps/openmw/mwlua/luabindings.cpp | 18 ++++++++--------- apps/openmw/mwlua/settingsbindings.cpp | 2 +- apps/openmw/mwlua/uibindings.cpp | 2 +- apps/openmw_test_suite/lua/test_lua.cpp | 6 +++--- components/lua/luastate.cpp | 26 ++++++++++++++----------- components/lua/luastate.hpp | 11 +++++------ components/lua/scriptscontainer.cpp | 6 +++--- components/lua/scriptscontainer.hpp | 2 +- 10 files changed, 44 insertions(+), 41 deletions(-) diff --git a/apps/openmw/mwlua/camerabindings.cpp b/apps/openmw/mwlua/camerabindings.cpp index 68f2331b9e..46bf079d65 100644 --- a/apps/openmw/mwlua/camerabindings.cpp +++ b/apps/openmw/mwlua/camerabindings.cpp @@ -7,7 +7,7 @@ namespace MWLua { sol::table api(context.mLua->sol(), sol::create); // TODO - return context.mLua->makeReadOnly(api); + return LuaUtil::makeReadOnly(api); } } diff --git a/apps/openmw/mwlua/inputbindings.cpp b/apps/openmw/mwlua/inputbindings.cpp index f815d9c344..aaa00f3da9 100644 --- a/apps/openmw/mwlua/inputbindings.cpp +++ b/apps/openmw/mwlua/inputbindings.cpp @@ -48,7 +48,7 @@ namespace MWLua api["getControlSwitch"] = [input](const std::string& key) { return input->getControlSwitch(key); }; api["setControlSwitch"] = [input](const std::string& key, bool v) { input->toggleControlSwitch(key, v); }; - api["ACTION"] = context.mLua->makeReadOnly(context.mLua->sol().create_table_with( + api["ACTION"] = LuaUtil::makeReadOnly(context.mLua->sol().create_table_with( "GameMenu", MWInput::A_GameMenu, "Screenshot", MWInput::A_Screenshot, "Inventory", MWInput::A_Inventory, @@ -102,7 +102,7 @@ namespace MWLua "ZoomOut", MWInput::A_ZoomOut )); - api["CONTROL_SWITCH"] = context.mLua->makeReadOnly(context.mLua->sol().create_table_with( + api["CONTROL_SWITCH"] = LuaUtil::makeReadOnly(context.mLua->sol().create_table_with( "Controls", "playercontrols", "Fighting", "playerfighting", "Jumping", "playerjumping", @@ -112,7 +112,7 @@ namespace MWLua "VanityMode", "vanitymode" )); - api["CONTROLLER_BUTTON"] = context.mLua->makeReadOnly(context.mLua->sol().create_table_with( + api["CONTROLLER_BUTTON"] = LuaUtil::makeReadOnly(context.mLua->sol().create_table_with( "A", SDL_CONTROLLER_BUTTON_A, "B", SDL_CONTROLLER_BUTTON_B, "X", SDL_CONTROLLER_BUTTON_X, @@ -130,7 +130,7 @@ namespace MWLua "DPadRight", SDL_CONTROLLER_BUTTON_DPAD_RIGHT )); - api["CONTROLLER_AXIS"] = context.mLua->makeReadOnly(context.mLua->sol().create_table_with( + api["CONTROLLER_AXIS"] = LuaUtil::makeReadOnly(context.mLua->sol().create_table_with( "LeftX", SDL_CONTROLLER_AXIS_LEFTX, "LeftY", SDL_CONTROLLER_AXIS_LEFTY, "RightX", SDL_CONTROLLER_AXIS_RIGHTX, @@ -144,7 +144,7 @@ namespace MWLua "MoveLeftRight", SDL_CONTROLLER_AXIS_MAX + MWInput::A_MoveLeftRight )); - return context.mLua->makeReadOnly(api); + return LuaUtil::makeReadOnly(api); } } diff --git a/apps/openmw/mwlua/luabindings.cpp b/apps/openmw/mwlua/luabindings.cpp index ed788eb525..e37241d692 100644 --- a/apps/openmw/mwlua/luabindings.cpp +++ b/apps/openmw/mwlua/luabindings.cpp @@ -18,7 +18,7 @@ namespace MWLua sol::table res(lua.sol(), sol::create); for (const std::string& v : values) res[v] = v; - return lua.makeReadOnly(res); + return LuaUtil::makeReadOnly(res); } sol::table initCorePackage(const Context& context) @@ -43,7 +43,7 @@ namespace MWLua "Activator", "Armor", "Book", "Clothing", "Creature", "Door", "Ingredient", "Light", "Miscellaneous", "NPC", "Player", "Potion", "Static", "Weapon" }); - api["EQUIPMENT_SLOT"] = lua->makeReadOnly(lua->sol().create_table_with( + api["EQUIPMENT_SLOT"] = LuaUtil::makeReadOnly(lua->sol().create_table_with( "Helmet", MWWorld::InventoryStore::Slot_Helmet, "Cuirass", MWWorld::InventoryStore::Slot_Cuirass, "Greaves", MWWorld::InventoryStore::Slot_Greaves, @@ -64,7 +64,7 @@ namespace MWLua "CarriedLeft", MWWorld::InventoryStore::Slot_CarriedLeft, "Ammunition", MWWorld::InventoryStore::Slot_Ammunition )); - return lua->makeReadOnly(api); + return LuaUtil::makeReadOnly(api); } sol::table initWorldPackage(const Context& context) @@ -107,7 +107,7 @@ namespace MWLua // return GObjectList{worldView->selectObjects(query, false)}; }; // TODO: add world.placeNewObject(recordId, cell, pos, [rot]) - return context.mLua->makeReadOnly(api); + return LuaUtil::makeReadOnly(api); } sol::table initNearbyPackage(const Context& context) @@ -137,7 +137,7 @@ namespace MWLua // TODO: Maybe use sqlite // return LObjectList{worldView->selectObjects(query, true)}; }; - return context.mLua->makeReadOnly(api); + return LuaUtil::makeReadOnly(api); } sol::table initQueryPackage(const Context& context) @@ -148,7 +148,7 @@ namespace MWLua query[t] = Queries::Query(std::string(t)); for (const QueryFieldGroup& group : getBasicQueryFieldGroups()) query[group.mName] = initFieldGroup(context, group); - return query; // makeReadonly is applied by LuaState::addCommonPackage + return query; // makeReadOnly is applied by LuaState::addCommonPackage } sol::table initFieldGroup(const Context& context, const QueryFieldGroup& group) @@ -163,12 +163,12 @@ namespace MWLua { const std::string& name = field->path()[i]; if (subgroup[name] == sol::nil) - subgroup[name] = context.mLua->makeReadOnly(context.mLua->newTable()); - subgroup = context.mLua->getMutableFromReadOnly(subgroup[name]); + subgroup[name] = LuaUtil::makeReadOnly(context.mLua->newTable()); + subgroup = LuaUtil::getMutableFromReadOnly(subgroup[name]); } subgroup[field->path().back()] = field; } - return context.mLua->makeReadOnly(res); + return LuaUtil::makeReadOnly(res); } } diff --git a/apps/openmw/mwlua/settingsbindings.cpp b/apps/openmw/mwlua/settingsbindings.cpp index 0e267182ac..12dd69f73a 100644 --- a/apps/openmw/mwlua/settingsbindings.cpp +++ b/apps/openmw/mwlua/settingsbindings.cpp @@ -62,7 +62,7 @@ namespace MWLua else return sol::make_object(lua->sol(), value.getFloat()); }; - return lua->makeReadOnly(config); + return LuaUtil::makeReadOnly(config); } sol::table initGlobalSettingsPackage(const Context& context) { return initSettingsPackage(context, true, false); } diff --git a/apps/openmw/mwlua/uibindings.cpp b/apps/openmw/mwlua/uibindings.cpp index cb14c41621..4fae84cd40 100644 --- a/apps/openmw/mwlua/uibindings.cpp +++ b/apps/openmw/mwlua/uibindings.cpp @@ -12,7 +12,7 @@ namespace MWLua { luaManager->addUIMessage(message); }; - return context.mLua->makeReadOnly(api); + return LuaUtil::makeReadOnly(api); } } diff --git a/apps/openmw_test_suite/lua/test_lua.cpp b/apps/openmw_test_suite/lua/test_lua.cpp index 9405dba4b2..32d4ea49b8 100644 --- a/apps/openmw_test_suite/lua/test_lua.cpp +++ b/apps/openmw_test_suite/lua/test_lua.cpp @@ -119,7 +119,7 @@ return { EXPECT_ERROR(LuaUtil::call(script["rawsetSystemLib"]), "bad argument #1 to 'rawset' (table expected, got userdata)"); EXPECT_ERROR(LuaUtil::call(script["modifySystemLib"]), "a userdata value"); - EXPECT_EQ(mLua.getMutableFromReadOnly(mLua.makeReadOnly(script)), script); + EXPECT_EQ(LuaUtil::getMutableFromReadOnly(LuaUtil::makeReadOnly(script)), script); } TEST_F(LuaStateTest, Print) @@ -150,8 +150,8 @@ return { { LuaUtil::LuaState lua(mVFS.get()); - sol::table api1 = lua.makeReadOnly(lua.sol().create_table_with("name", "api1")); - sol::table api2 = lua.makeReadOnly(lua.sol().create_table_with("name", "api2")); + sol::table api1 = LuaUtil::makeReadOnly(lua.sol().create_table_with("name", "api1")); + sol::table api2 = LuaUtil::makeReadOnly(lua.sol().create_table_with("name", "api2")); sol::table script1 = lua.runInNewSandbox("bbb/tests.lua", "", {{"test.api", api1}}); diff --git a/components/lua/luastate.cpp b/components/lua/luastate.cpp index fd42aa7fb2..8e4719dba4 100644 --- a/components/lua/luastate.cpp +++ b/components/lua/luastate.cpp @@ -67,27 +67,31 @@ namespace LuaUtil mSandboxEnv = sol::nil; } - sol::table LuaState::makeReadOnly(sol::table table) + sol::table makeReadOnly(sol::table table) { + if (table == sol::nil) + return table; if (table.is()) return table; // it is already userdata, no sense to wrap it again + lua_State* lua = table.lua_state(); table[sol::meta_function::index] = table; - sol::stack::push(mLua, std::move(table)); - lua_newuserdata(mLua, 0); - lua_pushvalue(mLua, -2); - lua_setmetatable(mLua, -2); - return sol::stack::pop(mLua); + sol::stack::push(lua, std::move(table)); + lua_newuserdata(lua, 0); + lua_pushvalue(lua, -2); + lua_setmetatable(lua, -2); + return sol::stack::pop(lua); } - sol::table LuaState::getMutableFromReadOnly(const sol::userdata& ro) + sol::table getMutableFromReadOnly(const sol::userdata& ro) { - sol::stack::push(mLua, ro); - int ok = lua_getmetatable(mLua, -1); + lua_State* lua = ro.lua_state(); + sol::stack::push(lua, ro); + int ok = lua_getmetatable(lua, -1); assert(ok); (void)ok; - sol::table res = sol::stack::pop(mLua); - lua_pop(mLua, 1); + sol::table res = sol::stack::pop(lua); + lua_pop(lua, 1); return res; } diff --git a/components/lua/luastate.hpp b/components/lua/luastate.hpp index acaaadea76..8982b49b36 100644 --- a/components/lua/luastate.hpp +++ b/components/lua/luastate.hpp @@ -3,7 +3,6 @@ #include -#include // missing from sol/sol.hpp #include #include @@ -37,11 +36,6 @@ namespace LuaUtil // A shortcut to create a new Lua table. sol::table newTable() { return sol::table(mLua, sol::create); } - // Makes a table read only (when accessed from Lua) by wrapping it with an empty userdata. - // Needed to forbid any changes in common resources that can accessed from different sandboxes. - sol::table makeReadOnly(sol::table); - sol::table getMutableFromReadOnly(const sol::userdata&); - // Registers a package that will be available from every sandbox via `require(name)`. // The package can be either a sol::table with an API or a sol::function. If it is a function, // it will be evaluated (once per sandbox) the first time when requested. If the package @@ -106,6 +100,11 @@ namespace LuaUtil // String representation of a Lua object. Should be used for debugging/logging purposes only. std::string toString(const sol::object&); + // Makes a table read only (when accessed from Lua) by wrapping it with an empty userdata. + // Needed to forbid any changes in common resources that can accessed from different sandboxes. + sol::table makeReadOnly(sol::table); + sol::table getMutableFromReadOnly(const sol::userdata&); + } #endif // COMPONENTS_LUA_LUASTATE_H diff --git a/components/lua/scriptscontainer.cpp b/components/lua/scriptscontainer.cpp index a53b1d0404..6e0a19b120 100644 --- a/components/lua/scriptscontainer.cpp +++ b/components/lua/scriptscontainer.cpp @@ -25,7 +25,7 @@ namespace LuaUtil void ScriptsContainer::addPackage(const std::string& packageName, sol::object package) { - API[packageName] = mLua.makeReadOnly(std::move(package)); + API[packageName] = makeReadOnly(std::move(package)); } bool ScriptsContainer::addNewScript(const std::string& path) @@ -63,7 +63,7 @@ namespace LuaUtil if (interfaceName.empty() != (publicInterface == sol::nil)) Log(Debug::Error) << mNamePrefix << "[" << path << "]: 'interfaceName' should always be used together with 'interface'"; else if (!interfaceName.empty()) - script.as()[INTERFACE] = mPublicInterfaces[interfaceName] = mLua.makeReadOnly(publicInterface); + script.as()[INTERFACE] = mPublicInterfaces[interfaceName] = makeReadOnly(publicInterface); mScriptOrder.push_back(path); mScripts[path].mInterface = std::move(script); return true; @@ -329,7 +329,7 @@ namespace LuaUtil mHoursTimersQueue.clear(); mPublicInterfaces.clear(); - // Assigned by mLua.makeReadOnly, but `clear` removes it, so we need to assign it again. + // Assigned by LuaUtil::makeReadOnly, but `clear` removes it, so we need to assign it again. mPublicInterfaces[sol::meta_function::index] = mPublicInterfaces; } diff --git a/components/lua/scriptscontainer.hpp b/components/lua/scriptscontainer.hpp index 7b2b2a7aa9..d3141a2ca3 100644 --- a/components/lua/scriptscontainer.hpp +++ b/components/lua/scriptscontainer.hpp @@ -76,7 +76,7 @@ namespace LuaUtil virtual ~ScriptsContainer() {} // Adds package that will be available (via `require`) for all scripts in the container. - // Automatically applies LuaState::makeReadOnly to the package. + // Automatically applies LuaUtil::makeReadOnly to the package. void addPackage(const std::string& packageName, sol::object package); // Finds a file with given path in the virtual file system, starts as a new script, and adds it to the container.