Regkey functions return error code instead of bool

chrome/installer/util/product.cc

 // Copyright (c) 2010 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/installer/util/product.h"
 
 #include <algorithm>
 
 #include "base/command_line.h"
 #include "base/logging.h"
@@ skipping 108 matching lines @@
 
     const MasterPreferences& prefs = MasterPreferences::ForCurrentProcess();
 
     bool is_msi = false;
     prefs.GetBool(installer::master_preferences::kMsi, &is_msi);
 
     if (!is_msi) {
       // 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;
     }
     cache_state_ |= MSI_STATE;
   }
 
   return msi_;
 }
 
 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 {
   if ((cache_state_ & VERSION) == 0) {
     DCHECK(installed_version_.get() == NULL);
     installed_version_.reset(InstallUtil::GetChromeVersion(distribution_,
                                                            system_level()));
     cache_state_ |= VERSION;
   }
 
@@ skipping 91 matching lines @@
   if (!distribution) {
     NOTREACHED();
     return false;
   }
 
   return AddDistribution(distribution);
 }
 
 }  // namespace installer