Regkey functions return error code instead of bool

chrome/installer/util/set_reg_value_work_item.cc

Index: chrome/installer/util/set_reg_value_work_item.cc
===================================================================
--- chrome/installer/util/set_reg_value_work_item.cc	(revision 70917)
+++ chrome/installer/util/set_reg_value_work_item.cc	(working copy)
@@ -43,116 +43,104 @@
 }
 
 bool SetRegValueWorkItem::Do() {
-  bool success = true;
-
+  LONG result = ERROR_SUCCESS;
+  base::win::RegKey key;
   if (status_ != SET_VALUE) {
     // we already did something.
     LOG(ERROR) << "multiple calls to Do()";
-    success = false;
+    result = ERROR_CANTWRITE;
+  } else {
+    result = key.Open(predefined_root_, key_path_.c_str(),
+                      KEY_READ | KEY_SET_VALUE);
+    if (result != ERROR_SUCCESS) {
+      LOG(ERROR) << "can not open " << key_path_ << " error: " << result;
+      status_ = VALUE_UNCHANGED;
+    }
   }
 
-  base::win::RegKey key;
-  if (success && !key.Open(predefined_root_, key_path_.c_str(),
-                           KEY_READ | KEY_SET_VALUE)) {
-    LOG(ERROR) << "can not open " << key_path_;
-    status_ = VALUE_UNCHANGED;
-    success = false;
-  }
-
-  if (success) {
+  if (result == ERROR_SUCCESS) {
     if (key.ValueExists(value_name_.c_str())) {
       if (overwrite_) {
         // Read previous value for rollback and write new value
         if (is_str_type_) {
-          std::wstring data;
-          if (key.ReadValue(value_name_.c_str(), &data)) {
-            previous_value_str_.assign(data);
-          }
-          success = key.WriteValue(value_name_.c_str(),
-                                   value_data_str_.c_str());
+          key.ReadValue(value_name_.c_str(), &previous_value_str_);
+          result = key.WriteValue(value_name_.c_str(), value_data_str_.c_str());
         } else {
-          DWORD data;
-          if (key.ReadValueDW(value_name_.c_str(), &data)) {
-            previous_value_dword_ = data;
-          }
-          success = key.WriteValue(value_name_.c_str(), value_data_dword_);
+          key.ReadValueDW(value_name_.c_str(), &previous_value_dword_);
+          result = key.WriteValue(value_name_.c_str(), value_data_dword_);
         }
-        if (success) {
-          VLOG(1) << "overwritten value for " << value_name_;
-          status_ = VALUE_OVERWRITTEN;
-        } else {
-          LOG(ERROR) << "failed to overwrite value for " << value_name_;
-          status_ = VALUE_UNCHANGED;
-        }
+        if (result == ERROR_SUCCESS) {
+            VLOG(1) << "overwritten value for " << value_name_;
+            status_ = VALUE_OVERWRITTEN;
+          } else {
+            LOG(ERROR) << "failed to overwrite value for " << value_name_;
+            status_ = VALUE_UNCHANGED;
+          }
       } else {
+        // value exists and cannot be overwriten
         VLOG(1) << value_name_ << " exists. not changed ";
         status_ = VALUE_UNCHANGED;
       }
     } else {
+      // Value does not exist, simply write a new value
       if (is_str_type_) {
-        success = key.WriteValue(value_name_.c_str(), value_data_str_.c_str());
+        result = key.WriteValue(value_name_.c_str(), value_data_str_.c_str());
       } else {
-        success = key.WriteValue(value_name_.c_str(), value_data_dword_);
+        result = key.WriteValue(value_name_.c_str(), value_data_dword_);
       }
-      if (success) {
+      if (result == ERROR_SUCCESS) {
         VLOG(1) << "created value for " << value_name_;
         status_ = NEW_VALUE_CREATED;
       } else {
-        LOG(ERROR) << "failed to create value for " << value_name_;
+        LOG(ERROR) << "failed to create value for " << value_name_
+                   << " error: " << result;
         status_ = VALUE_UNCHANGED;
       }
     }
   }
 
-  LOG_IF(ERROR, !success && !log_message_.empty()) << log_message_;
-
   if (ignore_failure_) {
-    success = true;
+    result = ERROR_SUCCESS;
   }
 
-  return success;
+  return result == ERROR_SUCCESS;
 }
 
 void SetRegValueWorkItem::Rollback() {
-  if (!ignore_failure_) {
-    if (status_ == SET_VALUE || status_ == VALUE_ROLL_BACK)
-      return;
+  if (ignore_failure_)
+    return;
 
-    if (status_ == VALUE_UNCHANGED) {
-      status_ = VALUE_ROLL_BACK;
-      VLOG(1) << "rollback: setting unchanged, nothing to do";
-      return;
-    }
+  if (status_ == SET_VALUE || status_ == VALUE_ROLL_BACK)
+    return;
 
-    base::win::RegKey key;
-    if (!key.Open(predefined_root_, key_path_.c_str(), KEY_SET_VALUE)) {
-      status_ = VALUE_ROLL_BACK;
-      VLOG(1) << "rollback: can not open " << key_path_;
-      return;
-    }
+  if (status_ == VALUE_UNCHANGED) {
+    status_ = VALUE_ROLL_BACK;
+    VLOG(1) << "rollback: setting unchanged, nothing to do";
+    return;
+  }
 
-    std::wstring result_str(L" failed");
-    if (status_ == NEW_VALUE_CREATED) {
-      if (key.DeleteValue(value_name_.c_str()))
-        result_str.assign(L" succeeded");
-      VLOG(1) << "rollback: deleting " << value_name_ << result_str;
-    } else if (status_ == VALUE_OVERWRITTEN) {
-      // try restore the previous value
-      bool success = true;
-      if (is_str_type_) {
-        success = key.WriteValue(value_name_.c_str(),
-                                 previous_value_str_.c_str());
-      } else {
-        success = key.WriteValue(value_name_.c_str(), previous_value_dword_);
-      }
-      if (success)
-        result_str.assign(L" succeeded");
-      VLOG(1) << "rollback: restoring " << value_name_ << result_str;
-    } else {
-      NOTREACHED();
-    }
-
+  base::win::RegKey key;
+  LONG result = key.Open(predefined_root_, key_path_.c_str(), KEY_SET_VALUE);
+  if (result != ERROR_SUCCESS) {
     status_ = VALUE_ROLL_BACK;
+    VLOG(1) << "rollback: can not open " << key_path_ << " error: " << result;
     return;
   }
+
+  if (status_ == NEW_VALUE_CREATED) {
+    result = key.DeleteValue(value_name_.c_str());
+    VLOG(1) << "rollback: deleting " << value_name_ << " error: " << result;
+  } else if (status_ == VALUE_OVERWRITTEN) {
+    // try restore the previous value
+    if (is_str_type_) {
+      result = key.WriteValue(value_name_.c_str(), previous_value_str_.c_str());
+    } else {
+      result = key.WriteValue(value_name_.c_str(), previous_value_dword_);
+    }
+    VLOG(1) << "rollback: restoring " << value_name_ << " error: " << result;
+  } else {
+    NOTREACHED();
+  }
+
+  status_ = VALUE_ROLL_BACK;
 }