Merge pull request #12 from dermojo/allocator-fix

Allocator fix

Author: dermojo

CMakeLists.txt

@@ -63,7 +63,11 @@ string(TOLOWER "${CMAKE_BUILD_TOOL}" MY_BUILDTOOL)
 if(MY_BUILDTOOL MATCHES "(msdev|devenv|nmake|msbuild)")
     target_compile_options(testlib PUBLIC /W4 /WX)
 else()
-    target_compile_options(testlib PUBLIC -Wall -Weffc++ -Werror)
+    target_compile_options(testlib PUBLIC -Wall -Weffc++ -Werror -Wextra -pedantic
+                                          -Wunused -Wredundant-decls  -Wunreachable-code
+                                          -Wold-style-cast -Wshadow
+                                          -Wconversion -Wsign-conversion  -Wno-conversion-null
+                                          -Wcast-align)
     # These are enabled on Clang and don't help...
     target_compile_options(testlib PUBLIC -Wno-missing-braces)
     target_link_libraries(testlib pthread)

include/spsl/hash.hpp

@@ -76,12 +76,12 @@ inline uint64_t rotl64(uint64_t x, int8_t r)
 // Block read - if your platform needs to do endian-swapping or can only
 // handle aligned reads, do the conversion here
 
-FORCE_INLINE uint32_t getblock32(const uint32_t* p, int i)
+FORCE_INLINE uint32_t getblock32(const uint32_t* p, uint32_t i)
 {
     return p[i];
 }
 
-FORCE_INLINE uint64_t getblock64(const uint64_t* p, int i)
+FORCE_INLINE uint64_t getblock64(const uint64_t* p, std::size_t i)
 {
     return p[i];
 }
@@ -115,10 +115,10 @@ FORCE_INLINE uint64_t fmix64(uint64_t k)
 
 //-----------------------------------------------------------------------------
 
-inline void MurmurHash3_x86_32(const void* key, int len, uint32_t seed, void* out)
+inline void MurmurHash3_x86_32(const void* key, const uint32_t len, uint32_t seed, void* out)
 {
-    const uint8_t* data = (const uint8_t*)key;
-    const int nblocks = len / 4;
+    const uint8_t* data = reinterpret_cast<const uint8_t*>(key);
+    const uint32_t nblocks = len / 4;
 
     uint32_t h1 = seed;
 
@@ -128,9 +128,9 @@ inline void MurmurHash3_x86_32(const void* key, int len, uint32_t seed, void* ou
     //----------
     // body
 
-    const uint32_t* blocks = (const uint32_t*)(data + nblocks * 4);
+    const uint32_t* blocks = reinterpret_cast<const uint32_t*>(data);
 
-    for (int i = -nblocks; i; i++)
+    for (uint32_t i = 0; i < nblocks; ++i)
     {
         uint32_t k1 = getblock32(blocks, i);
 
@@ -146,17 +146,17 @@ inline void MurmurHash3_x86_32(const void* key, int len, uint32_t seed, void* ou
     //----------
     // tail
 
-    const uint8_t* tail = (const uint8_t*)(data + nblocks * 4);
+    const uint8_t* tail = (data + nblocks * 4);
 
     uint32_t k1 = 0;
 
     switch (len & 3)
     {
     case 3:
-        k1 ^= tail[2] << 16;
+        k1 ^= static_cast<uint32_t>(tail[2] << 16);
     /* no break */
     case 2:
-        k1 ^= tail[1] << 8;
+        k1 ^= static_cast<uint32_t>(tail[1] << 8);
     /* no break */
     case 1:
         k1 ^= tail[0];
@@ -173,15 +173,16 @@ inline void MurmurHash3_x86_32(const void* key, int len, uint32_t seed, void* ou
 
     h1 = fmix32(h1);
 
-    *(uint32_t*)out = h1;
+    *static_cast<uint32_t*>(out) = h1;
 }
 
 //-----------------------------------------------------------------------------
 
-inline void MurmurHash3_x64_128(const void* key, const int len, const uint32_t seed, void* out)
+inline void MurmurHash3_x64_128(const void* key, const std::size_t len, const uint32_t seed,
+                                void* out)
 {
-    const uint8_t* data = (const uint8_t*)key;
-    const int nblocks = len / 16;
+    const uint8_t* data = reinterpret_cast<const uint8_t*>(key);
+    const std::size_t nblocks = len / 16;
 
     uint64_t h1 = seed;
     uint64_t h2 = seed;
@@ -192,9 +193,9 @@ inline void MurmurHash3_x64_128(const void* key, const int len, const uint32_t s
     //----------
     // body
 
-    const uint64_t* blocks = (const uint64_t*)(data);
+    const uint64_t* blocks = reinterpret_cast<const uint64_t*>(data);
 
-    for (int i = 0; i < nblocks; i++)
+    for (std::size_t i = 0; i < nblocks; i++)
     {
         uint64_t k1 = getblock64(blocks, i * 2 + 0);
         uint64_t k2 = getblock64(blocks, i * 2 + 1);
@@ -221,62 +222,62 @@ inline void MurmurHash3_x64_128(const void* key, const int len, const uint32_t s
     //----------
     // tail
 
-    const uint8_t* tail = (const uint8_t*)(data + nblocks * 16);
+    const uint8_t* tail = (data + nblocks * 16);
 
     uint64_t k1 = 0;
     uint64_t k2 = 0;
 
     switch (len & 15)
     {
     case 15:
-        k2 ^= ((uint64_t)tail[14]) << 48;
+        k2 ^= (static_cast<uint64_t>(tail[14])) << 48;
     /* no break */
     case 14:
-        k2 ^= ((uint64_t)tail[13]) << 40;
+        k2 ^= (static_cast<uint64_t>(tail[13])) << 40;
     /* no break */
     case 13:
-        k2 ^= ((uint64_t)tail[12]) << 32;
+        k2 ^= (static_cast<uint64_t>(tail[12])) << 32;
     /* no break */
     case 12:
-        k2 ^= ((uint64_t)tail[11]) << 24;
+        k2 ^= (static_cast<uint64_t>(tail[11])) << 24;
     /* no break */
     case 11:
-        k2 ^= ((uint64_t)tail[10]) << 16;
+        k2 ^= (static_cast<uint64_t>(tail[10])) << 16;
     /* no break */
     case 10:
-        k2 ^= ((uint64_t)tail[9]) << 8;
+        k2 ^= (static_cast<uint64_t>(tail[9])) << 8;
     /* no break */
     case 9:
-        k2 ^= ((uint64_t)tail[8]) << 0;
+        k2 ^= (static_cast<uint64_t>(tail[8])) << 0;
         k2 *= c2;
         k2 = ROTL64(k2, 33);
         k2 *= c1;
         h2 ^= k2;
     /* no break */
 
     case 8:
-        k1 ^= ((uint64_t)tail[7]) << 56;
+        k1 ^= (static_cast<uint64_t>(tail[7])) << 56;
     /* no break */
     case 7:
-        k1 ^= ((uint64_t)tail[6]) << 48;
+        k1 ^= (static_cast<uint64_t>(tail[6])) << 48;
     /* no break */
     case 6:
-        k1 ^= ((uint64_t)tail[5]) << 40;
+        k1 ^= (static_cast<uint64_t>(tail[5])) << 40;
     /* no break */
     case 5:
-        k1 ^= ((uint64_t)tail[4]) << 32;
+        k1 ^= (static_cast<uint64_t>(tail[4])) << 32;
     /* no break */
     case 4:
-        k1 ^= ((uint64_t)tail[3]) << 24;
+        k1 ^= (static_cast<uint64_t>(tail[3])) << 24;
     /* no break */
     case 3:
-        k1 ^= ((uint64_t)tail[2]) << 16;
+        k1 ^= (static_cast<uint64_t>(tail[2])) << 16;
     /* no break */
     case 2:
-        k1 ^= ((uint64_t)tail[1]) << 8;
+        k1 ^= (static_cast<uint64_t>(tail[1])) << 8;
     /* no break */
     case 1:
-        k1 ^= ((uint64_t)tail[0]) << 0;
+        k1 ^= (static_cast<uint64_t>(tail[0])) << 0;
         k1 *= c1;
         k1 = ROTL64(k1, 31);
         k1 *= c2;
@@ -298,8 +299,8 @@ inline void MurmurHash3_x64_128(const void* key, const int len, const uint32_t s
     h1 += h2;
     h2 += h1;
 
-    ((uint64_t*)out)[0] = h1;
-    ((uint64_t*)out)[1] = h2;
+    static_cast<uint64_t*>(out)[0] = h1;
+    static_cast<uint64_t*>(out)[1] = h2;
 }
 
 //-----------------------------------------------------------------------------
@@ -320,7 +321,7 @@ template <>
 inline size_t hash_impl<32>(const void* buffer, size_t len)
 {
     uint32_t result = 0;
-    murmurhash3::MurmurHash3_x86_32(buffer, len, 0 /* seed */, &result);
+    murmurhash3::MurmurHash3_x86_32(buffer, static_cast<uint32_t>(len), 0 /* seed */, &result);
     return result;
 }
 
@@ -337,10 +338,9 @@ inline size_t hash_impl<64>(const void* buffer, size_t len)
     murmurhash3::MurmurHash3_x64_128(buffer, len, 0 /* seed */, result128);
 
     // this is the only "defined" way to convert properly...
-    // TODO: Why does MSVC complain about size_t != uint64_t?!
     size_t result = 0;
     memcpy(&result, result128, sizeof(result));
-	return result;
+    return result;
 }
 
 /**

include/spsl/pagealloc.hpp

@@ -120,7 +120,7 @@ class SensitivePageAllocator
         // check for memory that is still in use
         bool firstCbCall = true;
 
-        // check each chunk and tell the callbcak
+        // check each chunk and tell the callback
         if (m_leakCallback)
         {
             for (auto& chunk : m_managedChunks)
@@ -160,10 +160,11 @@ class SensitivePageAllocator
                         chunk.segments |= resetMask;
 
                         // notify the callback
-                        m_leakCallback(
-                          this, AllocationInfo{ (char*)chunk.addr + startIndex * segment_size,
-                                                (endIndex - startIndex + 1) * segment_size },
-                          firstCbCall);
+                        m_leakCallback(this,
+                                       AllocationInfo{ reinterpret_cast<char*>(chunk.addr) +
+                                                         startIndex * segment_size,
+                                                       (endIndex - startIndex + 1) * segment_size },
+                                       firstCbCall);
                         firstCbCall = false;
                     }
                 }
@@ -198,7 +199,7 @@ class SensitivePageAllocator
     /**
      * Returns the "default instance", a.k.a. a static instance of the allocator. The instance is
      * created upon first use and destroyed when the application exits.
-     * Note: This functio may throw any exception that the constructor might throw.
+     * Note: This function may throw any exception that the constructor might throw.
      * @return a reference to the instance
      */
     static SensitivePageAllocator& getDefaultInstance()
@@ -210,7 +211,7 @@ class SensitivePageAllocator
 
     /**
      * Sets the leak callback function. This function is called by the destructor for every memory
-     * location that hasn't been deallcated yet.
+     * location that hasn't been deallocated yet.
      *
      * Note: This method is intentionally *not* thread-safe.
      * Another note: Pass @c nullptr to disable leak checks.
@@ -230,7 +231,8 @@ class SensitivePageAllocator
                          bool first)
     {
         if (first)
-            std::cerr << "!!! Leaks detected in PageAllocator(" << (const void*)instance << "):\n";
+            std::cerr << "!!! Leaks detected in PageAllocator("
+                      << reinterpret_cast<const void*>(instance) << "):\n";
         std::cerr << "!!!   " << leak.size << " bytes @ address " << leak.addr << '\n';
     }
 
@@ -258,7 +260,7 @@ class SensitivePageAllocator
 
     static inline constexpr uint64_t getBitmask(std::size_t n)
     {
-        return (n == segmentsPerChunk ? all64 : (((uint64_t)1 << n) - 1));
+        return (n == segmentsPerChunk ? all64 : ((static_cast<uint64_t>(1) << n) - 1));
     }
 
 
@@ -383,7 +385,7 @@ class SensitivePageAllocator
                 {
                     // found! -> mark as reserved
                     chunk.segments &= ~mask;
-                    return (char*)chunk.addr + index * segment_size;
+                    return reinterpret_cast<char*>(chunk.addr) + index * segment_size;
                 }
                 // shift the mask
                 mask <<= 1;
@@ -400,7 +402,7 @@ class SensitivePageAllocator
         for (std::size_t i = 0; i < m_chunksPerPage; ++i)
         {
             m_managedChunks.push_back(
-              ChunkManagementInfo{ (char*)addr + i * chunk_size, addr, all64 });
+              ChunkManagementInfo{ reinterpret_cast<char*>(addr) + i * chunk_size, addr, all64 });
         }
         ChunkManagementInfo& chunk = m_managedChunks[m_managedChunks.size() - m_chunksPerPage];
 
@@ -427,10 +429,12 @@ class SensitivePageAllocator
         pointer pageAddr = nullptr;
         for (auto& chunk : m_managedChunks)
         {
-            if (addr >= chunk.addr && addr < (char*)chunk.addr + chunk_size)
+            if (addr >= chunk.addr && addr < reinterpret_cast<char*>(chunk.addr) + chunk_size)
             {
                 // found: calculate the index and the bit mask
-                std::size_t index = ((char*)addr - (char*)chunk.addr) / segment_size;
+                std::size_t index = static_cast<std::size_t>(reinterpret_cast<char*>(addr) -
+                                                             reinterpret_cast<char*>(chunk.addr)) /
+                                    segment_size;
                 uint64_t mask = bitmask << index;
                 chunk.segments |= mask;
 
@@ -526,7 +530,7 @@ class SensitivePageAllocator
     std::vector<AllocationInfo> m_unmanagedAreas;
 
     /// This function is called by the destructor for every memory location that hasn't been
-    /// deallocted yet. The default implementation prints using std::cerr.
+    /// deallocated yet. The default implementation prints using std::cerr.
     LeakCallbackFunction m_leakCallback;
 };
 
@@ -545,17 +549,39 @@ class SensitiveSegmentAllocator
 
     // constructors
     SensitiveSegmentAllocator() : m_alloc(&SensitivePageAllocator::getDefaultInstance()) {}
-    explicit SensitiveSegmentAllocator(SensitivePageAllocator& alloc) noexcept : m_alloc(&alloc) {}
-    explicit SensitiveSegmentAllocator(const SensitiveSegmentAllocator& other) noexcept = default;
+    SensitiveSegmentAllocator(SensitivePageAllocator& alloc) noexcept : m_alloc(&alloc) {}
+    SensitiveSegmentAllocator(const SensitiveSegmentAllocator& other) noexcept = default;
+    SensitiveSegmentAllocator(SensitiveSegmentAllocator&& other) noexcept
+      : SensitiveSegmentAllocator()
+    {
+        this->swap(other);
+    }
     template <class U>
     explicit SensitiveSegmentAllocator(const SensitiveSegmentAllocator<U>& other) noexcept
       : m_alloc(other.m_alloc)
     {
     }
 
+    SensitiveSegmentAllocator& operator=(const SensitiveSegmentAllocator& other) noexcept = default;
+    SensitiveSegmentAllocator& operator=(SensitiveSegmentAllocator&& other) noexcept
+    {
+        this->swap(other);
+        return *this;
+    }
+
     // default destructor
     ~SensitiveSegmentAllocator() = default;
 
+    /**
+     * Swaps this allocator with another one by swapping the referenced page allocator.
+     * @param[in] other    the allocator to swap with
+     */
+    void swap(SensitiveSegmentAllocator& other) noexcept { std::swap(m_alloc, other.m_alloc); }
+
+    SensitivePageAllocator* pageAllocator() const noexcept { return m_alloc; }
+
+    // allocation / deallocation
+
     T* allocate(std::size_t n) { return static_cast<T*>(m_alloc->allocate(n * sizeof(T))); }
 
     void deallocate(T* p, std::size_t n) { m_alloc->deallocate(p, n * sizeof(T)); }
@@ -566,7 +592,7 @@ class SensitiveSegmentAllocator
     /// non-owning pointer to the "real" allocator (never nullptr, but cannot use a reference...)
     SensitivePageAllocator* m_alloc;
 };
-}
+} // namespace spsl
 
 
 #endif /* SPSL_PAGEALLOC_HPP_ */

include/spsl/policies.hpp

@@ -86,7 +86,7 @@ struct Throw
     {
         if (cap > max)
             throw std::length_error("requested capacity exceeds maximum");
-    };
+    }
     template <typename char_type, typename size_type>
     static size_type checkAssign(const char_type*, size_type n, size_type max)
     {

include/spsl/storage_array.hpp

@@ -133,7 +133,8 @@ class StorageArray
     }
     void push_back(char_type c)
     {
-        const size_type n = overflow_policy::checkAppend((size_type)1, c, size(), max_size());
+        const size_type n =
+          overflow_policy::checkAppend(static_cast<size_type>(1), c, size(), max_size());
         if (n)
         {
             m_buffer[m_length++] = c;