Module: check_mk
Branch: master
Commit: 6f4f60e1c40d01a05c96a3a0456f3f08b394ecd0
URL:
http://git.mathias-kettner.de/git/?p=check_mk.git;a=commit;h=6f4f60e1c40d01…
Author: Jukka Aro <ja(a)mathias-kettner.de>
Date: Fri Dec 15 11:58:33 2017 +0100
Windows agent: Refactor use of OpenHardwareMonitor
* Wrap all raw handles to managing classes.
* Avoid unnecessary use of unique_ptr.
* Use more descriptive names.
* Increase const correctness.
Change-Id: I0cd635340ab28ac91c00c7e9c211bdcb458297a3
---
agents/windows/OHMMonitor.cc | 74 +++++++++++++----------------------
agents/windows/OHMMonitor.h | 35 ++++++++++++-----
agents/windows/build_version | 2 +-
agents/windows/sections/SectionOHM.cc | 9 ++---
agents/windows/sections/SectionOHM.h | 3 +-
5 files changed, 59 insertions(+), 64 deletions(-)
diff --git a/agents/windows/OHMMonitor.cc b/agents/windows/OHMMonitor.cc
index af14903..1b782a0 100644
--- a/agents/windows/OHMMonitor.cc
+++ b/agents/windows/OHMMonitor.cc
@@ -27,83 +27,65 @@
#include "WinApiAdaptor.h"
#include "types.h"
-OHMMonitor::OHMMonitor(const std::string &bin_path, Logger *logger,
- const WinApiAdaptor &winapi)
- : _exe_path(bin_path + "\\OpenHardwareMonitorCLI.exe")
- , _logger(logger)
- , _winapi(winapi) {
- _available = (_winapi.GetFileAttributes(_exe_path.c_str()) !=
- INVALID_FILE_ATTRIBUTES);
-}
-
-OHMMonitor::~OHMMonitor() {
- if (_current_process != INVALID_HANDLE_VALUE) {
- DWORD exitCode = 0;
- if (!_winapi.GetExitCodeProcess(_current_process, &exitCode)) {
- // invalid handle
- _winapi.CloseHandle(_current_process);
- } else {
- if (exitCode == STILL_ACTIVE) {
- // shut down ohm process
- _winapi.TerminateProcess(_current_process, 0);
- }
- _winapi.CloseHandle(_current_process);
- }
- }
-}
+namespace {
HANDLE dev_null(const WinApiAdaptor &winapi) {
- SECURITY_ATTRIBUTES secattr = {};
- secattr.nLength = sizeof(SECURITY_ATTRIBUTES);
- secattr.lpSecurityDescriptor = NULL;
- secattr.bInheritHandle = TRUE;
+ SECURITY_ATTRIBUTES secattr{sizeof(SECURITY_ATTRIBUTES), nullptr, TRUE};
return winapi.CreateFile("nul:", GENERIC_READ | GENERIC_WRITE,
FILE_SHARE_READ | FILE_SHARE_WRITE, &secattr,
OPEN_EXISTING, 0, nullptr);
}
-bool OHMMonitor::checkAvailabe() {
+} // namespace
+
+OHMMonitor::OHMMonitor(const std::string &bin_path, Logger *logger,
+ const WinApiAdaptor &winapi)
+ : _exe_path(bin_path + "\\OpenHardwareMonitorCLI.exe")
+ , _available(winapi.GetFileAttributes(_exe_path.c_str()) !=
+ INVALID_FILE_ATTRIBUTES)
+ , _current_process(winapi)
+ , _logger(logger)
+ , _winapi(winapi) {}
+
+OHMMonitor::~OHMMonitor() {}
+
+bool OHMMonitor::startProcess() {
if (!_available) {
return false;
}
- if (_current_process != INVALID_HANDLE_VALUE) {
- DWORD exitCode = 0;
- if (!_winapi.GetExitCodeProcess(_current_process, &exitCode)) {
- // handle invalid???
- Debug(_logger) << "ohm process handle invalid";
- _winapi.CloseHandle(_current_process);
- _current_process = INVALID_HANDLE_VALUE;
- } else {
+ if (_current_process) {
+ DWORD exitCode = STILL_ACTIVE;
+ if (!_winapi.GetExitCodeProcess(_current_process.get(), &exitCode) ||
+ exitCode != STILL_ACTIVE) {
if (exitCode != STILL_ACTIVE) {
Debug(_logger)
<< "OHM process ended with exit code " <<
exitCode;
- _winapi.CloseHandle(_current_process);
- _current_process = INVALID_HANDLE_VALUE;
}
+ _current_process = {INVALID_HANDLE_VALUE, _winapi};
}
}
- if (_current_process == INVALID_HANDLE_VALUE) {
- STARTUPINFO si = {};
+ if (!_current_process) {
+ STARTUPINFO si{0};
si.cb = sizeof(STARTUPINFO);
si.dwFlags |= STARTF_USESTDHANDLES;
si.hStdOutput = si.hStdError = dev_null(_winapi);
+ WrappedHandle<InvalidHandleTraits> fileHandle{si.hStdOutput, _winapi};
- OnScopeExit close_stdout([&]() { _winapi.CloseHandle(si.hStdOutput); });
-
- PROCESS_INFORMATION pi = {};
+ PROCESS_INFORMATION pi{0};
if (!_winapi.CreateProcess(_exe_path.c_str(), nullptr, nullptr, nullptr,
TRUE, 0, nullptr, nullptr, &si, &pi)) {
Error(_logger) << "failed to run %s" << _exe_path;
return false;
} else {
- _current_process = pi.hProcess;
+ _current_process = {pi.hProcess, _winapi};
Debug(_logger) << "started " << _exe_path <<
" (pid "
<< pi.dwProcessId << ")";
- _winapi.CloseHandle(pi.hThread);
+ WrappedHandle<NullHandleTraits> threadHandle{pi.hThread, _winapi};
}
}
+
return true;
}
diff --git a/agents/windows/OHMMonitor.h b/agents/windows/OHMMonitor.h
index 84cb39e..cb6ebed 100644
--- a/agents/windows/OHMMonitor.h
+++ b/agents/windows/OHMMonitor.h
@@ -27,17 +27,32 @@
#include <winsock2.h>
#include <windows.h>
#include <string>
+#include "types.h"
class Logger;
class WinApiAdaptor;
-/**
- * Ensure the Open Hardware Monitor is running (if it's available)
- **/
+struct OhmProcessHandleTraits {
+ using HandleT = HANDLE;
+ static HandleT invalidValue() { return INVALID_HANDLE_VALUE; }
+
+ static void closeHandle(HandleT value, const WinApiAdaptor &winapi) {
+ DWORD exitCode = 0;
+ if (winapi.GetExitCodeProcess(value, &exitCode) &&
+ exitCode == STILL_ACTIVE) {
+ // shut down ohm process
+ winapi.TerminateProcess(value, 0);
+ }
+ winapi.CloseHandle(value);
+ }
+};
+
+using OhmProcessHandle = WrappedHandle<OhmProcessHandleTraits>;
+
class OHMMonitor {
- std::string _exe_path;
- bool _available;
- HANDLE _current_process{INVALID_HANDLE_VALUE};
+ const std::string _exe_path;
+ const bool _available;
+ OhmProcessHandle _current_process;
Logger *_logger;
const WinApiAdaptor &_winapi;
@@ -46,7 +61,9 @@ public:
const WinApiAdaptor &winapi);
~OHMMonitor();
- // this call actually starts OHM if necessary and returns
- // true if it was already running or was successfully started
- bool checkAvailabe();
+ /**
+ * Ensure the Open Hardware Monitor is running (if it's available).
+ * Return true if it was already running or was successfully started.
+ **/
+ bool startProcess();
};
diff --git a/agents/windows/build_version b/agents/windows/build_version
index 95a3b3e..be8773d 100644
--- a/agents/windows/build_version
+++ b/agents/windows/build_version
@@ -1 +1 @@
-3026
+3028
diff --git a/agents/windows/sections/SectionOHM.cc
b/agents/windows/sections/SectionOHM.cc
index d85d24f..703f447 100644
--- a/agents/windows/sections/SectionOHM.cc
+++ b/agents/windows/sections/SectionOHM.cc
@@ -33,16 +33,13 @@ SectionOHM::SectionOHM(Configuration &config, Logger *logger,
const WinApiAdaptor &winapi)
: SectionWMI("openhardwaremonitor", "openhardwaremonitor",
config.getEnvironment(), logger, winapi)
- , _bin_path(_env.binDirectory()) {
+ , _ohm_monitor(_env.binDirectory(), _logger, _winapi) {
withNamespace(L"Root\\OpenHardwareMonitor");
withObject(L"Sensor");
}
void SectionOHM::startIfAsync() {
- if (_ohm_monitor.get() == nullptr) {
- _ohm_monitor.reset(new OHMMonitor(_bin_path, _logger, _winapi));
- _ohm_monitor->checkAvailabe();
- }
+ _ohm_monitor.startProcess();
}
bool SectionOHM::produceOutputInner(std::ostream &out) {
@@ -54,7 +51,7 @@ bool SectionOHM::produceOutputInner(std::ostream &out) {
Debug(_logger) << "ComException: " << e.what();
res = false;
}
- if (!res && !_ohm_monitor->checkAvailabe()) {
+ if (!res && !_ohm_monitor.startProcess()) {
Debug(_logger)
<< "ohm not installed or not runnable -> section
disabled";
suspend(3600);
diff --git a/agents/windows/sections/SectionOHM.h b/agents/windows/sections/SectionOHM.h
index 8b594b1..8b37671 100644
--- a/agents/windows/sections/SectionOHM.h
+++ b/agents/windows/sections/SectionOHM.h
@@ -42,8 +42,7 @@ protected:
virtual bool produceOutputInner(std::ostream &out) override;
private:
- std::unique_ptr<OHMMonitor> _ohm_monitor;
- std::string _bin_path;
+ OHMMonitor _ohm_monitor;
};
#endif // SectionOHM_h