Regkey functions return error code instead of bool

chrome_frame/chrome_tab.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.
 
 // chrome_tab.cc : Implementation of DLL Exports.
 
 // Include without path to make GYP build see it.
 #include "chrome_tab.h"  // NOLINT
 
 #include <atlsecurity.h>
@@ skipping 302 matching lines @@
         channel_name.find(L"ceee") != std::wstring::npos) {
 
       HKEY hive = HKEY_CURRENT_USER;
       if (IsSystemProcess()) {
         // For system installs, our updates will be running as SYSTEM which
         // makes writing to a RunOnce key under HKCU not so terribly useful.
         hive = HKEY_LOCAL_MACHINE;
       }
 
       RegKey run_once;
-      if (run_once.Create(hive, kRunOnce, KEY_READ | KEY_WRITE)) {
+      LONG ret = run_once.Create(hive, kRunOnce, KEY_READ | KEY_WRITE);

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

nit: 
if (run_once.Create(hive, kRunOnce, KEY_READ | KEY_WRITE) == ERROR_SUCCESS) {
would fit on one line, allowing you to do away with ret, at least at this scope
level.

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

Actually, it does not fit on one line but that's not the point here.
SetupRunOnce now returns a proper return code in case of failure.

+      if (ret == ERROR_SUCCESS) {
         CommandLine run_once_cmd(chrome_launcher::GetChromeExecutablePath());
         run_once_cmd.AppendSwitchASCII(switches::kAutomationClientChannelID,
                                        "0");
         run_once_cmd.AppendSwitch(switches::kChromeFrame);
-        if (run_once.WriteValue(L"A",
-                                run_once_cmd.command_line_string().c_str())) {
-          result = S_OK;
-        }
+        ret = run_once.WriteValue(L"A",
+                                  run_once_cmd.command_line_string().c_str());
+        result = HRESULT_FROM_WIN32(ret);
       }
     } else {
       result = S_FALSE;
     }
   } else {
     // We're on a non-XP version of Windows or on a stable channel. Nothing
     // needs doing.
     result = S_FALSE;
   }
 
@@ skipping 81 matching lines @@
 const wchar_t kChromeFramePrefix[] = L"chromeframe/";
 
 // To delete the user agent, set value to NULL.
 // The is_system parameter indicates whether this is a per machine or a per
 // user installation.
 HRESULT SetChromeFrameUA(bool is_system, const wchar_t* value) {
   HRESULT hr = E_FAIL;
   HKEY parent_hive = is_system ? HKEY_LOCAL_MACHINE : HKEY_CURRENT_USER;
 
   RegKey ua_key;
-  if (ua_key.Create(parent_hive, kPostPlatformUAKey, KEY_WRITE)) {
+  LONG result = ua_key.Create(parent_hive, kPostPlatformUAKey, KEY_WRITE);
+  if (result == ERROR_SUCCESS) {
     std::wstring chrome_frame_ua_value_name = kChromeFramePrefix;
     chrome_frame_ua_value_name += GetCurrentModuleVersion();
     if (value) {
       ua_key.WriteValue(chrome_frame_ua_value_name.c_str(), value);
     } else {
       ua_key.DeleteValue(chrome_frame_ua_value_name.c_str());
     }
     hr = S_OK;
   } else {
     DLOG(ERROR) << __FUNCTION__ << ": " << kPostPlatformUAKey;
@@ skipping 177 matching lines @@
 
   // Save given security descriptor to the backup key.
   bool SaveSecurity(const CSecurityDesc& sd) {
     CString str;
     if (!sd.ToString(&str))
       return false;
 
     RegKey backup_key(HKEY_LOCAL_MACHINE, backup_key_name_.c_str(),
                       KEY_READ | KEY_WRITE);
     if (backup_key.Valid()) {
-      return backup_key.WriteValue(NULL, str.GetString());
+      return backup_key.WriteValue(NULL, str.GetString()) == ERROR_SUCCESS;
     }
 
     return false;
   }
 
   // Restore security descriptor from backup key to given key name.
   bool RestoreSecurity(const wchar_t* key_name) {
     std::wstring sddl;
     if (!ReadBackupKey(&sddl))
       return false;
@@ skipping 21 matching lines @@
 
  private:
   // Read SDDL string from backup key
   bool ReadBackupKey(std::wstring* sddl) {
     RegKey backup_key(HKEY_LOCAL_MACHINE, backup_key_name_.c_str(), KEY_READ);
     if (!backup_key.Valid())
       return false;
 
     DWORD len = 0;
     DWORD reg_type = REG_NONE;
-    if (!backup_key.ReadValue(NULL, NULL, &len, &reg_type))
+    if (backup_key.ReadValue(NULL, NULL, &len, &reg_type) != ERROR_SUCCESS)
       return false;
 
     if (reg_type != REG_SZ)
       return false;
 
     size_t wchar_count = 1 + len / sizeof(wchar_t);
-    if (!backup_key.ReadValue(NULL, WriteInto(sddl, wchar_count), &len,
-                              &reg_type)) {
+    if (backup_key.ReadValue(NULL, WriteInto(sddl, wchar_count), &len,
+                             &reg_type) != ERROR_SUCCESS) {
       return false;
     }
 
     return true;
   }
 
   void DeleteBackupKey() {
     ::RegDeleteKey(HKEY_LOCAL_MACHINE, backup_key_name_.c_str());
   }
 
@@ skipping 35 matching lines @@
   CSid user_;
 };
 
 static bool SetOrDeleteMimeHandlerKey(bool set, HKEY root_key) {
   std::wstring key_name = kInternetSettings;
   key_name.append(L"\\Secure Mime Handlers");
   RegKey key(root_key, key_name.c_str(), KEY_READ | KEY_WRITE);
   if (!key.Valid())
     return false;
 
-  bool result;
+  LONG result1 = ERROR_SUCCESS;
+  LONG result2 = ERROR_SUCCESS;
   if (set) {
-    result = key.WriteValue(L"ChromeTab.ChromeActiveDocument", 1);
-    result = key.WriteValue(L"ChromeTab.ChromeActiveDocument.1", 1) && result;
+    result1 = key.WriteValue(L"ChromeTab.ChromeActiveDocument", 1);
+    result2 = key.WriteValue(L"ChromeTab.ChromeActiveDocument.1", 1);
   } else {
-    result = key.DeleteValue(L"ChromeTab.ChromeActiveDocument");
-    result = key.DeleteValue(L"ChromeTab.ChromeActiveDocument.1") && result;
+    result1 = key.DeleteValue(L"ChromeTab.ChromeActiveDocument");
+    result2 = key.DeleteValue(L"ChromeTab.ChromeActiveDocument.1");
   }
 
-  return result;
+  return (result2 == ERROR_SUCCESS) || (result2 == ERROR_SUCCESS);

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

The || should be an &&: previously both writes/deletes had to succeed for this
to return true, now only one of them has to succeed.

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

Done.

 }
 
 bool RegisterSecuredMimeHandler(bool enable, bool is_system) {
   if (!is_system) {
     return SetOrDeleteMimeHandlerKey(enable, HKEY_CURRENT_USER);
   } else if (base::win::GetVersion() < base::win::VERSION_VISTA) {
     return SetOrDeleteMimeHandlerKey(enable, HKEY_LOCAL_MACHINE);
   }
 
   std::wstring mime_key = kInternetSettings;
@@ skipping 27 matching lines @@
     sd.GetDacl(&new_dacl);
     new_dacl.AddAllowedAce(token_.GetUser(), GENERIC_WRITE | GENERIC_READ);
     if (AtlSetDacl(object_name.c_str(), SE_REGISTRY_KEY, new_dacl)) {
       result = SetOrDeleteMimeHandlerKey(enable, HKEY_LOCAL_MACHINE);
     }
   }
 
   backup.RestoreSecurity(object_name.c_str());
   return result;
 }