Merge pull request #35 from Sidelobe/bugfix/AddressSonarcloudBugs

Address Sonarcloud 'bugs' (in test code)
Sidelobe · Dec 12, 2023 · 7fafbb2 · 7fafbb2
2 parents a658eb3 + 162c1f9
commit 7fafbb2
Show file tree

Hide file tree

Showing 9 changed files with 21 additions and 17 deletions.
diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -42,6 +42,7 @@ set(TEST_WITH_AMALGAMATED_HEADER OFF CACHE BOOL "Build & Test with Amalgamated h
 add_library(${PROJECT_NAME} INTERFACE)
 
 if (TEST_WITH_AMALGAMATED_HEADER)
+  message(STATUS "using Amalgamated Headers")
   target_sources(${PROJECT_NAME} INTERFACE "single_include/HyperBuffer.hpp")
   target_include_directories(${PROJECT_NAME} INTERFACE "single_include")
   target_compile_definitions(${PROJECT_NAME} INTERFACE SLB_AMALGATED_HEADER)

diff --git a/deploy/generate-code-coverage.command b/deploy/generate-code-coverage.command
@@ -9,7 +9,7 @@ cd build/coverage
 
 find . -maxdepth 10 -type f  \( -name \*.gcno -o -name \*.gcda \) -delete
 
-cmake -j 4 -DCMAKE_BUILD_TYPE=Release -DCODE_COVERAGE=yes ../../..
+cmake -DCMAKE_BUILD_TYPE=Release -DCODE_COVERAGE=yes ../../..
 make
 ./HyperBufferTest
 

diff --git a/deploy/generate-xcode-macos.command b/deploy/generate-xcode-macos.command
@@ -8,6 +8,7 @@ cd build/xcode-macos
 cmake -G Xcode \
 -DCMAKE_OSX_DEPLOYMENT_TARGET=10.13 \
 -DCMAKE_OSX_ARCHITECTURES=arm64\;x86_64 \
+-DTEST_WITH_AMALGAMATED_HEADER=No \
 ../../..
 
 exit
diff --git a/single_include/HyperBuffer.hpp b/single_include/HyperBuffer.hpp
@@ -328,7 +328,7 @@ constexpr T productOverRange(int firstFactor, int numFactors, Args... args) noex
     numFactors = std::min<int>(numFactors, static_cast<int>(sizeof...(args)) - firstFactor + 1);
 
     T values[]{ args... };
-     for (int i=firstFactor; i < firstFactor+numFactors; ++i) {
+    for (int i=firstFactor; i < firstFactor+numFactors; ++i) {
         product *= values[i-1];
     }
     return product;
@@ -554,7 +554,7 @@ class BufferGeometry
         // Get number of data pointers (length of 2nd-lowest dim)
         int numDataPointers = StdArrayOperations::productCapped(N-1, m_dimensionExtents);
 
-        //Hook up pointer that point to data (second lowest-order dimension)
+        // Hook up pointer that point to data (second lowest-order dimension)
         for (int i=0; i < numDataPointers; ++i) {
             int offsetInDataArray = i * m_dimensionExtents[N-1];
             pointerArray[dataPointerStartOffset + i] = &dataArray[offsetInDataArray];
@@ -706,6 +706,7 @@ class StoragePolicyView
         m_pointers(m_bufferGeometry.getRequiredPointerArraySize())
     {
         ASSERT(CompiletimeMath::areAllPositive(i...), "Invalid Dimension extents");
+        ASSERT(m_externalData != nullptr);
         m_bufferGeometry.hookupPointerArrayToData(m_externalData, m_pointers.data());
     }
 

diff --git a/source/BufferGeometry.hpp b/source/BufferGeometry.hpp
@@ -94,7 +94,7 @@ class BufferGeometry
         // Get number of data pointers (length of 2nd-lowest dim)
         int numDataPointers = StdArrayOperations::productCapped(N-1, m_dimensionExtents);
 
-        //Hook up pointer that point to data (second lowest-order dimension)
+        // Hook up pointer that point to data (second lowest-order dimension)
         for (int i=0; i < numDataPointers; ++i) {
             int offsetInDataArray = i * m_dimensionExtents[N-1];
             pointerArray[dataPointerStartOffset + i] = &dataArray[offsetInDataArray];

diff --git a/source/CompiletimeMath.hpp b/source/CompiletimeMath.hpp
@@ -71,7 +71,7 @@ constexpr T productOverRange(int firstFactor, int numFactors, Args... args) noex
     numFactors = std::min<int>(numFactors, static_cast<int>(sizeof...(args)) - firstFactor + 1);
 
     T values[]{ args... };
-     for (int i=firstFactor; i < firstFactor+numFactors; ++i) {
+    for (int i=firstFactor; i < firstFactor+numFactors; ++i) {
         product *= values[i-1];
     }
     return product;

diff --git a/source/HyperBufferStoragePolicies.hpp b/source/HyperBufferStoragePolicies.hpp
@@ -117,6 +117,7 @@ class StoragePolicyView
         m_pointers(m_bufferGeometry.getRequiredPointerArraySize())
     {
         ASSERT(CompiletimeMath::areAllPositive(i...), "Invalid Dimension extents");
+        ASSERT(m_externalData != nullptr);
         m_bufferGeometry.hookupPointerArrayToData(m_externalData, m_pointers.data());
     }
 

diff --git a/test/tests/BufferGeometryTests.cpp b/test/tests/BufferGeometryTests.cpp
@@ -41,8 +41,8 @@ TEST_CASE("BufferGeometry Tests")
     SECTION("2D") {
         constexpr int N = 2;
         BufferGeometry<N> bufferGeo(3, 3);
-        CHECK(bufferGeo.getDimensionExtents() == std::array<int, 2>{3, 3});
-        CHECK(bufferGeo.getDimensionExtentsPointer() != nullptr);
+        REQUIRE(bufferGeo.getDimensionExtents() == std::array<int, 2>{3, 3});
+        REQUIRE(bufferGeo.getDimensionExtentsPointer() != nullptr);
         CHECK(bufferGeo.getDimensionExtentsPointer()[0] == 3);
         CHECK(bufferGeo.getDimensionExtentsPointer()[1] == 3);
         CHECK(bufferGeo.getDataArrayOffsetForHighestOrderSubDim(0) == 0);

diff --git a/test/tests/HyperBufferCopyMoveTests.cpp b/test/tests/HyperBufferCopyMoveTests.cpp
@@ -152,7 +152,7 @@ TEST_CASE("Copy/Move a HyperBuffer with external, flat allocation")
     REQUIRE(bufferCopy[2][0] == buffer[2][0]);
 
     // Copy Ctor - verify no memory is allocated
-    HyperBufferView<int, N> bufferCopyCtor(nullptr, 3, 2, 8);
+    HyperBufferView<int, N> bufferCopyCtor(preAllocData, 3, 2, 8);
     {
         ScopedMemorySentinel sentinel;
         bufferCopyCtor = buffer;
@@ -166,10 +166,10 @@ TEST_CASE("Copy/Move a HyperBuffer with external, flat allocation")
     // Move assignment operator
     {
         HyperBufferView<int, N> bufferMovedFrom = buffer; // working copy
-        HyperBufferView<int, N> bufferMovedTo(nullptr, 3, 2, 8);
+        HyperBufferView<int, N> bufferMovedTo(preAllocData, 3, 2, 8);
         bufferMovedTo = std::move(bufferMovedFrom);
         verifyBuffer(bufferMovedTo);
-        REQUIRE(bufferMovedFrom.data() == nullptr);
+        //REQUIRE bufferMovedFrom.data() == nullptr -- this cannot be relied upon
         REQUIRE(bufferMovedTo[0] == buffer[0]); // moved points to the same data
         REQUIRE(bufferMovedTo[0][1] == buffer[0][1]);
         REQUIRE(bufferMovedTo[2][0] == buffer[2][0]);
@@ -192,7 +192,7 @@ TEST_CASE("Copy/Move a HyperBuffer with external, flat allocation")
         sentinel.setArmed(false);
         verifyBuffer(bufferMovedTo);
         REQUIRE(bufferMovedTo.data() != nullptr);
-        REQUIRE(bufferMovedFrom.data() == nullptr);
+        //REQUIRE bufferMovedFrom.data() == nullptr -- this cannot be relied upon
     }
 #endif
 
@@ -201,7 +201,7 @@ TEST_CASE("Copy/Move a HyperBuffer with external, flat allocation")
         HyperBufferView<int, N> bufferMovedFrom = buffer;
         HyperBufferView<int, N> bufferMovedToCtor(std::move(bufferMovedFrom));
         verifyBuffer(bufferMovedToCtor);
-        REQUIRE(bufferMovedFrom.data() == nullptr);
+        // REQUIRE bufferMovedFrom.data() == nullptr -- this cannot be relied upon
         REQUIRE(bufferMovedToCtor[0] == buffer[0]); // moved points to the same data
         REQUIRE(bufferMovedToCtor[0][1] == buffer[0][1]);
         REQUIRE(bufferMovedToCtor[2][0] == buffer[2][0]);
@@ -224,7 +224,7 @@ TEST_CASE("Copy/Move a HyperBuffer with external, flat allocation")
         sentinel.setArmed(false);
         verifyBuffer(bufferMovedCtor);
         REQUIRE(bufferMovedCtor.data() != nullptr);
-        REQUIRE(bufferMovedFrom.data() == nullptr);
+        //REQUIRE bufferMovedFrom.data() == nullptr -- this cannot be relied upon
     }
 #endif
 
@@ -282,7 +282,7 @@ TEST_CASE("Copy/Move a HyperBuffer with external, multi-dimensional allocation")
         HyperBufferViewNC<int, N> bufferMovedFrom = buffer; // working copy
         HyperBufferViewNC<int, N> bufferMovedTo = std::move(bufferMovedFrom);
         verifyBuffer(bufferMovedTo);
-        REQUIRE(bufferMovedFrom.data() != nullptr); // original remains untouched
+        //REQUIRE bufferMovedFrom.data() != nullptr -- original remains untouched -- NOTE: this cannot be relied upon
         REQUIRE(bufferMovedTo[0] == buffer[0]); // moved points to the same data
         REQUIRE(bufferMovedTo[0][1] == buffer[0][1]);
         REQUIRE(bufferMovedTo[2][0] == buffer[2][0]);
@@ -297,15 +297,15 @@ TEST_CASE("Copy/Move a HyperBuffer with external, multi-dimensional allocation")
             bufferMovedToPreAlloc = std::move(bufferMovedFrom);
         }
         verifyBuffer(bufferMovedToPreAlloc);
-        REQUIRE(bufferMovedFrom.data() != nullptr); // original remains untouched
+        //REQUIRE bufferMovedFrom.data() != nullptr -- original remains untouched -- NOTE: this cannot be relied upon
     }
 
     // Move Ctor
     {
         HyperBufferViewNC<int, N> bufferMovedFrom = buffer;
         HyperBufferViewNC<int, N> bufferMovedToCtor(std::move(bufferMovedFrom));
         verifyBuffer(bufferMovedToCtor);
-        REQUIRE(bufferMovedFrom.data() != nullptr); // original remains untouched
+        //REQUIRE bufferMovedFrom.data() != nullptr -- original remains untouched -- NOTE: this cannot be relied upon
         REQUIRE(bufferMovedToCtor[0] == buffer[0]); // moved points to the same data
         REQUIRE(bufferMovedToCtor[0][1] == buffer[0][1]);
         REQUIRE(bufferMovedToCtor[2][0] == buffer[2][0]);
@@ -319,7 +319,7 @@ TEST_CASE("Copy/Move a HyperBuffer with external, multi-dimensional allocation")
             HyperBufferViewNC<int, N> bufferMovedCtor(std::move(bufferMovedFrom));
             UNUSED(bufferMovedCtor);
         }
-        REQUIRE(bufferMovedFrom.data() != nullptr); // original remains untouched
+        //REQUIRE bufferMovedFrom.data() != nullptr -- original remains untouched -- NOTE: this cannot be relied upon
     }
 }