Regkey functions return error code instead of bool

chrome_frame/utils.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_frame/utils.h"
 
 #include <htiframe.h>
 #include <mshtml.h>
 #include <shlobj.h>
 
@@ skipping 215 matching lines @@
 bool UtilChangePersistentNPAPIMarker(bool set) {
   BrowserDistribution* cf_dist = BrowserDistribution::GetDistribution();
   std::wstring cf_state_key_path(cf_dist->GetStateKey());
 
   RegKey cf_state_key(HKEY_LOCAL_MACHINE, cf_state_key_path.c_str(),
                       KEY_READ | KEY_WRITE);
 
   bool success = false;
   if (cf_state_key.Valid()) {
     if (set) {
-      success = cf_state_key.WriteValue(kChromeFramePersistNPAPIReg, 1);
+      LONG result = cf_state_key.WriteValue(kChromeFramePersistNPAPIReg, 1);
+      success = result == ERROR_SUCCESS;

[robertshield, 2011/01/12 15:01:34]

please add parenthesis around (result == ERROR_SUCCCESS)

     } else {
       // Unfortunately, DeleteValue returns true only if the value
       // previously existed, so we do a separate existence check to
       // validate success.
       cf_state_key.DeleteValue(kChromeFramePersistNPAPIReg);
       success = !cf_state_key.ValueExists(kChromeFramePersistNPAPIReg);
     }
   }
   return success;
 }
 
 bool UtilIsPersistentNPAPIMarkerSet() {
   BrowserDistribution* cf_dist = BrowserDistribution::GetDistribution();
   std::wstring cf_state_key_path(cf_dist->GetStateKey());
 
   RegKey cf_state_key(HKEY_LOCAL_MACHINE, cf_state_key_path.c_str(),
                       KEY_QUERY_VALUE);
 
   bool success = false;
   if (cf_state_key.Valid()) {
     DWORD val = 0;
-    if (cf_state_key.ReadValueDW(kChromeFramePersistNPAPIReg, &val)) {
+    LONG result = cf_state_key.ReadValueDW(kChromeFramePersistNPAPIReg, &val);
+    if (result = ERROR_SUCCESS) {
       success = (val != 0);
     }
   }
   return success;
 }
 
 
 HRESULT UtilGetXUACompatContentValue(const std::wstring& html_string,
                                      std::wstring* content_value) {
   if (!content_value) {
@@ skipping 400 matching lines @@
     ret = true;
   }
 
   return ret;
 }
 
 int GetConfigInt(int default_value, const wchar_t* value_name) {
   int ret = default_value;
   RegKey config_key;
   if (config_key.Open(HKEY_CURRENT_USER, kChromeFrameConfigKey,
-                      KEY_QUERY_VALUE)) {
-    int value = FALSE;
-    if (config_key.ReadValueDW(value_name, reinterpret_cast<DWORD*>(&value))) {
-      ret = value;
-    }
+                      KEY_QUERY_VALUE) == ERROR_SUCCESS) {

[robertshield, 2011/01/12 15:01:34]

Summary comment, I like this style better than using a temporary.

+    config_key.ReadValueDW(value_name, reinterpret_cast<DWORD*>(&ret));
   }
 
   return ret;
 }
 
 bool GetConfigBool(bool default_value, const wchar_t* value_name) {
   DWORD value = GetConfigInt(default_value, value_name);
   return (value != FALSE);
 }
 
 bool SetConfigInt(const wchar_t* value_name, int value) {
   RegKey config_key;
   if (config_key.Create(HKEY_CURRENT_USER, kChromeFrameConfigKey,
-                        KEY_SET_VALUE)) {
-    if (config_key.WriteValue(value_name, value)) {
+                        KEY_SET_VALUE) == ERROR_SUCCESS) {
+    if (config_key.WriteValue(value_name, value) == ERROR_SUCCESS) {
       return true;
     }
   }
 
   return false;
 }
 
 bool SetConfigBool(const wchar_t* value_name, bool value) {
   return SetConfigInt(value_name, value);
 }
 
 bool DeleteConfigValue(const wchar_t* value_name) {
   RegKey config_key;
   if (config_key.Open(HKEY_CURRENT_USER, kChromeFrameConfigKey,
-                      KEY_WRITE)) {
-    return config_key.DeleteValue(value_name);
+                      KEY_WRITE) == ERROR_SUCCESS) {
+    if (config_key.DeleteValue(value_name) == ERROR_SUCCESS) {
+      return true;
+    }
   }
   return false;
 }
 
 bool IsGcfDefaultRenderer() {
   DWORD is_default = 0;  // NOLINT
 
   // First check policy settings
   PolicySettings::RendererForUrl renderer =
       PolicySettings::GetInstance()->default_renderer();
   if (renderer != PolicySettings::RENDERER_NOT_SPECIFIED) {
     is_default = (renderer == PolicySettings::RENDER_IN_CHROME_FRAME);
   } else {
     // TODO(tommi): Implement caching for this config value as it gets
     // checked frequently.
     RegKey config_key;
-    if (config_key.Open(HKEY_CURRENT_USER, kChromeFrameConfigKey, KEY_READ)) {
+    LONG result = config_key.Open(HKEY_CURRENT_USER, kChromeFrameConfigKey,
+                                  KEY_READ);
+    if (result == ERROR_SUCCESS) {
       config_key.ReadValueDW(kEnableGCFRendererByDefault, &is_default);
     }
   }
 
   return is_default != 0;
 }
 
 RendererType RendererTypeForUrl(const std::wstring& url) {
   // First check if the default renderer settings are specified by policy.
   // If so, then that overrides the user settings.
   PolicySettings::RendererForUrl renderer =
       PolicySettings::GetInstance()->GetRendererForUrl(url.c_str());
   if (renderer != PolicySettings::RENDERER_NOT_SPECIFIED) {
     // We may know at this point that policy says do NOT render in Chrome Frame.
     // To maintain consistency, we return RENDERER_TYPE_UNDETERMINED so that
     // content sniffing, etc. still take place.
     // TODO(tommi): Clarify the intent here.
     return (renderer == PolicySettings::RENDER_IN_CHROME_FRAME) ?
         RENDERER_TYPE_CHROME_OPT_IN_URL : RENDERER_TYPE_UNDETERMINED;
   }
 
   RegKey config_key;
-  if (!config_key.Open(HKEY_CURRENT_USER, kChromeFrameConfigKey, KEY_READ))
+  LONG result = config_key.Open(HKEY_CURRENT_USER, kChromeFrameConfigKey,
+                                KEY_READ);
+  if (result != ERROR_SUCCESS)
     return RENDERER_TYPE_UNDETERMINED;
 
   RendererType renderer_type = RENDERER_TYPE_UNDETERMINED;
 
   const wchar_t* url_list_name = NULL;
   int render_in_cf_by_default = FALSE;
   config_key.ReadValueDW(kEnableGCFRendererByDefault,
                          reinterpret_cast<DWORD*>(&render_in_cf_by_default));
   if (render_in_cf_by_default) {
     url_list_name = kRenderInHostUrlList;
@@ skipping 800 matching lines @@
     ++url_list;
   }
 }
 
 std::wstring GetCurrentModuleVersion() {
   scoped_ptr<FileVersionInfo> module_version_info(
       FileVersionInfo::CreateFileVersionInfoForCurrentModule());
   DCHECK(module_version_info.get() != NULL);
   return module_version_info->file_version();
 }