Module: check_mk
Branch: master
Commit: 6469268e05e3c760e919396c1a1b2083d04e78d3
URL:
http://git.mathias-kettner.de/git/?p=check_mk.git;a=commit;h=6469268e05e3c7…
Author: Jukka Aro <ja(a)mathias-kettner.de>
Date: Fri Dec 22 10:58:36 2017 +0100
Windows agent: use RAII for registry handles
Change-Id: I1ca3488be31f6ce3279eb16f6b095bd96741186f
---
agents/windows/Environment.cc | 7 +++----
agents/windows/EventLog.cc | 14 +++++++-------
agents/windows/PerfCounter.cc | 3 ++-
agents/windows/build_version | 2 +-
agents/windows/sections/SectionEventlog.cc | 9 ++++-----
agents/windows/test/EnvironmentTest.cc | 2 +-
agents/windows/test/typesTest.cc | 18 ++++++++++++++++++
agents/windows/types.h | 10 ++++++++++
8 files changed, 46 insertions(+), 19 deletions(-)
diff --git a/agents/windows/Environment.cc b/agents/windows/Environment.cc
index b9075b2..26d5a90 100644
--- a/agents/windows/Environment.cc
+++ b/agents/windows/Environment.cc
@@ -109,16 +109,15 @@ string Environment::determineAgentDirectory(bool use_cwd) const {
&key);
}
- // TODO: wrap registry handling properly
- OnScopeExit close_key([&]() { _winapi.RegCloseKey(key); });
+ HKeyHandle hKey{key, _winapi};
if (ret == ERROR_SUCCESS) {
vector<unsigned char> buffer(MAX_PATH_UNICODE, '\0');
DWORD dsize = MAX_PATH_UNICODE;
if (ERROR_SUCCESS ==
- _winapi.RegQueryValueEx(key, "ImagePath", NULL, NULL,
buffer.data(),
- &dsize)) {
+ _winapi.RegQueryValueEx(hKey.get(), "ImagePath", NULL, NULL,
+ buffer.data(), &dsize)) {
buffer.resize(dsize);
string directory{buffer.begin(), buffer.end()};
// search backwards for backslash
diff --git a/agents/windows/EventLog.cc b/agents/windows/EventLog.cc
index b890d09..c2b1ac0 100644
--- a/agents/windows/EventLog.cc
+++ b/agents/windows/EventLog.cc
@@ -71,7 +71,7 @@ vector<wstring> MessageResolver::getMessageFiles(LPCWSTR source)
const {
wstring(L"SYSTEM\\CurrentControlSet\\Services\\EventLog");
wstring regpath = base + L"\\" + _name + L"\\" + source;
- HKEY key;
+ HKEY key = nullptr;
DWORD ret = _winapi.RegOpenKeyExW(HKEY_LOCAL_MACHINE, regpath.c_str(), 0,
KEY_READ, &key);
if (ret != ERROR_SUCCESS) {
@@ -79,19 +79,19 @@ vector<wstring> MessageResolver::getMessageFiles(LPCWSTR source)
const {
return vector<wstring>();
}
- // TODO: wrap registry handling properly
- OnScopeExit close_key([&]() { _winapi.RegCloseKey(key); });
+ HKeyHandle hKey{key, _winapi};
DWORD size = 64;
vector<BYTE> buffer(size);
// first try with fixed-size buffer
- DWORD res = _winapi.RegQueryValueExW(key, L"EventMessageFile", nullptr,
- nullptr, &buffer[0], &size);
+ DWORD res =
+ _winapi.RegQueryValueExW(hKey.get(), L"EventMessageFile", nullptr,
+ nullptr, buffer.data(), &size);
if (res == ERROR_MORE_DATA) {
buffer.resize(size);
// actual read
- res = _winapi.RegQueryValueExW(key, L"EventMessageFile", nullptr,
- nullptr, &buffer[0], &size);
+ res = _winapi.RegQueryValueExW(hKey.get(), L"EventMessageFile",
nullptr,
+ nullptr, buffer.data(), &size);
}
if (res != ERROR_SUCCESS) {
Error(_logger) << "failed to read at EventMessageFile in HKLM:%ls :
%s"
diff --git a/agents/windows/PerfCounter.cc b/agents/windows/PerfCounter.cc
index ed73071..185169d 100644
--- a/agents/windows/PerfCounter.cc
+++ b/agents/windows/PerfCounter.cc
@@ -9,6 +9,7 @@
#include "Logger.h"
#include "PerfCounterCommon.h"
#include "stringutil.h"
+#include "types.h"
#include "win_error.h"
// Helper functions to navigate the performance counter data
@@ -203,7 +204,7 @@ std::vector<BYTE> PerfCounterObject::retrieveCounterData(
// to be closed manually, otherwise we may be blocking installation of apps
// that create new performance counters.
// say WHAT???
- _winapi.RegCloseKey(HKEY_PERFORMANCE_DATA);
+ HKeyHandle hKey{HKEY_PERFORMANCE_DATA, _winapi};
result.resize(buffer_size);
return result;
diff --git a/agents/windows/build_version b/agents/windows/build_version
index e3c07a4..45a020f 100644
--- a/agents/windows/build_version
+++ b/agents/windows/build_version
@@ -1 +1 @@
-3030
+3032
diff --git a/agents/windows/sections/SectionEventlog.cc
b/agents/windows/sections/SectionEventlog.cc
index 89feb22..609994d 100644
--- a/agents/windows/sections/SectionEventlog.cc
+++ b/agents/windows/sections/SectionEventlog.cc
@@ -253,10 +253,10 @@ bool SectionEventlog::find_eventlogs(std::ostream &out) {
char regpath[128];
snprintf(regpath, sizeof(regpath),
"SYSTEM\\CurrentControlSet\\Services\\Eventlog");
- HKEY key;
+ HKEY key = nullptr;
DWORD ret = _winapi.RegOpenKeyEx(HKEY_LOCAL_MACHINE, regpath, 0,
KEY_ENUMERATE_SUB_KEYS, &key);
-
+ HKeyHandle hKey{key, _winapi};
bool success = true;
if (ret == ERROR_SUCCESS) {
DWORD i = 0;
@@ -264,8 +264,8 @@ bool SectionEventlog::find_eventlogs(std::ostream &out) {
DWORD len;
while (true) {
len = sizeof(buffer);
- DWORD r = _winapi.RegEnumKeyEx(key, i, buffer, &len, NULL, NULL,
- NULL, NULL);
+ DWORD r = _winapi.RegEnumKeyEx(hKey.get(), i, buffer, &len, NULL,
+ NULL, NULL, NULL);
if (r == ERROR_SUCCESS)
registerEventlog(buffer);
else if (r != ERROR_MORE_DATA) {
@@ -279,7 +279,6 @@ bool SectionEventlog::find_eventlogs(std::ostream &out) {
}
i++;
}
- _winapi.RegCloseKey(key);
} else {
success = false;
const auto lastError = _winapi.GetLastError();
diff --git a/agents/windows/test/EnvironmentTest.cc
b/agents/windows/test/EnvironmentTest.cc
index 520ca2a..e82ef7a 100644
--- a/agents/windows/test/EnvironmentTest.cc
+++ b/agents/windows/test/EnvironmentTest.cc
@@ -30,7 +30,7 @@ TEST_F(wa_EnvironmentTest, constructor_CurrentDirectory) {
TEST_F(wa_EnvironmentTest, constructor_AgentDirectory_use_cwd) {
EXPECT_CALL(_mockwinapi, RegOpenKeyEx(_, _, _, _, _)).Times(0);
EXPECT_CALL(_mockwinapi, RegQueryValueEx(_, _, _, _, _, _)).Times(0);
- EXPECT_CALL(_mockwinapi, RegCloseKey(_)).Times(1);
+ EXPECT_CALL(_mockwinapi, RegCloseKey(_)).Times(0);
const ::Environment testEnvironment(true, &_mocklogger, _mockwinapi);
EXPECT_EQ(testEnvironment.currentDirectory(),
testEnvironment.agentDirectory());
diff --git a/agents/windows/test/typesTest.cc b/agents/windows/test/typesTest.cc
index c729257..aa6eccd 100644
--- a/agents/windows/test/typesTest.cc
+++ b/agents/windows/test/typesTest.cc
@@ -307,3 +307,21 @@ TEST_F(wa_WrappedHandleTest, NullHandleTraits_valid_handle) {
ASSERT_EQ(rawHandle, testHandle.get());
}
}
+
+class wa_HKeyHandleTest : public wa_typesTest {};
+
+TEST_F(wa_HKeyHandleTest, valid_key) {
+ HKEY rawKey = reinterpret_cast<HKEY>(0x1);
+ const char *testPath = "foo\\bar";
+ EXPECT_CALL(_mockwinapi, RegOpenKeyEx(HKEY_LOCAL_MACHINE, testPath, 0,
+ KEY_ENUMERATE_SUB_KEYS, _))
+ .WillOnce(DoAll(SetArgPointee<4>(rawKey), Return(ERROR_SUCCESS)));
+ EXPECT_CALL(_mockwinapi, RegCloseKey(rawKey));
+ {
+ HKEY key = nullptr;
+ auto result = _mockwinapi.RegOpenKeyEx(HKEY_LOCAL_MACHINE, testPath, 0,
+ KEY_ENUMERATE_SUB_KEYS, &key);
+ ASSERT_EQ(ERROR_SUCCESS, result);
+ HKeyHandle testKey{key, _mockwinapi};
+ }
+}
diff --git a/agents/windows/types.h b/agents/windows/types.h
index 29f1540..e6d5b0c 100644
--- a/agents/windows/types.h
+++ b/agents/windows/types.h
@@ -460,4 +460,14 @@ using HModuleHandle = WrappedHandle<HModuleTraits>;
template <int exitCode>
using JobHandle = WrappedHandle<JobHandleTraits<exitCode>>;
+struct HKeyHandleTraits {
+ using HandleT = HKEY;
+ static HandleT invalidValue() { return nullptr; }
+
+ static void closeHandle(HandleT value, const WinApiAdaptor &winapi) {
+ winapi.RegCloseKey(value);
+ }
+};
+
+using HKeyHandle = WrappedHandle<HKeyHandleTraits>;
#endif // types_h