Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1391)

Issues Search
    My Issues | Starred     Open | Closed | All

Unified Diff: chrome_elf/nt_registry/nt_registry.cc

Issue 2507263002: Make nt_registry Create/OpenRegKey return a scoped object
Patch Set: fixes Created 3 years, 10 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« chrome/install_static/install_util.cc ('K') | « chrome_elf/nt_registry/nt_registry.h ('k') | chrome_elf/nt_registry/nt_registry_unittest.cc » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: chrome_elf/nt_registry/nt_registry.cc
diff --git a/chrome_elf/nt_registry/nt_registry.cc b/chrome_elf/nt_registry/nt_registry.cc
index 4c65af8daff7c76ea5a9e4a0255da0225aeb58a4..58b0e9cd6fab5ad2035b10d736ebde2f93127251 100644
--- a/chrome_elf/nt_registry/nt_registry.cc
+++ b/chrome_elf/nt_registry/nt_registry.cc
@@ -565,19 +565,43 @@ NTSTATUS CreateKeyWrapper(const std::wstring& key_path,
namespace nt {
+ScopedHANDLE::ScopedHANDLE() : handle_(nullptr) {}
+
+ScopedHANDLE::ScopedHANDLE(HANDLE handle) : handle_(handle) {}
+
+ScopedHANDLE::~ScopedHANDLE() {
+ if (!g_initialized)
+ InitNativeRegApi();
+ if (is_valid())
+ g_nt_close(handle_);
+}
+
+bool ScopedHANDLE::is_valid() const {
+ // This class should only be used for HANDLEs that have an invalid value of
+ // nullptr.
+ assert(handle_ != INVALID_HANDLE_VALUE);
+
+ return handle_ != nullptr;
+}
+
+HANDLE ScopedHANDLE::release() {
+ HANDLE result = handle_;
+ handle_ = nullptr;
+ return result;
+}
+
//------------------------------------------------------------------------------
// Create, open, delete, close functions
//------------------------------------------------------------------------------
-bool CreateRegKey(ROOT_KEY root,
- const wchar_t* key_path,
- ACCESS_MASK access,
- HANDLE* out_handle OPTIONAL) {
+ScopedHANDLE CreateRegKey(ROOT_KEY root,
+ const wchar_t* key_path,
+ ACCESS_MASK access) {
// |key_path| can be null or empty, but it can't be longer than
// |g_kRegMaxPathLen| at this point.
if (key_path != nullptr &&
::wcsnlen(key_path, g_kRegMaxPathLen + 1) == g_kRegMaxPathLen + 1)
- return false;
+ return ScopedHANDLE();
if (!g_initialized)
InitNativeRegApi();
@@ -596,14 +620,14 @@ bool CreateRegKey(ROOT_KEY root,
std::vector<std::wstring> subkeys;
if (!ParseFullRegPath(ConvertRootKey(root), redirected_key_path,
&current_path, &subkeys))
- return false;
+ return ScopedHANDLE();
// Open the base hive first. It should always exist already.
- HANDLE last_handle = INVALID_HANDLE_VALUE;
+ HANDLE last_handle = nullptr;
NTSTATUS status =
CreateKeyWrapper(current_path, access, &last_handle, nullptr);
if (!NT_SUCCESS(status))
- return false;
+ return ScopedHANDLE();
size_t subkeys_size = subkeys.size();
if (subkeys_size != 0)
@@ -618,7 +642,7 @@ bool CreateRegKey(ROOT_KEY root,
// Process the latest subkey.
ULONG created = 0;
- HANDLE key_handle = INVALID_HANDLE_VALUE;
+ HANDLE key_handle = nullptr;
status =
CreateKeyWrapper(current_path.c_str(), access, &key_handle, &created);
if (!NT_SUCCESS(status)) {
@@ -648,35 +672,29 @@ bool CreateRegKey(ROOT_KEY root,
g_nt_close(handle);
}
if (!success)
- return false;
-
- // See if caller wants the handle left open.
- if (out_handle)
- *out_handle = last_handle;
- else
- g_nt_close(last_handle);
+ return ScopedHANDLE();
- return true;
+ return ScopedHANDLE(last_handle);
}
-bool OpenRegKey(ROOT_KEY root,
- const wchar_t* key_path,
- ACCESS_MASK access,
- HANDLE* out_handle,
- NTSTATUS* error_code OPTIONAL) {
+ScopedHANDLE OpenRegKey(ROOT_KEY root,
+ const wchar_t* key_path,
+ ACCESS_MASK access,
+ NTSTATUS* error_code OPTIONAL) {
+ if (error_code)
+ *error_code = STATUS_UNSUCCESSFUL;
+
// |key_path| can be null or empty, but it can't be longer than
// |g_kRegMaxPathLen| at this point.
if (key_path != nullptr &&
::wcsnlen(key_path, g_kRegMaxPathLen + 1) == g_kRegMaxPathLen + 1)
- return false;
+ return ScopedHANDLE();
if (!g_initialized)
InitNativeRegApi();
- NTSTATUS status = STATUS_UNSUCCESSFUL;
UNICODE_STRING key_path_uni = {};
OBJECT_ATTRIBUTES obj = {};
- *out_handle = INVALID_HANDLE_VALUE;
if (root == nt::AUTO)
root = g_system_install ? nt::HKLM : nt::HKCU;
@@ -693,53 +711,36 @@ bool OpenRegKey(ROOT_KEY root,
InitializeObjectAttributes(&obj, &key_path_uni, OBJ_CASE_INSENSITIVE, NULL,
NULL);
- status = g_nt_open_key_ex(out_handle, access, &obj, 0);
- // See if caller wants the NTSTATUS.
- if (error_code)
- *error_code = status;
+ HANDLE result_handle = nullptr;
+ NTSTATUS status = g_nt_open_key_ex(&result_handle, access, &obj, 0);
- if (NT_SUCCESS(status))
- return true;
+ if (!NT_SUCCESS(status)) {
+ // See if caller wants the NTSTATUS.
+ if (error_code)
+ *error_code = status;
+ return ScopedHANDLE();
+ }
- return false;
+ return ScopedHANDLE(result_handle);
}
bool DeleteRegKey(HANDLE key) {
if (!g_initialized)
InitNativeRegApi();
- NTSTATUS status = STATUS_UNSUCCESSFUL;
-
- status = g_nt_delete_key(key);
-
- if (NT_SUCCESS(status))
- return true;
-
- return false;
+ return NT_SUCCESS(g_nt_delete_key(key));
}
// wrapper function
bool DeleteRegKey(ROOT_KEY root,
WOW64_OVERRIDE wow64_override,
const wchar_t* key_path) {
- HANDLE key = INVALID_HANDLE_VALUE;
-
- if (!OpenRegKey(root, key_path, DELETE | wow64_override, &key, nullptr))
- return false;
-
- if (!DeleteRegKey(key)) {
- CloseRegKey(key);
+ ScopedHANDLE key =
+ OpenRegKey(root, key_path, DELETE | wow64_override, nullptr);
+ if (!key.is_valid())
return false;
- }
- CloseRegKey(key);
- return true;
-}
-
-void CloseRegKey(HANDLE key) {
- if (!g_initialized)
- InitNativeRegApi();
- g_nt_close(key);
+ return DeleteRegKey(key.get());
}
//------------------------------------------------------------------------------
@@ -810,18 +811,12 @@ bool QueryRegValueDWORD(ROOT_KEY root,
const wchar_t* key_path,
const wchar_t* value_name,
DWORD* out_dword) {
- HANDLE key = INVALID_HANDLE_VALUE;
-
- if (!OpenRegKey(root, key_path, KEY_QUERY_VALUE | wow64_override, &key, NULL))
+ ScopedHANDLE key =
+ OpenRegKey(root, key_path, KEY_QUERY_VALUE | wow64_override, nullptr);
+ if (!key.is_valid())
return false;
- if (!QueryRegValueDWORD(key, value_name, out_dword)) {
- CloseRegKey(key);
- return false;
- }
-
- CloseRegKey(key);
- return true;
+ return QueryRegValueDWORD(key.get(), value_name, out_dword);
}
// wrapper function
@@ -833,8 +828,9 @@ bool QueryRegValueSZ(HANDLE key,
ULONG type = REG_NONE;
if (!QueryRegKeyValue(key, value_name, &type, &value_bytes, &ret_size) ||
- type != REG_SZ)
+ type != REG_SZ) {
return false;
+ }
*out_sz = reinterpret_cast<wchar_t*>(value_bytes);
@@ -848,18 +844,12 @@ bool QueryRegValueSZ(ROOT_KEY root,
const wchar_t* key_path,
const wchar_t* value_name,
std::wstring* out_sz) {
- HANDLE key = INVALID_HANDLE_VALUE;
-
- if (!OpenRegKey(root, key_path, KEY_QUERY_VALUE | wow64_override, &key, NULL))
+ ScopedHANDLE key =
+ OpenRegKey(root, key_path, KEY_QUERY_VALUE | wow64_override, nullptr);
+ if (!key.is_valid())
return false;
- if (!QueryRegValueSZ(key, value_name, out_sz)) {
- CloseRegKey(key);
- return false;
- }
-
- CloseRegKey(key);
- return true;
+ return QueryRegValueSZ(key.get(), value_name, out_sz);
}
// wrapper function
@@ -902,18 +892,12 @@ bool QueryRegValueMULTISZ(ROOT_KEY root,
const wchar_t* key_path,
const wchar_t* value_name,
std::vector<std::wstring>* out_multi_sz) {
- HANDLE key = INVALID_HANDLE_VALUE;
-
- if (!OpenRegKey(root, key_path, KEY_QUERY_VALUE | wow64_override, &key, NULL))
+ ScopedHANDLE key =
+ OpenRegKey(root, key_path, KEY_QUERY_VALUE | wow64_override, nullptr);
+ if (!key.is_valid())
return false;
- if (!QueryRegValueMULTISZ(key, value_name, out_multi_sz)) {
- CloseRegKey(key);
- return false;
- }
-
- CloseRegKey(key);
- return true;
+ return QueryRegValueMULTISZ(key.get(), value_name, out_multi_sz);
}
//------------------------------------------------------------------------------
@@ -954,17 +938,12 @@ bool SetRegValueDWORD(ROOT_KEY root,
const wchar_t* key_path,
const wchar_t* value_name,
DWORD value) {
- HANDLE key = INVALID_HANDLE_VALUE;
-
- if (!OpenRegKey(root, key_path, KEY_SET_VALUE | wow64_override, &key, NULL))
+ ScopedHANDLE key =
+ OpenRegKey(root, key_path, KEY_SET_VALUE | wow64_override, nullptr);
+ if (!key.is_valid())
return false;
- if (!SetRegValueDWORD(key, value_name, value)) {
- CloseRegKey(key);
- return false;
- }
-
- return true;
+ return SetRegValueDWORD(key.get(), value_name, value);
}
// wrapper function
@@ -987,17 +966,12 @@ bool SetRegValueSZ(ROOT_KEY root,
const wchar_t* key_path,
const wchar_t* value_name,
const std::wstring& value) {
- HANDLE key = INVALID_HANDLE_VALUE;
-
- if (!OpenRegKey(root, key_path, KEY_SET_VALUE | wow64_override, &key, NULL))
+ ScopedHANDLE key =
+ OpenRegKey(root, key_path, KEY_SET_VALUE | wow64_override, nullptr);
+ if (!key.is_valid())
return false;
- if (!SetRegValueSZ(key, value_name, value)) {
- CloseRegKey(key);
- return false;
- }
-
- return true;
+ return SetRegValueSZ(key.get(), value_name, value);
}
// wrapper function
@@ -1037,17 +1011,12 @@ bool SetRegValueMULTISZ(ROOT_KEY root,
const wchar_t* key_path,
const wchar_t* value_name,
const std::vector<std::wstring>& values) {
- HANDLE key = INVALID_HANDLE_VALUE;
-
- if (!OpenRegKey(root, key_path, KEY_SET_VALUE | wow64_override, &key, NULL))
+ ScopedHANDLE key =
+ OpenRegKey(root, key_path, KEY_SET_VALUE | wow64_override, nullptr);
+ if (!key.is_valid())
return false;
- if (!SetRegValueMULTISZ(key, value_name, values)) {
- CloseRegKey(key);
- return false;
- }
-
- return true;
+ return SetRegValueMULTISZ(key.get(), value_name, values);
}
//------------------------------------------------------------------------------
« chrome/install_static/install_util.cc ('K') | « chrome_elf/nt_registry/nt_registry.h ('k') | chrome_elf/nt_registry/nt_registry_unittest.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698