Resolve clang-tidy bugprone warnings

Most of these issues are pretty innocuous, but I did find a couple places that constructors/destructors were relying on dynamic dispatch.
cpputest · Nov 19, 2022 · 44ca541 · 44ca541
1 parent 65846fb
commit 44ca541
Show file tree

Hide file tree

Showing 16 changed files with 44 additions and 38 deletions.
diff --git a/examples/AllTests/MockDocumentationTest.cpp b/examples/AllTests/MockDocumentationTest.cpp
@@ -125,8 +125,8 @@ TEST(MockDocumentation, ObjectParameters)
 TEST(MockDocumentation, returnValue)
 {
     mock().expectOneCall("function").andReturnValue(10);
-    int value = mock().actualCall("function").returnValue().getIntValue();
-    value = mock().returnValue().getIntValue();
+    mock().actualCall("function").returnValue().getIntValue();
+    int value = mock().returnValue().getIntValue();
     LONGS_EQUAL(10, value);
 }
 

diff --git a/include/CppUTest/UtestMacros.h b/include/CppUTest/UtestMacros.h
@@ -312,7 +312,7 @@
   BITS_LOCATION(expected, actual, mask, text, __FILE__, __LINE__)
 
 #define BITS_LOCATION(expected, actual, mask, text, file, line)\
-  do { UtestShell::getCurrent()->assertBitsEqual(expected, actual, mask, sizeof(actual), text, file, line); } while(0)
+  do { UtestShell::getCurrent()->assertBitsEqual(expected, actual, mask, sizeof(actual), text, file, line); } while(0) // NOLINT
 
 #define ENUMS_EQUAL_INT(expected, actual)\
   ENUMS_EQUAL_TYPE(int, expected, actual)

diff --git a/include/CppUTestExt/MockCheckedActualCall.h b/include/CppUTestExt/MockCheckedActualCall.h
@@ -150,7 +150,7 @@ class MockCheckedActualCall : public MockActualCall
     MockOutputParametersListNode* outputParameterExpectations_;
 
     virtual void addOutputParameter(const SimpleString& name, const SimpleString& type, void* ptr);
-    virtual void cleanUpOutputParameterList();
+    void cleanUpOutputParameterList();
 };
 
 class MockActualCallTrace : public MockActualCall

diff --git a/src/CppUTest/CommandLineArguments.cpp b/src/CppUTest/CommandLineArguments.cpp
@@ -71,8 +71,7 @@ bool CommandLineArguments::parse(TestPlugin* plugin)
         else if (argument == "-ll") listTestLocations_ = true;
         else if (argument == "-ri") runIgnored_ = true;
         else if (argument == "-f") crashOnFail_ = true;
-        else if (argument == "-e") rethrowExceptions_ = false;
-        else if (argument == "-ci") rethrowExceptions_ = false;
+        else if ((argument == "-e") || (argument == "-ci")) rethrowExceptions_ = false;
         else if (argument.startsWith("-r")) setRepeatCount(ac_, av_, i);
         else if (argument.startsWith("-g")) addGroupFilter(ac_, av_, i);
         else if (argument.startsWith("-t")) correctParameters = addGroupDotNameFilter(ac_, av_, i, "-t", false, false);

diff --git a/src/CppUTest/MemoryLeakWarningPlugin.cpp b/src/CppUTest/MemoryLeakWarningPlugin.cpp
@@ -131,7 +131,7 @@ void cpputest_free_location_with_leak_detection(void* buffer, const char* file,
 #undef new
 
 #if CPPUTEST_HAVE_EXCEPTIONS
-#define UT_THROW_BAD_ALLOC_WHEN_NULL(memory) if (memory == NULLPTR) throw CPPUTEST_BAD_ALLOC()
+#define UT_THROW_BAD_ALLOC_WHEN_NULL(memory) if ((memory) == NULLPTR) throw CPPUTEST_BAD_ALLOC()
 #else
 #define UT_THROW_BAD_ALLOC_WHEN_NULL(memory)
 #endif

diff --git a/src/CppUTest/SimpleString.cpp b/src/CppUTest/SimpleString.cpp
@@ -62,7 +62,7 @@ GlobalSimpleStringMemoryAccountant::~GlobalSimpleStringMemoryAccountant()
 
 void GlobalSimpleStringMemoryAccountant::restoreAllocator()
 {
-    if (SimpleString::getStringAllocator() == allocator_)
+    if (allocator_ && (SimpleString::getStringAllocator() == allocator_))
         SimpleString::setStringAllocator(allocator_->originalAllocator());
 }
 
@@ -1031,19 +1031,17 @@ SimpleString StringFromMaskedBits(unsigned long value, unsigned long mask, size_
 
 SimpleString StringFromOrdinalNumber(unsigned int number)
 {
-    unsigned int onesDigit = number % 10;
-
-    const char* suffix;
-    if (number >= 11 && number <= 13) {
-        suffix = "th";
-    } else if (3 == onesDigit) {
-        suffix = "rd";
-    } else if (2 == onesDigit) {
-        suffix = "nd";
-    } else if (1 == onesDigit) {
-        suffix = "st";
-    } else {
-        suffix = "th";
+    const char* suffix = "th";
+
+    if ((number < 11) || (number > 13)) {
+        unsigned int const onesDigit = number % 10;
+        if (3 == onesDigit) {
+            suffix = "rd";
+        } else if (2 == onesDigit) {
+            suffix = "nd";
+        } else if (1 == onesDigit) {
+            suffix = "st";
+        }
     }
 
     return StringFromFormat("%u%s", number, suffix);

diff --git a/src/CppUTest/TestMemoryAllocator.cpp b/src/CppUTest/TestMemoryAllocator.cpp
@@ -418,7 +418,7 @@ MemoryAccountantAllocationNode* MemoryAccountant::createNewAccountantAllocationN
 
 void MemoryAccountant::destroyAccountantAllocationNode(MemoryAccountantAllocationNode* node) const
 {
-    allocator_->free_memory((char*) node, sizeof(node), __FILE__, __LINE__);
+    allocator_->free_memory((char*) node, sizeof(node), __FILE__, __LINE__); // NOLINT
 }
 
 MemoryAccountant::MemoryAccountant()
@@ -479,9 +479,9 @@ MemoryAccountantAllocationNode* MemoryAccountant::findNodeOfSize(size_t size) co
 {
     if (useCacheSizes_) {
         for (MemoryAccountantAllocationNode* node = head_; node; node = node->next_) {
-            if (size > node->size_ && node->next_ == NULLPTR)
-                return node;
-            else if (size <= node->size_ && !(node->next_->size_ != 0 && node->next_->size_ <= size))
+            if (((size > node->size_) && (node->next_ == NULLPTR))
+                || ((size <= node->size_) &&
+                    !((node->next_->size_ != 0) && (node->next_->size_ <= size))))
                 return node;
         }
     }

diff --git a/src/CppUTestExt/CodeMemoryReportFormatter.cpp b/src/CppUTestExt/CodeMemoryReportFormatter.cpp
@@ -34,7 +34,7 @@
 #define MAX_VARIABLE_NAME_LINE_PART 10
 #define MAX_VARIABLE_NAME_FILE_PART 53
 #define MAX_VARIABLE_NAME_SEPERATOR_PART 1
-#define MAX_VARIABLE_NAME_LENGTH MAX_VARIABLE_NAME_FILE_PART + MAX_VARIABLE_NAME_SEPERATOR_PART + MAX_VARIABLE_NAME_LINE_PART
+#define MAX_VARIABLE_NAME_LENGTH (MAX_VARIABLE_NAME_FILE_PART + MAX_VARIABLE_NAME_SEPERATOR_PART + MAX_VARIABLE_NAME_LINE_PART)
 
 struct CodeReportingAllocationNode
 {

diff --git a/src/CppUTestExt/MockSupport.cpp b/src/CppUTestExt/MockSupport.cpp
@@ -44,9 +44,18 @@ MockSupport& mock(const SimpleString& mockName, MockFailureReporter* failureRepo
 }
 
 MockSupport::MockSupport(const SimpleString& mockName)
-    : actualCallOrder_(0), expectedCallOrder_(0), strictOrdering_(false), standardReporter_(&defaultReporter_), ignoreOtherCalls_(false), enabled_(true), lastActualFunctionCall_(NULLPTR), mockName_(mockName), tracing_(false)
+    :
+        actualCallOrder_(0),
+        expectedCallOrder_(0),
+        strictOrdering_(false),
+        activeReporter_(NULLPTR),
+        standardReporter_(&defaultReporter_),
+        ignoreOtherCalls_(false),
+        enabled_(true),
+        lastActualFunctionCall_(NULLPTR),
+        mockName_(mockName),
+        tracing_(false)
 {
-    setActiveReporter(NULLPTR);
 }
 
 MockSupport::~MockSupport()

diff --git a/tests/CppUTest/SimpleStringCacheTest.cpp b/tests/CppUTest/SimpleStringCacheTest.cpp
@@ -188,7 +188,7 @@ TEST(SimpleStringInternalCache, clearCacheWillRemoveAllCachedMemoryButNotAllUsed
     char* mem = cache.alloc(10);
     cache.dealloc(mem, 10);
 
-    mem = cache.alloc(60);
+    cache.alloc(60);
 
     cache.clearCache();
 

diff --git a/tests/CppUTest/TestFailureNaNTest.cpp b/tests/CppUTest/TestFailureNaNTest.cpp
@@ -54,7 +54,7 @@ TEST_GROUP(TestFailureNanAndInf)
         delete test;
     }
 };
-#define FAILURE_EQUAL(a, b) STRCMP_EQUAL_LOCATION(a, b.getMessage().asCharString(), "", __FILE__, __LINE__)
+#define FAILURE_EQUAL(a, b) STRCMP_EQUAL_LOCATION(a, (b).getMessage().asCharString(), "", __FILE__, __LINE__)
 
 TEST(TestFailureNanAndInf, DoublesEqualExpectedIsNaN)
 {

diff --git a/tests/CppUTest/TestFailureTest.cpp b/tests/CppUTest/TestFailureTest.cpp
@@ -47,7 +47,7 @@ TEST_GROUP(TestFailure)
         delete test;
     }
 };
-#define FAILURE_EQUAL(a, b) STRCMP_EQUAL_LOCATION(a, b.getMessage().asCharString(), "", __FILE__, __LINE__)
+#define FAILURE_EQUAL(a, b) STRCMP_EQUAL_LOCATION(a, (b).getMessage().asCharString(), "", __FILE__, __LINE__)
 
 TEST(TestFailure, CreateFailure)
 {

diff --git a/tests/CppUTest/TestMemoryAllocatorTest.cpp b/tests/CppUTest/TestMemoryAllocatorTest.cpp
@@ -127,7 +127,7 @@ TEST(TestMemoryAllocatorTest, NullUnknownNames)
 
 #if (! CPPUTEST_SANITIZE_ADDRESS)
 
-#define MAX_SIZE_FOR_ALLOC ((size_t) -1 > (unsigned short)-1) ? (size_t) -97 : (size_t) -1
+#define MAX_SIZE_FOR_ALLOC ((size_t) -1 > (unsigned short)-1) ? (size_t)(-97) : (size_t)(-1)
 
 static void failTryingToAllocateTooMuchMemory(void)
 {

diff --git a/tests/CppUTest/TestUTestMacro.cpp b/tests/CppUTest/TestUTestMacro.cpp
@@ -615,16 +615,16 @@ TEST(UnitTestMacros, FailureWithLONGS_EQUALS)
 
 static void failingTestMethodWithLONGS_EQUALWithSymbolicParameters_()
 {
-#define _MONDAY 1
-    int day_of_the_week = _MONDAY+1;
-    LONGS_EQUAL(_MONDAY, day_of_the_week);
+#define MONDAY 1
+    int day_of_the_week = MONDAY+1;
+    LONGS_EQUAL(MONDAY, day_of_the_week);
     TestTestingFixture::lineExecutedAfterCheck(); // LCOV_EXCL_LINE
 } // LCOV_EXCL_LINE
 
 TEST(UnitTestMacros, FailureWithLONGS_EQUALShowsSymbolicParameters)
 {
     fixture.runTestWithMethod(failingTestMethodWithLONGS_EQUALWithSymbolicParameters_);
-    CHECK_TEST_FAILS_PROPER_WITH_TEXT("LONGS_EQUAL(_MONDAY, day_of_the_week) failed");
+    CHECK_TEST_FAILS_PROPER_WITH_TEXT("LONGS_EQUAL(MONDAY, day_of_the_week) failed");
     CHECK_TEST_FAILS_PROPER_WITH_TEXT("expected <1 (0x1)>");
     CHECK_TEST_FAILS_PROPER_WITH_TEXT("but was  <2 (0x2)>");
     CHECK_FALSE(fixture.getOutput().contains("Message: "));

diff --git a/tests/CppUTest/UtestPlatformTest.cpp b/tests/CppUTest/UtestPlatformTest.cpp
@@ -126,7 +126,7 @@ TEST(UTestPlatformsTest_PlatformSpecificRunTestInASeperateProcess, FailureInSepa
 
 static int accessViolationTestFunction_()
 {
-    return *(volatile int*) NULLPTR;
+    return *(volatile int*) NULLPTR; // NOLINT
 }
 
 TEST(UTestPlatformsTest_PlatformSpecificRunTestInASeperateProcess, AccessViolationInSeparateProcessWorks)

diff --git a/tests/CppUTestExt/OrderedTestTest.cpp b/tests/CppUTestExt/OrderedTestTest.cpp
@@ -82,7 +82,7 @@ TEST_GROUP(TestOrderedTest)
 
 TEST(TestOrderedTest, TestInstallerSetsFields)
 {
-    OrderedTestInstaller(orderedTest, "testgroup", "testname", "this.cpp", 10, 5);
+    OrderedTestInstaller installer(orderedTest, "testgroup", "testname", "this.cpp", 10, 5);
     STRCMP_EQUAL("testgroup", orderedTest.getGroup().asCharString());
     STRCMP_EQUAL("testname", orderedTest.getName().asCharString());
     STRCMP_EQUAL("this.cpp", orderedTest.getFile().asCharString());