Regkey functions return error code instead of bool

chrome/browser/autofill/autofill_ie_toolbar_import_win.cc

Index: chrome/browser/autofill/autofill_ie_toolbar_import_win.cc
===================================================================
--- chrome/browser/autofill/autofill_ie_toolbar_import_win.cc	(revision 70414)
+++ chrome/browser/autofill/autofill_ie_toolbar_import_win.cc	(working copy)
@@ -57,12 +57,13 @@
 string16 ReadAndDecryptValue(RegKey* key, const wchar_t* value_name) {
   DWORD data_type = REG_BINARY;
   DWORD data_size = 0;
-  if (!key->ReadValue(value_name, NULL, &data_size, &data_type) ||
-      !data_size || data_type != REG_BINARY)
+  LONG result = key->ReadValue(value_name, NULL, &data_size, &data_type);

[brettw, 2011/01/10 20:42:28]

In other places you didn't introduce the extra var and just did the ReadValue
inside the if statement. I think that's a bit more readable since it's clear
that the return value isn't used any other place.

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

My logic was to directly check for ERROR_SUCCESS if it fits on the same line. If
not then it's more readable to declare a temporary. 

For. e.g. in this particular change itself, wrapping ERROR_SUCCESS on the next
line would make it hard to spot the data_size and data_type checks. Whereas as
they stand now, the conditions are obvious in a single glance.

I am not attached to this logic, however, and willing to be convinced otherwise
:)

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

FWIW, I think I spent more time trying to figure out why a local was used in
some places and not in others than I would have spent reading wrapped lines.

+  if ((result != ERROR_SUCCESS) || !data_size || data_type != REG_BINARY)
     return string16();
   std::vector<uint8> data;
   data.resize(data_size);
-  if (key->ReadValue(value_name, &(data[0]), &data_size, &data_type)) {
+  result = key->ReadValue(value_name, &(data[0]), &data_size, &data_type);
+  if (result == ERROR_SUCCESS) {
     std::string out_data;
     if (DecryptData(data, &out_data)) {
       // The actual data is in UTF16 already.
@@ -125,7 +126,7 @@
 
   for (uint32 value_index = 0; value_index < key->ValueCount(); ++value_index) {
     std::wstring value_name;
-    if (!key->ReadName(value_index, &value_name))
+    if (key->ReadName(value_index, &value_name) != ERROR_SUCCESS)
       continue;
     RegToFieldMap::const_iterator it = reg_to_field.find(value_name);
     if (it == reg_to_field.end())