Merge topic 'pvs-cleanup'

7fe3e874d5 cmCPackLog: Fix support for multiple log message tags
74f2c0ea56 cmCTestTestHandler: Remove extra layer of parentheses
7c2767ef3b cmCTestMultiProcessHandler: Explain testRun ownership in comments
303e813438 CTest: Simplify some boolean conditions
51565abe79 cmMessageCommand: Remove extra layer of parentheses
b1cfaf7b91 cmVSSetupHelper: Remove unused SmartBSTR copy operations
3f4c4e7afe cmVSSetupHelper: Fix SmartBSTR copy operations
a8ca5aea94 cmMakefileTargetGenerator: Check for null before using a pointer
...

Acked-by: Kitware Robot <kwrobot@kitware.com>
Acked-by: Daniel Pfeifer <daniel@pfeifer-mail.de>
Acked-by: Artalus <artalus-mail@yandex.ru>
Merge-request: !3715

8 files changed, 27 insertions, 45 deletions

diff --git a/Source/CPack/cmCPackLog.cxx b/Source/CPack/cmCPackLog.cxx
index a3ca4b5..ca675fd 100644
--- a/Source/CPack/cmCPackLog.cxx
+++ b/Source/CPack/cmCPackLog.cxx
@@ -83,7 +83,7 @@ void cmCPackLog::Log(int tag, const char* file, int line, const char* msg,
       if (!tagString.empty()) {
         tagString += ",";
       }
-      tagString = "VERBOSE";
+      tagString += "VERBOSE";
     }
   }
   if (tag & LOG_WARNING) {
@@ -93,7 +93,7 @@ void cmCPackLog::Log(int tag, const char* file, int line, const char* msg,
       if (!tagString.empty()) {
         tagString += ",";
       }
-      tagString = "WARNING";
+      tagString += "WARNING";
     }
   }
   if (tag & LOG_ERROR) {
@@ -103,7 +103,7 @@ void cmCPackLog::Log(int tag, const char* file, int line, const char* msg,
       if (!tagString.empty()) {
         tagString += ",";
       }
-      tagString = "ERROR";
+      tagString += "ERROR";
     }
   }
   if (tag & LOG_DEBUG && this->Debug) {
@@ -113,7 +113,7 @@ void cmCPackLog::Log(int tag, const char* file, int line, const char* msg,
       if (!tagString.empty()) {
         tagString += ",";
       }
-      tagString = "DEBUG";
+      tagString += "DEBUG";
     }
     useFileAndLine = true;
   }
@@ -124,7 +124,7 @@ void cmCPackLog::Log(int tag, const char* file, int line, const char* msg,
       if (!tagString.empty()) {
         tagString += ",";
       }
-      tagString = "VERBOSE";
+      tagString += "VERBOSE";
     }
   }
   if (this->Quiet) {
diff --git a/Source/CTest/cmCTestMultiProcessHandler.cxx b/Source/CTest/cmCTestMultiProcessHandler.cxx
index 9925dc1..af1980a 100644
--- a/Source/CTest/cmCTestMultiProcessHandler.cxx
+++ b/Source/CTest/cmCTestMultiProcessHandler.cxx
@@ -189,10 +189,13 @@ bool cmCTestMultiProcessHandler::StartTestProcess(int test)
                           std::strerror(workdir.GetLastResult()));
   } else {
     if (testRun->StartTest(this->Completed, this->Total)) {
+      // Ownership of 'testRun' has moved to another structure.
+      // When the test finishes, FinishTestProcess will be called.
       return true;
     }
   }
 
+  // Pass ownership of 'testRun'.
   this->FinishTestProcess(testRun, false);
   return false;
 }
diff --git a/Source/CTest/cmCTestRunTest.cxx b/Source/CTest/cmCTestRunTest.cxx
index 89b585e..23c6b0d 100644
--- a/Source/CTest/cmCTestRunTest.cxx
+++ b/Source/CTest/cmCTestRunTest.cxx
@@ -142,8 +142,7 @@ bool cmCTestRunTest::EndTest(size_t completed, size_t total, bool started)
       this->TestResult.CompletionStatus = s.str();
       cmCTestLog(this->CTest, HANDLER_OUTPUT, "***Skipped ");
       skipped = true;
-    } else if ((success && !this->TestProperties->WillFail) ||
-               (!success && this->TestProperties->WillFail)) {
+    } else if (success != this->TestProperties->WillFail) {
       this->TestResult.Status = cmCTestTestHandler::COMPLETED;
       outputStream << "   Passed  ";
     } else {
diff --git a/Source/CTest/cmCTestTestHandler.cxx b/Source/CTest/cmCTestTestHandler.cxx
index ca0eaf2..9181daa 100644
--- a/Source/CTest/cmCTestTestHandler.cxx
+++ b/Source/CTest/cmCTestTestHandler.cxx
@@ -695,8 +695,7 @@ void cmCTestTestHandler::PrintLabelOrSubprojectSummary(bool doSubProject)
       }
       // if we are doing sub projects and this label is one, then use it
       // if we are not doing sub projects and the label is not one use it
-      if ((doSubProject && isSubprojectLabel) ||
-          (!doSubProject && !isSubprojectLabel)) {
+      if (doSubProject == isSubprojectLabel) {
         if (l.size() > maxlen) {
           maxlen = l.size();
         }
@@ -889,7 +888,7 @@ void cmCTestTestHandler::ComputeTestListForRerunFailed()
     cnt++;
 
     // if this test is not in our list of tests to run, then skip it.
-    if ((!this->TestsToRun.empty() && !cmContains(TestsToRun, cnt))) {
+    if (!this->TestsToRun.empty() && !cmContains(this->TestsToRun, cnt)) {
       continue;
     }
 
diff --git a/Source/cmGlobalVisualStudioGenerator.cxx b/Source/cmGlobalVisualStudioGenerator.cxx
index acf616f..6e0b804 100644
--- a/Source/cmGlobalVisualStudioGenerator.cxx
+++ b/Source/cmGlobalVisualStudioGenerator.cxx
@@ -555,9 +555,9 @@ bool IsVisualStudioMacrosFileRegistered(const std::string& macrosFile,
   if (ERROR_SUCCESS == result) {
     // Iterate the subkeys and look for the values of interest in each subkey:
     wchar_t subkeyname[256];
-    DWORD cch_subkeyname = sizeof(subkeyname) * sizeof(subkeyname[0]);
+    DWORD cch_subkeyname = cm::size(subkeyname);
     wchar_t keyclass[256];
-    DWORD cch_keyclass = sizeof(keyclass) * sizeof(keyclass[0]);
+    DWORD cch_keyclass = cm::size(keyclass);
     FILETIME lastWriteTime;
     lastWriteTime.dwHighDateTime = 0;
     lastWriteTime.dwLowDateTime = 0;
@@ -571,8 +571,8 @@ bool IsVisualStudioMacrosFileRegistered(const std::string& macrosFile,
       if (ERROR_SUCCESS == result) {
         DWORD valueType = REG_SZ;
         wchar_t data1[256];
-        DWORD cch_data1 = sizeof(data1) * sizeof(data1[0]);
-        RegQueryValueExW(hsubkey, L"Path", 0, &valueType, (LPBYTE)&data1[0],
+        DWORD cch_data1 = sizeof(data1);
+        RegQueryValueExW(hsubkey, L"Path", 0, &valueType, (LPBYTE)data1,
                          &cch_data1);
 
         DWORD data2 = 0;
@@ -620,8 +620,8 @@ bool IsVisualStudioMacrosFileRegistered(const std::string& macrosFile,
       }
 
       ++index;
-      cch_subkeyname = sizeof(subkeyname) * sizeof(subkeyname[0]);
-      cch_keyclass = sizeof(keyclass) * sizeof(keyclass[0]);
+      cch_subkeyname = cm::size(subkeyname);
+      cch_keyclass = cm::size(keyclass);
       lastWriteTime.dwHighDateTime = 0;
       lastWriteTime.dwLowDateTime = 0;
     }
@@ -646,9 +646,8 @@ bool IsVisualStudioMacrosFileRegistered(const std::string& macrosFile,
   if (ERROR_SUCCESS == result) {
     DWORD valueType = REG_SZ;
     wchar_t data1[256];
-    DWORD cch_data1 = sizeof(data1) * sizeof(data1[0]);
-    RegQueryValueExW(hkey, L"Path", 0, &valueType, (LPBYTE)&data1[0],
-                     &cch_data1);
+    DWORD cch_data1 = sizeof(data1);
+    RegQueryValueExW(hkey, L"Path", 0, &valueType, (LPBYTE)data1, &cch_data1);
 
     DWORD data2 = 0;
     DWORD cch_data2 = sizeof(data2);
diff --git a/Source/cmMakefileTargetGenerator.cxx b/Source/cmMakefileTargetGenerator.cxx
index b1db7c6..f99fe4e 100644
--- a/Source/cmMakefileTargetGenerator.cxx
+++ b/Source/cmMakefileTargetGenerator.cxx
@@ -130,10 +130,10 @@ void cmMakefileTargetGenerator::CreateRuleFile()
   this->BuildFileStream =
     new cmGeneratedFileStream(this->BuildFileNameFull, false,
                               this->GlobalGenerator->GetMakefileEncoding());
-  this->BuildFileStream->SetCopyIfDifferent(true);
   if (!this->BuildFileStream) {
     return;
   }
+  this->BuildFileStream->SetCopyIfDifferent(true);
   this->LocalGenerator->WriteDisclaimer(*this->BuildFileStream);
   if (this->GlobalGenerator->AllowDeleteOnError()) {
     std::vector<std::string> no_depends;
@@ -299,10 +299,10 @@ void cmMakefileTargetGenerator::WriteCommonCodeRules()
   this->FlagFileStream =
     new cmGeneratedFileStream(this->FlagFileNameFull, false,
                               this->GlobalGenerator->GetMakefileEncoding());
-  this->FlagFileStream->SetCopyIfDifferent(true);
   if (!this->FlagFileStream) {
     return;
   }
+  this->FlagFileStream->SetCopyIfDifferent(true);
   this->LocalGenerator->WriteDisclaimer(*this->FlagFileStream);
 
   // Include the flags for the target.
@@ -1012,10 +1012,10 @@ void cmMakefileTargetGenerator::WriteTargetDependRules()
   this->InfoFileNameFull =
     this->LocalGenerator->ConvertToFullPath(this->InfoFileNameFull);
   this->InfoFileStream = new cmGeneratedFileStream(this->InfoFileNameFull);
-  this->InfoFileStream->SetCopyIfDifferent(true);
-  if (!*this->InfoFileStream) {
+  if (!this->InfoFileStream) {
     return;
   }
+  this->InfoFileStream->SetCopyIfDifferent(true);
   this->LocalGenerator->WriteDependLanguageInfo(*this->InfoFileStream,
                                                 this->GeneratorTarget);
 
diff --git a/Source/cmMessageCommand.cxx b/Source/cmMessageCommand.cxx
index 3f33312..39b40b8 100644
--- a/Source/cmMessageCommand.cxx
+++ b/Source/cmMessageCommand.cxx
@@ -70,8 +70,8 @@ bool cmMessageCommand(std::vector<std::string> const& args,
       fatal = true;
       type = MessageType::DEPRECATION_ERROR;
       level = cmake::LogLevel::LOG_ERROR;
-    } else if ((!status.GetMakefile().IsSet("CMAKE_WARN_DEPRECATED") ||
-                status.GetMakefile().IsOn("CMAKE_WARN_DEPRECATED"))) {
+    } else if (!status.GetMakefile().IsSet("CMAKE_WARN_DEPRECATED") ||
+               status.GetMakefile().IsOn("CMAKE_WARN_DEPRECATED")) {
       type = MessageType::DEPRECATION_WARNING;
       level = cmake::LogLevel::LOG_WARNING;
     } else {
diff --git a/Source/cmVSSetupHelper.h b/Source/cmVSSetupHelper.h
index 1bda54a..ad46c75 100644
--- a/Source/cmVSSetupHelper.h
+++ b/Source/cmVSSetupHelper.h
@@ -74,26 +74,8 @@ class SmartBSTR
 {
 public:
   SmartBSTR() { str = NULL; }
-  SmartBSTR(const SmartBSTR& src)
-  {
-    if (src.str != NULL) {
-      str = ::SysAllocStringByteLen((char*)str, ::SysStringByteLen(str));
-    } else {
-      str = ::SysAllocStringByteLen(NULL, 0);
-    }
-  }
-  SmartBSTR& operator=(const SmartBSTR& src)
-  {
-    if (str != src.str) {
-      ::SysFreeString(str);
-      if (src.str != NULL) {
-        str = ::SysAllocStringByteLen((char*)str, ::SysStringByteLen(str));
-      } else {
-        str = ::SysAllocStringByteLen(NULL, 0);
-      }
-    }
-    return *this;
-  }
+  SmartBSTR(const SmartBSTR& src) = delete;
+  SmartBSTR& operator=(const SmartBSTR& src) = delete;
   operator BSTR() const { return str; }
   BSTR* operator&() throw() { return &str; }
   ~SmartBSTR() throw() { ::SysFreeString(str); }