Merge pull request #323 from tcahuzax/klocwork_fix_3.1.0

Fix errors found by Klocwork on the PF 3.1.0

 - Wrong instance lifetime
 - Using uninitialized variable
 - Avoiding using memcpy
 - Removing strncpy call

Author: krocard

test/functional-tests/Basic.cpp

@@ -62,29 +62,34 @@ SCENARIO_METHOD(ParameterFramework, "No Logger", "[log]")
     }
 }
 
-SCENARIO_METHOD(WarningPF, "Logger should receive info and warnings", "[log]")
+SCENARIO("Logger should receive info and warnings", "[log]")
 {
-    GIVEN ("Config files that emit warnings") {
-        GIVEN ("A logger that stores logs") {
-            StoreLogger logger{};
-            WHEN ("The record logger is set") {
-                setLogger(&logger);
-                THEN ("Start should succeed") {
-                    REQUIRE_NOTHROW(start());
-                    AND_THEN ("The logger should have stored info and warning log") {
-                        using Logs = StoreLogger::Logs;
-                        using Level = StoreLogger::Log::Level;
-                        CHECK(logger.filter(Level::warning) != Logs{});
-                        CHECK(logger.getLogs() != Logs{});
-                    }
-                }
-                AND_WHEN ("A nullptr logger is set") {
-                    setLogger(nullptr);
+    GIVEN ("A logger that stores logs") {
+        /* Instantiating logger first to ensure that its lifetime is longer than the pfw's one,
+         * because the pfw references the logger. */
+        StoreLogger logger{};
+        GIVEN ("A parameter framework") {
+            WarningPF pfw;
+            GIVEN ("Config files that emit warnings") {
+                WHEN ("The record logger is set") {
+                    pfw.setLogger(&logger);
                     THEN ("Start should succeed") {
-                        REQUIRE_NOTHROW(start());
-                        AND_THEN (
-                            "The record logger should NOT have stored any info or warning log") {
-                            CHECK(logger.getLogs() == StoreLogger::Logs{});
+                        REQUIRE_NOTHROW(pfw.start());
+                        AND_THEN ("The logger should have stored info and warning log") {
+                            using Logs = StoreLogger::Logs;
+                            using Level = StoreLogger::Log::Level;
+                            CHECK(logger.filter(Level::warning) != Logs{});
+                            CHECK(logger.getLogs() != Logs{});
+                        }
+                    }
+                    AND_WHEN ("A nullptr logger is set") {
+                        pfw.setLogger(nullptr);
+                        THEN ("Start should succeed") {
+                            REQUIRE_NOTHROW(pfw.start());
+                            AND_THEN ("The record logger should NOT have stored any info or "
+                                      "warning log") {
+                                CHECK(logger.getLogs() == StoreLogger::Logs{});
+                            }
                         }
                     }
                 }

test/functional-tests/FloatingPoint.cpp

@@ -99,7 +99,7 @@ SCENARIO_METHOD(FloatsPF, "Floating points", "[floating points]")
                     GIVEN ("A valid value " + vec.title) {
                         CHECK_NOTHROW(setParameter(path, vec.payload));
                         string getValueBack;
-                        CHECK_NOTHROW(getParameter(path, getValueBack));
+                        REQUIRE_NOTHROW(getParameter(path, getValueBack));
                         CHECK(getValueBack == vec.payload);
                     }
                 }
@@ -138,7 +138,7 @@ SCENARIO_METHOD(FloatsPF, "Floating points", "[floating points]")
                     GIVEN ("A valid value " + vec.title) {
                         CHECK_NOTHROW(setParameter(path, vec.payload));
                         string getValueBack;
-                        CHECK_NOTHROW(getParameter(path, getValueBack));
+                        REQUIRE_NOTHROW(getParameter(path, getValueBack));
                         CHECK(getValueBack == vec.payload);
                     }
                 }
@@ -161,7 +161,7 @@ SCENARIO_METHOD(FloatsPF, "Floating points", "[floating points]")
                     GIVEN ("A valid value " + vec.title) {
                         CHECK_NOTHROW(handle.setAsDouble(vec.payload));
                         double getValueBack;
-                        CHECK_NOTHROW(handle.getAsDouble(getValueBack));
+                        REQUIRE_NOTHROW(handle.getAsDouble(getValueBack));
                         CHECK(getValueBack == vec.payload);
                     }
                 }

test/test-subsystem/TESTSubsystemBinary.cpp

@@ -34,6 +34,7 @@
 #include <stdlib.h>
 #include <assert.h>
 #include <algorithm>
+#include <AlwaysAssert.hpp>
 #include <Iterator.hpp>
 #include "TESTSubsystemBinary.h"
 
@@ -53,7 +54,10 @@ std::string CTESTSubsystemBinary::toString(const void *pvValue, size_t size) con
 
     assert(size <= sizeof(uiValue));
 
-    memcpy(&uiValue, pvValue, size);
+    auto first = MAKE_ARRAY_ITERATOR(static_cast<const uint8_t *>(pvValue), size);
+    auto destination = MAKE_ARRAY_ITERATOR(reinterpret_cast<uint8_t *>(&uiValue), sizeof(uiValue));
+
+    std::copy_n(first, size, destination);
 
     strStream << "0x" << std::hex << uiValue;
 

test/test-subsystem/TESTSubsystemString.cpp

@@ -28,6 +28,10 @@
 * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 */
 #include <string.h>
+#include <iterator>
+#include <algorithm>
+#include <stdexcept>
+#include <Iterator.hpp>
 #include "TESTSubsystemString.h"
 
 #define base CTESTSubsystemObject
@@ -46,5 +50,15 @@ std::string CTESTSubsystemString::toString(const void *pvValue, size_t /*size*/)
 
 void CTESTSubsystemString::fromString(const std::string &strValue, void *pvValue, size_t size)
 {
-    strncpy((char *)pvValue, strValue.c_str(), size);
+    std::size_t requiredBufferSize = strValue.length() + 1; /* Adding one for null character */
+    if (size < requiredBufferSize) {
+        throw std::logic_error("Buffer is to small: " + std::to_string(size) + " Minimum size: " +
+                               std::to_string(requiredBufferSize));
+    }
+
+    auto destination = MAKE_ARRAY_ITERATOR(static_cast<char *>(pvValue), size);
+    auto last = std::copy(begin(strValue), end(strValue), destination);
+
+    /* Adding null character */
+    *last = 0;
 }