Regkey functions return error code instead of bool

chrome/installer/util/google_update_settings.cc

Index: chrome/installer/util/google_update_settings.cc
===================================================================
--- chrome/installer/util/google_update_settings.cc	(revision 70414)
+++ chrome/installer/util/google_update_settings.cc	(working copy)
@@ -40,9 +40,9 @@
   BrowserDistribution* dist = BrowserDistribution::GetDistribution();
   std::wstring reg_path = dist->GetStateKey();
   RegKey key(HKEY_CURRENT_USER, reg_path.c_str(), KEY_READ);
-  if (!key.ReadValue(name, value)) {
+  if (key.ReadValue(name, value) != ERROR_SUCCESS) {
     RegKey hklm_key(HKEY_LOCAL_MACHINE, reg_path.c_str(), KEY_READ);
-    return hklm_key.ReadValue(name, value);
+    return (hklm_key.ReadValue(name, value) == ERROR_SUCCESS);
   }
   return true;
 }
@@ -52,7 +52,7 @@
   BrowserDistribution* dist = BrowserDistribution::GetDistribution();
   std::wstring reg_path = dist->GetStateKey();
   RegKey key(HKEY_CURRENT_USER, reg_path.c_str(), KEY_READ | KEY_WRITE);
-  return key.WriteValue(name, value.c_str());
+  return key.WriteValue(name, value.c_str()) == ERROR_SUCCESS;
 }
 
 bool ClearGoogleUpdateStrKey(const wchar_t* const name) {
@@ -60,9 +60,9 @@
   std::wstring reg_path = dist->GetStateKey();
   RegKey key(HKEY_CURRENT_USER, reg_path.c_str(), KEY_READ | KEY_WRITE);
   std::wstring value;
-  if (!key.ReadValue(name, &value))
+  if (key.ReadValue(name, &value) != ERROR_SUCCESS)
     return false;
-  return key.WriteValue(name, L"");
+  return key.WriteValue(name, L"") == ERROR_SUCCESS;
 }
 
 bool RemoveGoogleUpdateStrKey(const wchar_t* const name) {
@@ -71,18 +71,20 @@
   RegKey key(HKEY_CURRENT_USER, reg_path.c_str(), KEY_READ | KEY_WRITE);
   if (!key.ValueExists(name))
     return true;
-  return key.DeleteValue(name);
+  return key.DeleteValue(name) == ERROR_SUCCESS;
 }
 
 EulaSearchResult HasEULASetting(HKEY root, const std::wstring& state_key,
                                 bool setting) {
   RegKey key;
-  DWORD previous_value;
+  DWORD previous_value = setting ? 1 : 0;
 
-  if (!key.Open(root, state_key.c_str(), KEY_QUERY_VALUE))
+  if (key.Open(root, state_key.c_str(), KEY_QUERY_VALUE) != ERROR_SUCCESS)
     return NO_SETTING;
 
-  if (!key.ReadValueDW(google_update::kRegEULAAceptedField, &previous_value))
+  LONG result = key.ReadValueDW(google_update::kRegEULAAceptedField,
+                                &previous_value);
+  if (result != ERROR_SUCCESS)
     return FOUND_CLIENT_STATE;
 
   return ((previous_value != 0) == setting) ?
@@ -95,11 +97,11 @@
   BrowserDistribution* dist = BrowserDistribution::GetDistribution();
   std::wstring reg_path = dist->GetStateKey();
   RegKey key(HKEY_CURRENT_USER, reg_path.c_str(), KEY_READ);
-  DWORD value;
-  if (!key.ReadValueDW(google_update::kRegUsageStatsField, &value)) {
+  DWORD value = 0;
+  LONG result = key.ReadValueDW(google_update::kRegUsageStatsField, &value);
+  if (result != ERROR_SUCCESS) {
     RegKey hklm_key(HKEY_LOCAL_MACHINE, reg_path.c_str(), KEY_READ);

[grt (UTC plus 2), 2011/01/11 03:51:30]

how about use key.Open() rather than making a new instance?

[amit, 2011/01/12 04:11:23]

Done.

-    if (!hklm_key.ReadValueDW(google_update::kRegUsageStatsField, &value))
-      return false;
+    hklm_key.ReadValueDW(google_update::kRegUsageStatsField, &value);

[grt (UTC plus 2), 2011/01/11 03:51:30]

I prefer checking the return value here rather than relying on "value" remaining
0.

   }
   return (1 == value);
 }
@@ -114,7 +116,8 @@
   // Writing to HKCU is used both by chrome and by the crash reporter.
   reg_path = dist->GetStateKey();
   RegKey key_hkcu(HKEY_CURRENT_USER, reg_path.c_str(), KEY_READ | KEY_WRITE);

[grt (UTC plus 2), 2011/01/11 03:51:30]

Change to use only one RegKey instance?

[amit, 2011/01/12 04:11:23]

Done.

-  return key_hkcu.WriteValue(google_update::kRegUsageStatsField, value);
+  LONG result = key_hkcu.WriteValue(google_update::kRegUsageStatsField, value);
+  return result == ERROR_SUCCESS;
 }
 
 bool GoogleUpdateSettings::GetMetricsId(std::wstring* metrics_id) {
@@ -156,7 +159,9 @@
     }
   }
   RegKey key(HKEY_LOCAL_MACHINE, reg_path.c_str(), KEY_SET_VALUE);
-  return key.WriteValue(google_update::kRegEULAAceptedField, consented ? 1 : 0);
+  LONG result = key.WriteValue(google_update::kRegEULAAceptedField,
+                               consented ? 1 : 0);
+  return result == ERROR_SUCCESS;
 }
 
 int GoogleUpdateSettings::GetLastRunTime() {
@@ -247,8 +252,8 @@
   std::wstring reg_key(google_update::kRegPathClientState);
   reg_key.append(L"\\");
   reg_key.append(product_guid);
-  if (!key.Open(reg_root, reg_key.c_str(), KEY_ALL_ACCESS) ||
-      !channel_info.Initialize(key)) {
+  LONG result = key.Open(reg_root, reg_key.c_str(), KEY_ALL_ACCESS);
+  if (result != ERROR_SUCCESS || !channel_info.Initialize(key)) {
     VLOG(1) << "Application key not found.";
     if (!incremental_install || !install_return_code) {
       VLOG(1) << "Returning without changing application key.";
@@ -256,8 +261,11 @@
       return;
     } else if (!key.Valid()) {
       reg_key.assign(google_update::kRegPathClientState);
-      if (!key.Open(reg_root, reg_key.c_str(), KEY_ALL_ACCESS) ||
-          !key.CreateKey(product_guid.c_str(), KEY_ALL_ACCESS)) {
+      result = key.Open(reg_root, reg_key.c_str(), KEY_ALL_ACCESS);
+      if (result != ERROR_SUCCESS)

[grt (UTC plus 2), 2011/01/11 03:51:30]

result == ERROR_SUCCESS

[amit, 2011/01/12 04:11:23]

Are you sure? As per the old code, key.CreateKey will execute only if key.Open
fails.

[grt (UTC plus 2), 2011/01/12 15:06:38]

I'm pretty sure CreateKey executes only if Open succeeds (unless the demons are
talking to me again).  My read of the old code is "if we can't Open the
ClientState key or if we can't create the product GUID key beneath it, log and
bail."

[amit, 2011/01/14 05:23:33]

Done.

+        key.CreateKey(product_guid.c_str(), KEY_ALL_ACCESS);

[grt (UTC plus 2), 2011/01/11 03:51:30]

result = key.CreateKey(...)

[grt (UTC plus 2), 2011/01/12 15:06:38]

Ping.

[amit, 2011/01/14 05:23:33]

Done.

+
+      if (result != ERROR_SUCCESS) {
         LOG(ERROR) << "Failed to create application key.";
         key.Close();

[grt (UTC plus 2), 2011/01/12 15:06:38]

Remove

[amit, 2011/01/14 05:23:33]

Done.

         return;