Regkey functions return error code instead of bool

chrome/installer/util/product.cc

Index: chrome/installer/util/product.cc
===================================================================
--- chrome/installer/util/product.cc	(revision 70414)
+++ chrome/installer/util/product.cc	(working copy)
@@ -126,12 +126,12 @@
       // We didn't find it in the preferences, try looking in the registry.
       HKEY reg_root = system_level() ? HKEY_LOCAL_MACHINE : HKEY_CURRENT_USER;
       RegKey key;
-      if (key.Open(reg_root, distribution_->GetStateKey().c_str(), KEY_READ)) {
-        DWORD msi_value;
-        if (key.ReadValueDW(google_update::kRegMSIField, &msi_value) &&
-            msi_value != 0) {
-          msi_ = true;
-        }
+      LONG result = key.Open(reg_root, distribution_->GetStateKey().c_str(),
+                             KEY_READ);
+      if (result == ERROR_SUCCESS) {
+        DWORD msi_value = 0;
+        result = key.ReadValueDW(google_update::kRegMSIField, &msi_value);

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

I prefer an if (result == ERROR_SUCCESS) before relying on msi_value.

+        msi_ = msi_value != 0;
       }
     } else {
       msi_ = true;
@@ -143,22 +143,21 @@
 }
 
 bool Product::SetMsiMarker(bool set) const {
-  bool success = false;
   HKEY reg_root = system_level() ? HKEY_LOCAL_MACHINE : HKEY_CURRENT_USER;
   RegKey client_state_key;
-  if (client_state_key.Open(reg_root, distribution_->GetStateKey().c_str(),
-                            KEY_READ | KEY_WRITE)) {
-    DWORD msi_value = set ? 1 : 0;
-    if (client_state_key.WriteValue(google_update::kRegMSIField, msi_value)) {
-      success = true;
-    } else {
-      LOG(ERROR) << "Could not write MSI value to client state key.";
-    }
-  } else {
-    LOG(ERROR) << "SetMsiMarker: Could not open client state key!";
+  LONG result = client_state_key.Open(reg_root,
+      distribution_->GetStateKey().c_str(), KEY_READ | KEY_WRITE);
+  if (result == ERROR_SUCCESS) {
+    result = client_state_key.WriteValue(google_update::kRegMSIField,
+                                         set ? 1 : 0);
   }
 
-  return success;
+  if (result != ERROR_SUCCESS) {
+    LOG(ERROR) << "Failed to Open or Write MSI value to client state key."

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

LOG_IF(ERROR, result != ERROR_SUCCESS) << ...

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

Done.

+               << " error: " << result;

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

com::LogWe(result)  (i love this!)

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

com::LogWe is cool and I have changed to it in CEEE. However, I am not sure if
we want to make installer depend on a CEEE lib. Maybe a first step is to factor
that out in a common lib elsewhere?

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

Yeah; I didn't notice that it was in a CEEE lib.

+  }
+
+  return (result == ERROR_SUCCESS);
 }
 
 const Version* Product::GetInstalledVersion() const {