AllocatorDefinition implementation improvements

7 files changed, 101 insertions, 24 deletions

diff --git a/lanes-4.0.0-0.rockspec b/lanes-4.0.0-0.rockspec
index 120cf51..dbe398a 100644
--- a/lanes-4.0.0-0.rockspec
+++ b/lanes-4.0.0-0.rockspec
@@ -61,6 +61,7 @@ build = {
                         sources =
                         {
                                 "src/_pch.cpp",
+                                "src/allocator.cpp",
                                 "src/cancel.cpp",
                                 "src/compat.cpp",
                                 "src/deep.cpp",
diff --git a/src/Makefile b/src/Makefile
index 5fc3482..f2e2eb5 100644
--- a/src/Makefile
+++ b/src/Makefile
@@ -9,7 +9,7 @@ MODULE=lanes
 
 CC= g++ -std=c++20
 
-SRC=_pch.cpp cancel.cpp compat.cpp deep.cpp intercopycontext.cpp keeper.cpp lane.cpp lanes.cpp linda.cpp lindafactory.cpp nameof.cpp state.cpp threading.cpp tools.cpp tracker.cpp universe.cpp
+SRC=_pch.cpp allocator.cpp cancel.cpp compat.cpp deep.cpp intercopycontext.cpp keeper.cpp lane.cpp lanes.cpp linda.cpp lindafactory.cpp nameof.cpp state.cpp threading.cpp tools.cpp tracker.cpp universe.cpp
 
 OBJ=$(SRC:.cpp=.o)
 
diff --git a/src/allocator.cpp b/src/allocator.cpp
new file mode 100644
index 0000000..84acde5
--- /dev/null
+++ b/src/allocator.cpp
@@ -0,0 +1,51 @@
+/*
+ * ALLOCATOR.CPP                  Copyright (c) 2017-2024, Benoit Germain
+ */
+
+/*
+===============================================================================
+
+Copyright (C) 2017-2024 Benoit Germain <bnt.germain@gmail.com>
+
+Permission is hereby granted, free of charge, to any person obtaining a copy
+of this software and associated documentation files (the "Software"), to deal
+in the Software without restriction, including without limitation the rights
+to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+copies of the Software, and to permit persons to whom the Software is
+furnished to do so, subject to the following conditions:
+
+The above copyright notice and this permission notice shall be included in
+all copies or substantial portions of the Software.
+
+THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL THE
+AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+THE SOFTWARE.
+
+===============================================================================
+*/
+
+#include "_pch.hpp"
+#include "allocator.hpp"
+
+namespace lanes
+{
+    AllocatorDefinition& AllocatorDefinition::Validated(lua_State* const L_, StackIndex const idx_)
+    {
+        lanes::AllocatorDefinition* const _def{ luaG_tofulluserdata<lanes::AllocatorDefinition>(L_, idx_) };
+        // raise an error and don't return if the full userdata at the specified index is not a valid AllocatorDefinition
+        if (!_def) {
+            raise_luaL_error(L_, "Bad config.allocator function, provided value is not a userdata");
+        }
+        if (lua_rawlen(L_, idx_) < sizeof(lanes::AllocatorDefinition)) {
+            raise_luaL_error(L_, "Bad config.allocator function, provided value is too small to contain a valid AllocatorDefinition");
+        }
+        if (_def->version != kAllocatorVersion) {
+            raise_luaL_error(L_, "Bad config.allocator function, AllocatorDefinition version mismatch");
+        }
+        return *_def;
+    }
+}
diff --git a/src/allocator.hpp b/src/allocator.hpp
index fa48c46..4fec044 100644
--- a/src/allocator.hpp
+++ b/src/allocator.hpp
@@ -9,22 +9,30 @@ namespace lanes {
     // everything we need to provide to lua_newstate()
     class AllocatorDefinition
     {
-        public:
+        private:
         // xxh64 of string "kAllocatorVersion_1" generated at https://www.pelock.com/products/hash-calculator
-        static constexpr uintptr_t kAllocatorVersion{ static_cast<uintptr_t>(0xCF9D321B0DFB5715ull) };
-        uintptr_t version{ kAllocatorVersion };
+        static constexpr auto kAllocatorVersion{ static_cast<uintptr_t>(0xCF9D321B0DFB5715ull) };
+
+        public:
+        using version_t = std::remove_const_t<decltype(kAllocatorVersion)>;
+
+        private:
+        // can't make these members const because we need to be able to copy an AllocatorDefinition into another
+        // so the members are not const, but they are private to avoid accidents.
+        version_t version{ kAllocatorVersion };
         lua_Alloc allocF{ nullptr };
         void* allocUD{ nullptr };
 
+        public:
+
         [[nodiscard]] static void* operator new(size_t const size_) noexcept = delete; // can't create one outside of a Lua state
         [[nodiscard]] static void* operator new(size_t const size_, lua_State* const L_) noexcept { return lua_newuserdatauv(L_, size_, UserValueCount{ 0 }); }
         // always embedded somewhere else or "in-place constructed" as a full userdata
         // can't actually delete the operator because the compiler generates stack unwinding code that could call it in case of exception
         static void operator delete([[maybe_unused]] void* const p_, [[maybe_unused]] lua_State* const L_) {}
 
-        AllocatorDefinition(uintptr_t const version_, lua_Alloc const allocF_, void* const allocUD_) noexcept
-        : version{ version_ }
-        , allocF{ allocF_ }
+        AllocatorDefinition(lua_Alloc const allocF_, void* const allocUD_) noexcept
+        : allocF{ allocF_ }
         , allocUD{ allocUD_ }
         {
         }
@@ -36,16 +44,35 @@ namespace lanes {
         AllocatorDefinition& operator=(AllocatorDefinition const& rhs_) = default;
         AllocatorDefinition& operator=(AllocatorDefinition&& rhs_) = default;
 
+        static AllocatorDefinition& Validated(lua_State* L_, StackIndex idx_);
+
         void initFrom(lua_State* const L_)
         {
             allocF = lua_getallocf(L_, &allocUD);
         }
 
+        void installIn(lua_State* const L_) const
+        {
+            if (allocF) {
+                lua_setallocf(L_, allocF, allocUD);
+            }
+        }
+
+        lua_State* newState() const
+        {
+            return lua_newstate(allocF, allocUD);
+        }
+
         void* alloc(size_t const nsize_)
         {
             return allocF(allocUD, nullptr, 0, nsize_);
         }
 
+        void* alloc(void* const ptr_, size_t const osize_, size_t const nsize_)
+        {
+            return allocF(allocUD, ptr_, osize_, nsize_);
+        }
+
         void free(void* const ptr_, size_t const osize_)
         {
             std::ignore = allocF(allocUD, ptr_, osize_, 0);
diff --git a/src/state.cpp b/src/state.cpp
index 6c49694..88f406a 100644
--- a/src/state.cpp
+++ b/src/state.cpp
@@ -159,7 +159,7 @@ namespace state {
                         return luaL_newstate();
                     } else {
                         lanes::AllocatorDefinition const _def{ U->resolveAllocator(from, hint) };
-                        return lua_newstate(_def.allocF, _def.allocUD);
+                        return _def.newState();
                     }
                 }
             )
diff --git a/src/universe.cpp b/src/universe.cpp
index 357aa08..283747f 100644
--- a/src/universe.cpp
+++ b/src/universe.cpp
@@ -263,7 +263,7 @@ void Universe::initializeAllocatorFunction(lua_State* const L_)
     LUA_ASSERT(L_, lua_isstring(L_, kIdxTop)); // should be the case due to lanes.lua parameter validation
     std::string_view const _allocator{ luaG_tostring(L_, kIdxTop) };
     if (_allocator == "libc") {
-        internalAllocator = lanes::AllocatorDefinition{ lanes::AllocatorDefinition::kAllocatorVersion, libc_lua_Alloc, nullptr };
+        internalAllocator = lanes::AllocatorDefinition{ libc_lua_Alloc, nullptr };
     } else {
         // use whatever the provider provides
         internalAllocator = resolveAllocator(L_, "internal");
@@ -338,11 +338,8 @@ lanes::AllocatorDefinition Universe::resolveAllocator(lua_State* const L_, std::
     lua_pushcclosure(L_, provideAllocator, 0);                                                     // L_: provideAllocator()
     luaG_pushstring(L_, hint_);                                                                    // L_: provideAllocator() "<hint>"
     lua_call(L_, 1, 1);                                                                            // L_: result
-    lanes::AllocatorDefinition* const _def{ luaG_tofulluserdata<lanes::AllocatorDefinition>(L_, kIdxTop) };
-    if (!_def || _def->version != lanes::AllocatorDefinition::kAllocatorVersion) {
-        raise_luaL_error(L_, "Bad config.allocator function, must provide a valid AllocatorDefinition");
-    }
-    _ret = *_def;
+    // make sure we have a valid AllocatorDefinition on the stack (an error is raised instead if it is not the case)
+    _ret = lanes::AllocatorDefinition::Validated(L_, kIdxTop);
     lua_pop(L_, 1);                                                                                // L_:
     STACK_CHECK(L_, 0);
     return _ret;
diff --git a/src/universe.hpp b/src/universe.hpp
index a45ce86..77fbb52 100644
--- a/src/universe.hpp
+++ b/src/universe.hpp
@@ -20,13 +20,16 @@ class ProtectedAllocator
 : public lanes::AllocatorDefinition
 {
     private:
+
+    using super = lanes::AllocatorDefinition;
+
     std::mutex mutex;
 
     [[nodiscard]] static void* protected_lua_Alloc(void* ud_, void* ptr_, size_t osize_, size_t nsize_)
     {
         ProtectedAllocator* const allocator{ static_cast<ProtectedAllocator*>(ud_) };
         std::lock_guard<std::mutex> guard{ allocator->mutex };
-        return allocator->allocF(allocator->allocUD, ptr_, osize_, nsize_);
+        return allocator->alloc(ptr_, osize_, nsize_);
     }
 
     public:
@@ -36,21 +39,19 @@ class ProtectedAllocator
 
     AllocatorDefinition makeDefinition()
     {
-        return AllocatorDefinition{ version, protected_lua_Alloc, this };
+        return AllocatorDefinition{ protected_lua_Alloc, this };
     }
 
-    void installIn(lua_State* L_)
+    void installIn(lua_State* const L_) const
     {
-        lua_setallocf(L_, protected_lua_Alloc, this);
+        // install our replacement allocator function (this is a C function, we need to deconst ourselves)
+        lua_setallocf(L_, protected_lua_Alloc, static_cast<void*>(const_cast<ProtectedAllocator*>(this)));
     }
 
-    void removeFrom(lua_State* L_)
+    void removeFrom(lua_State* const L_) const
     {
-        // remove the protected allocator, if any
-        if (allocF != nullptr) {
-            // install the non-protected allocator
-            lua_setallocf(L_, allocF, allocUD);
-        }
+        // restore the base allocator function
+        super::installIn(L_);
     }
 };