Merge branch 'coverity/analysis-fixes-1'

Author: planetchili

IntelPresentMon/InterprocessMock/Main.cpp

@@ -301,8 +301,10 @@ int main(int argc, char** argv)
 		std::cout << "Invalid Dynamic Cast Exception: " << e.what();
 		return -1;
 	}
-
-
+	catch (const CLI::BadNameString& e)
+	{
+		return e.get_exit_code();
+	}
 
 	return 0;
 }

IntelPresentMon/PresentMonAPI2Tests/CsvHelper.h

@@ -137,7 +137,6 @@ constexpr char const* GetHeaderString(Header h)
     case Header_DwmNotified:            return "DwmNotified";
     default:                            return "<unknown>";
     }
-    return "<unknown>";
 }
 
 std::wstring CreateErrorString(Header columnId, size_t line)
@@ -316,7 +315,7 @@ class CsvParser {
     size_t line_ = 0;
     std::vector<char const*> cols_;
     v2Metrics v2MetricRow_;
-    uint32_t processId_;
+    uint32_t processId_ = 0;
     std::map<size_t, Header> activeColHeadersMap_;
 };
 
@@ -502,11 +501,11 @@ bool CsvParser::FindFirstRowWithPid(const unsigned int& searchProcessId)
             char const* processIdString = nullptr;
             if (processColIdx < cols_.size()) {
                 processIdString = cols_[Header_ProcessID];
-            }
-            unsigned int currentProcessId = 0;
-            int succeededCount = sscanf_s(processIdString, "%ud", &currentProcessId);
-            if (searchProcessId == currentProcessId) {
-                return true;
+                unsigned int currentProcessId = 0;
+                int succeededCount = sscanf_s(processIdString, "%ud", &currentProcessId);
+                if (searchProcessId == currentProcessId) {
+                    return true;
+                }
             }
         }
     }
@@ -552,15 +551,21 @@ bool CsvParser::Open(std::wstring const& path, uint32_t processId) {
         case UnknownHeader:
             Assert::Fail(CreateErrorString(h, line_).c_str());
         default:
-            if (headerColumnIndex_[(size_t)h] != SIZE_MAX) {
-                std::wstring errorMessage = L"Duplicate column: ";
-                errorMessage += pmon::util::str::ToWide(cols_[i]);
-                Assert::Fail(errorMessage.c_str());
+            if ((size_t)h < KnownHeaderCount) {
+                if (headerColumnIndex_[(size_t)h] != SIZE_MAX) {
+                    std::wstring errorMessage = L"Duplicate column: ";
+                    errorMessage += pmon::util::str::ToWide(cols_[i]);
+                    Assert::Fail(errorMessage.c_str());
+                }
+                else {
+                    headerColumnIndex_[(size_t)h] = i;
+                }
+                break;
             }
             else {
-                headerColumnIndex_[(size_t)h] = i;
+                std::wstring errorMessage = L"Index outside of known headers.";
+                Assert::Fail(errorMessage.c_str());
             }
-            break;
         }
     }
 

IntelPresentMon/PresentMonAPI2Tests/RealtimeMetricTests.cpp

@@ -182,7 +182,7 @@ namespace RealtimeMetricTests
 
 			std::wstring appName = L"PresentBench.exe";
 			std::optional<unsigned int> appPid;
-			GetProcessInformation(appName, appPid);
+			GetProcessInformation(std::move(appName), appPid);
 			if (!appPid.has_value())
 			{
 				Assert::AreEqual(false, true, L"*** Unable to detect PresentBench.exe.");
@@ -241,7 +241,7 @@ namespace RealtimeMetricTests
 
 			std::wstring appName = L"PresentBench.exe";
 			std::optional<unsigned int> appPid;
-			GetProcessInformation(appName, appPid);
+			GetProcessInformation(std::move(appName), appPid);
 			if (!appPid.has_value())
 			{
 				Assert::AreEqual(false, true, L"*** Unable to detect PresentBench.exe.");

IntelPresentMon/PresentMonAPIWrapper/DiagnosticHandler.h

@@ -32,6 +32,9 @@ namespace pmapi
 			// capture source file and line number as a string
 			bool enableLocation = false
 		);
+		DiagnosticHandler(const DiagnosticHandler& other) = delete;
+		DiagnosticHandler(DiagnosticHandler&& other) = delete;
+		DiagnosticHandler& operator=(const DiagnosticHandler& other) = delete;
 		~DiagnosticHandler();
 	private:
 		std::shared_ptr<void> pImpl;

IntelPresentMon/SampleClient/DiagnosticDemo.cpp

@@ -38,11 +38,12 @@ void RunDiagnosticDemo(int mode)
 
 	// basic log info w/ message
 	if (mode == 0) {
+		std::string timeStamp = std::format("{}", std::chrono::zoned_time{ std::chrono::current_zone(), std::chrono::system_clock::now() });
 		PM_DIAGNOSTIC_MESSAGE dm{
 			.level = PM_DIAGNOSTIC_LEVEL_WARNING,
 			.system = PM_DIAGNOSTIC_SUBSYSTEM(PM_DIAGNOSTIC_SUBSYSTEM_USER + 2),
 			.pText = "@#$ test test test",
-			.pTimestamp = std::format("{}", std::chrono::zoned_time{ std::chrono::current_zone(), std::chrono::system_clock::now() }).c_str(),
+			.pTimestamp = timeStamp.c_str(),
 		};
 		pmDiagnosticEnqueueMessage(&dm);
 	}

IntelPresentMon/SampleClient/LogSetup.h

@@ -8,6 +8,9 @@ namespace p2sam
 	{
 		LogChannelManager() noexcept;
 		~LogChannelManager();
+		LogChannelManager(const LogChannelManager& other) = delete;
+		LogChannelManager(LogChannelManager&& other) = delete;
+		LogChannelManager& operator=(const LogChannelManager& other) = delete;
 	};
 	// call after command line arguments have been parsed
 	void ConfigureLogging() noexcept;

PresentData/PresentMonTraceConsumer.cpp

@@ -1918,21 +1918,26 @@ void PMTraceConsumer::AddPresentToCompletedList(std::shared_ptr<PresentEvent> co
     }
 
     UpdateReadyCount(present);
-
-    // It's possible for a deferred condition to never be cleared.  e.g., a process' last present
-    // doesn't get a Present_Stop event.  When this happens the deferred present will prevent all
-    // subsequent presents from other processes from being dequeued until the ring buffer wraps and
-    // forces it out, which is likely longer than we want to wait.  So we check here if there is 
-    // a stuck deferred present and clear the deferral if it gets too old.
-    if (mReadyCount == 0) {
-        auto const& deferredPresent = mCompletedPresents[mCompletedIndex];
-        if (present->PresentStartTime >= deferredPresent->PresentStartTime &&
-            present->PresentStartTime - deferredPresent->PresentStartTime > mDeferralTimeLimit) {
-            VerboseTraceBeforeModifyingPresent(deferredPresent.get());
-            deferredPresent->IsLost = true;
-            deferredPresent->WaitingForPresentStop = false;
-            deferredPresent->WaitingForFlipFrameType = false;
-            UpdateReadyCount(deferredPresent);
+    
+    {
+        std::unique_lock<std::mutex> lock(mPresentEventMutex);
+        // It's possible for a deferred condition to never be cleared.  e.g., a process' last present
+        // doesn't get a Present_Stop event.  When this happens the deferred present will prevent all
+        // subsequent presents from other processes from being dequeued until the ring buffer wraps and
+        // forces it out, which is likely longer than we want to wait.  So we check here if there is 
+        // a stuck deferred present and clear the deferral if it gets too old.
+        if (mReadyCount == 0) {
+            auto const& deferredPresent = mCompletedPresents[mCompletedIndex];
+            if (present->PresentStartTime >= deferredPresent->PresentStartTime &&
+                present->PresentStartTime - deferredPresent->PresentStartTime > mDeferralTimeLimit) {
+                VerboseTraceBeforeModifyingPresent(deferredPresent.get());
+                deferredPresent->IsLost = true;
+                deferredPresent->WaitingForPresentStop = false;
+                deferredPresent->WaitingForFlipFrameType = false;
+                // UpdateReadyCount also locks the mPresentEventMutex so unlock here
+                lock.unlock();
+                UpdateReadyCount(deferredPresent);
+            }
         }
     }
 }
@@ -2379,9 +2384,10 @@ void PMTraceConsumer::DequeueProcessEvents(std::vector<ProcessEvent>& outProcess
 void PMTraceConsumer::DequeuePresentEvents(std::vector<std::shared_ptr<PresentEvent>>& outPresentEvents)
 {
     outPresentEvents.clear();
-    if (mReadyCount > 0) {
-        std::lock_guard<std::mutex> lock(mPresentEventMutex);
 
+    std::lock_guard<std::mutex> lock(mPresentEventMutex);
+
+    if (mReadyCount > 0) {
         outPresentEvents.resize(mReadyCount, nullptr);
         for (uint32_t i = 0; i < mReadyCount; ++i) {
             std::swap(outPresentEvents[i], mCompletedPresents[mCompletedIndex]);