Fixes for re-enabling more MSVC level 4 warnings: misc edition #2

This contains fixes for the following sorts of issues:
* Assignment inside conditional
* Taking the address of a temporary
* Octal escape sequence terminated by decimal number
* Signedness mismatch
* Possibly-uninitialized local variable

This also contains a small number of cleanups to nearby code (e.g. no else after return).

BUG=81439
TEST=none

Review URL: https://codereview.chromium.org/382673002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@283967 0039d316-1c4b-4281-b951-d872f2087c98

ash/drag_drop/drag_drop_controller.cc

@@ -244,11 +244,12 @@ int DragDropController::StartDragAndDrop(
 
 void DragDropController::DragUpdate(aura::Window* target,
                                     const ui::LocatedEvent& event) {
-  aura::client::DragDropDelegate* delegate = NULL;
   int op = ui::DragDropTypes::DRAG_NONE;
   if (target != drag_window_) {
     if (drag_window_) {
-      if ((delegate = aura::client::GetDragDropDelegate(drag_window_)))
+      aura::client::DragDropDelegate* delegate =
+          aura::client::GetDragDropDelegate(drag_window_);
+      if (delegate)
         delegate->OnDragExited();
       if (drag_window_ != drag_source_window_)
         drag_window_->RemoveObserver(this);
@@ -257,7 +258,9 @@ void DragDropController::DragUpdate(aura::Window* target,
     // We are already an observer of |drag_source_window_| so no need to add.
     if (drag_window_ != drag_source_window_)
       drag_window_->AddObserver(this);
-    if ((delegate = aura::client::GetDragDropDelegate(drag_window_))) {
+    aura::client::DragDropDelegate* delegate =
+        aura::client::GetDragDropDelegate(drag_window_);
+    if (delegate) {
       ui::DropTargetEvent e(*drag_data_,
                             event.location(),
                             event.root_location(),
@@ -266,7 +269,9 @@ void DragDropController::DragUpdate(aura::Window* target,
       delegate->OnDragEntered(e);
     }
   } else {
-    if ((delegate = aura::client::GetDragDropDelegate(drag_window_))) {
+    aura::client::DragDropDelegate* delegate =
+        aura::client::GetDragDropDelegate(drag_window_);
+    if (delegate) {
       ui::DropTargetEvent e(*drag_data_,
                             event.location(),
                             event.root_location(),
@@ -298,7 +303,6 @@ void DragDropController::DragUpdate(aura::Window* target,
 void DragDropController::Drop(aura::Window* target,
                               const ui::LocatedEvent& event) {
   ash::Shell::GetInstance()->cursor_manager()->SetCursor(ui::kCursorPointer);
-  aura::client::DragDropDelegate* delegate = NULL;
 
   // We must guarantee that a target gets a OnDragEntered before Drop. WebKit
   // depends on not getting a Drop without DragEnter. This behavior is
@@ -307,7 +311,9 @@ void DragDropController::Drop(aura::Window* target,
     DragUpdate(target, event);
   DCHECK(target == drag_window_);
 
-  if ((delegate = aura::client::GetDragDropDelegate(target))) {
+  aura::client::DragDropDelegate* delegate =
+      aura::client::GetDragDropDelegate(target);
+  if (delegate) {
     ui::DropTargetEvent e(
         *drag_data_, event.location(), event.root_location(), drag_operation_);
     e.set_flags(event.flags());

cloud_print/virtual_driver/win/install/setup.cc

@@ -210,6 +210,7 @@ UINT CALLBACK CabinetCallback(PVOID data,
 }
 
 void ReadyDriverDependencies(const base::FilePath& destination) {
+  base::FilePath destination_copy(destination);
   if (base::win::GetVersion() >= base::win::VERSION_VISTA) {
     // GetCorePrinterDrivers and GetPrinterDriverPackagePath only exist on
     // Vista and later. Winspool.drv must be delayloaded so these calls don't
@@ -221,15 +222,14 @@ void ReadyDriverDependencies(const base::FilePath& destination) {
                           1, &driver);
     GetPrinterDriverPackagePath(NULL, NULL, NULL, driver.szPackageID,
                                 package_path, MAX_PATH, &size);
-    SetupIterateCabinet(package_path, 0, &CabinetCallback,
+    SetupIterateCabinet(package_path, 0, &CabinetCallback, &destination_copy);
-                        &base::FilePath(destination));
   } else {
     // Driver files are in the sp3 cab.
     base::FilePath package_path;
     PathService::Get(base::DIR_WINDOWS, &package_path);
     package_path = package_path.Append(L"Driver Cache\\i386\\sp3.cab");
     SetupIterateCabinet(package_path.value().c_str(), 0, &CabinetCallback,
-                        &base::FilePath(destination));
+                        &destination_copy);
 
     // Copy the rest from the driver cache or system dir.
     base::FilePath driver_cache_path;

crypto/symmetric_key_unittest.cc

@@ -172,7 +172,7 @@ static const PBKDF2TestVector kTestVectors[] = {
   {
     crypto::SymmetricKey::HMAC_SHA1,
     "password",
-    "\0224VxxV4\022", /* 0x1234567878563412 */
+    "\022" "4VxxV4\022", /* 0x1234567878563412 */
     5,
     160,
     "d1daa78615f287e6a1c8b120d7062a493f98d203",

device/bluetooth/bluetooth_socket_win.cc

@@ -91,7 +91,7 @@ BluetoothSocketWin::BluetoothSocketWin(
     const net::NetLog::Source& source)
     : BluetoothSocketNet(ui_task_runner, socket_thread, net_log, source),
       supports_rfcomm_(false),
-      rfcomm_channel_(-1),
+      rfcomm_channel_(0xFF),
       bth_addr_(BTH_ADDR_NULL) {
 }
 

extensions/browser/extension_prefs.cc

@@ -1005,7 +1005,7 @@ void ExtensionPrefs::MigratePermissions(const ExtensionIdList& extension_ids) {
     // An extension's granted permissions need to be migrated if the
     // full_access bit is present. This bit was always present in the previous
     // scheme and is never present now.
-    bool full_access;
+    bool full_access = false;
     const base::DictionaryValue* ext = GetExtensionPref(*ext_id);
     if (!ext || !ext->GetBoolean(kPrefOldGrantedFullAccess, &full_access))
       continue;
@@ -1288,7 +1288,7 @@ BlacklistState ExtensionPrefs::GetExtensionBlacklistState(
   if (IsExtensionBlacklisted(extension_id))
     return BLACKLISTED_MALWARE;
   const base::DictionaryValue* ext_prefs = GetExtensionPref(extension_id);
-  int int_value;
+  int int_value = 0;
   if (ext_prefs && ext_prefs->GetInteger(kPrefBlacklistState, &int_value))
     return static_cast<BlacklistState>(int_value);
 

extensions/common/id_util.cc

@@ -60,8 +60,8 @@ base::FilePath MaybeNormalizePath(const base::FilePath& path) {
   // comparisons simpler.
   base::FilePath::StringType path_str = path.value();
   if (path_str.size() >= 2 && path_str[0] >= L'a' && path_str[0] <= L'z' &&
-      path_str[1] == ':')
+      path_str[1] == L':')
-    path_str[0] += ('A' - 'a');
+    path_str[0] = towupper(path_str[0]);
 
   return base::FilePath(path_str);
 #else

google_apis/gaia/fake_gaia.cc

@@ -413,14 +413,10 @@ void FakeGaia::HandleSSO(const HttpRequest& request,
 
 void FakeGaia::HandleAuthToken(const HttpRequest& request,
                                BasicHttpResponse* http_response) {
-  std::string grant_type;
-  std::string refresh_token;
-  std::string client_id;
   std::string scope;
-  std::string auth_code;
-  const AccessTokenInfo* token_info = NULL;
   GetQueryParameter(request.content, "scope", &scope);
 
+  std::string grant_type;
   if (!GetQueryParameter(request.content, "grant_type", &grant_type)) {
     http_response->set_code(net::HTTP_BAD_REQUEST);
     LOG(ERROR) << "No 'grant_type' param in /o/oauth2/token";
@@ -428,6 +424,7 @@ void FakeGaia::HandleAuthToken(const HttpRequest& request,
   }
 
   if (grant_type == "authorization_code") {
+    std::string auth_code;
     if (!GetQueryParameter(request.content, "code", &auth_code) ||
         auth_code != merge_session_params_.auth_code) {
       http_response->set_code(net::HTTP_BAD_REQUEST);
@@ -448,26 +445,29 @@ void FakeGaia::HandleAuthToken(const HttpRequest& request,
                             merge_session_params_.access_token);
     response_dict.SetInteger("expires_in", 3600);
     FormatJSONResponse(response_dict, http_response);
-  } else if (GetQueryParameter(request.content,
+    return;
-                               "refresh_token",
-                               &refresh_token) &&
-             GetQueryParameter(request.content,
-                               "client_id",
-                               &client_id) &&
-             (token_info = FindAccessTokenInfo(refresh_token,
-                                              client_id,
-                                              scope))) {
-    base::DictionaryValue response_dict;
-    response_dict.SetString("access_token", token_info->token);
-    response_dict.SetInteger("expires_in", 3600);
-    FormatJSONResponse(response_dict, http_response);
-  } else {
-    LOG(ERROR) << "Bad request for /o/oauth2/token - "
-               << "refresh_token = " << refresh_token
-               << ", scope = " << scope
-               << ", client_id = " << client_id;
-    http_response->set_code(net::HTTP_BAD_REQUEST);
   }
+
+  std::string refresh_token;
+  std::string client_id;
+  if (GetQueryParameter(request.content, "refresh_token", &refresh_token) &&
+      GetQueryParameter(request.content, "client_id", &client_id)) {
+    const AccessTokenInfo* token_info =
+        FindAccessTokenInfo(refresh_token, client_id, scope);
+    if (token_info) {
+      base::DictionaryValue response_dict;
+      response_dict.SetString("access_token", token_info->token);
+      response_dict.SetInteger("expires_in", 3600);
+      FormatJSONResponse(response_dict, http_response);
+      return;
+    }
+  }
+
+  LOG(ERROR) << "Bad request for /o/oauth2/token - "
+              << "refresh_token = " << refresh_token
+              << ", scope = " << scope
+              << ", client_id = " << client_id;
+  http_response->set_code(net::HTTP_BAD_REQUEST);
 }
 
 void FakeGaia::HandleTokenInfo(const HttpRequest& request,
@@ -507,20 +507,22 @@ void FakeGaia::HandleIssueToken(const HttpRequest& request,
   std::string access_token;
   std::string scope;
   std::string client_id;
-  const AccessTokenInfo* token_info = NULL;
   if (GetAccessToken(request, kAuthHeaderBearer, &access_token) &&
       GetQueryParameter(request.content, "scope", &scope) &&
-      GetQueryParameter(request.content, "client_id", &client_id) &&
+      GetQueryParameter(request.content, "client_id", &client_id)) {
-      (token_info = FindAccessTokenInfo(access_token, client_id, scope))) {
+    const AccessTokenInfo* token_info =
-    base::DictionaryValue response_dict;
+        FindAccessTokenInfo(access_token, client_id, scope);
-    response_dict.SetString("issueAdvice", "auto");
+    if (token_info) {
-    response_dict.SetString("expiresIn",
+      base::DictionaryValue response_dict;
-                            base::IntToString(token_info->expires_in));
+      response_dict.SetString("issueAdvice", "auto");
-    response_dict.SetString("token", token_info->token);
+      response_dict.SetString("expiresIn",
-    FormatJSONResponse(response_dict, http_response);
+                              base::IntToString(token_info->expires_in));
-  } else {
+      response_dict.SetString("token", token_info->token);
-    http_response->set_code(net::HTTP_BAD_REQUEST);
+      FormatJSONResponse(response_dict, http_response);
+      return;
+    }
   }
+  http_response->set_code(net::HTTP_BAD_REQUEST);
 }
 
 void FakeGaia::HandleListAccounts(const HttpRequest& request,

ipc/ipc_message_utils.cc

@@ -567,21 +567,17 @@ void ParamTraits<base::File::Info>::Write(Message* m,
 bool ParamTraits<base::File::Info>::Read(const Message* m,
                                          PickleIterator* iter,
                                          param_type* p) {
-  double last_modified;
+  double last_modified, last_accessed, creation_time;
-  double last_accessed;
+  if (!ReadParam(m, iter, &p->size) ||
-  double creation_time;
+      !ReadParam(m, iter, &p->is_directory) ||
-  bool result =
+      !ReadParam(m, iter, &last_modified) ||
-      ReadParam(m, iter, &p->size) &&
+      !ReadParam(m, iter, &last_accessed) ||
-      ReadParam(m, iter, &p->is_directory) &&
+      !ReadParam(m, iter, &creation_time))
-      ReadParam(m, iter, &last_modified) &&
+    return false;
-      ReadParam(m, iter, &last_accessed) &&
+  p->last_modified = base::Time::FromDoubleT(last_modified);
-      ReadParam(m, iter, &creation_time);
+  p->last_accessed = base::Time::FromDoubleT(last_accessed);
-  if (result) {
+  p->creation_time = base::Time::FromDoubleT(creation_time);
-    p->last_modified = base::Time::FromDoubleT(last_modified);
+  return true;
-    p->last_accessed = base::Time::FromDoubleT(last_accessed);
-    p->creation_time = base::Time::FromDoubleT(creation_time);
-  }
-  return result;
 }
 
 void ParamTraits<base::File::Info>::Log(const param_type& p,

jingle/glue/thread_wrapper_unittest.cc

@@ -298,7 +298,7 @@ TEST_F(ThreadWrapperTest, SendDuringSend) {
 }
 
 TEST_F(ThreadWrapperTest, Dispose) {
-  bool deleted_;
+  bool deleted_ = false;
   thread_->Dispose(new DeletableObject(&deleted_));
   EXPECT_FALSE(deleted_);
   message_loop_.RunUntilIdle();

jingle/glue/utils.cc

@@ -62,13 +62,13 @@ bool DeserializeP2PCandidate(const std::string& candidate_str,
       static_cast<base::DictionaryValue*>(value.get());
 
   std::string ip;
-  int port;
+  int port = 0;
   std::string type;
   std::string protocol;
   std::string username;
   std::string password;
-  double preference;
+  double preference = 0;
-  int generation;
+  int generation = 0;
 
   if (!dic_value->GetString("ip", &ip) ||
       !dic_value->GetInteger("port", &port) ||

skia/ext/pixel_ref_utils_unittest.cc

@@ -31,15 +31,7 @@ class TestDiscardableShader : public SkShader {
     CreateBitmap(gfx::Size(50, 50), "discardable", &bitmap_);
   }
 
-  TestDiscardableShader(SkFlattenableReadBuffer& flattenable_buffer) {
+  TestDiscardableShader(SkReadBuffer& buffer) : SkShader(buffer) {
-    SkOrderedReadBuffer& buffer =
-        static_cast<SkOrderedReadBuffer&>(flattenable_buffer);
-    SkReader32* reader = buffer.getReader32();
-
-    reader->skip(-4);
-    uint32_t toSkip = reader->readU32();
-    reader->skip(toSkip);
-
     CreateBitmap(gfx::Size(50, 50), "discardable", &bitmap_);
   }
 

skia/ext/platform_canvas_unittest.cc

@@ -20,12 +20,20 @@
 #include "testing/gtest/include/gtest/gtest.h"
 #include "third_party/skia/include/core/SkBitmap.h"
 #include "third_party/skia/include/core/SkColor.h"
+#include "third_party/skia/include/core/SkColorPriv.h"
 #include "third_party/skia/include/core/SkPixelRef.h"
 
 namespace skia {
 
 namespace {
 
+bool IsOfColor(const SkBitmap& bitmap, int x, int y, uint32_t color) {
+  // For masking out the alpha values.
+  static uint32_t alpha_mask =
+      static_cast<uint32_t>(SK_A32_MASK) << SK_A32_SHIFT;
+  return (*bitmap.getAddr32(x, y) | alpha_mask) == (color | alpha_mask);
+}
+
 // Return true if the canvas is filled to canvas_color, and contains a single
 // rectangle filled to rect_color. This function ignores the alpha channel,
 // since Windows will sometimes clear the alpha channel when drawing, and we
@@ -37,21 +45,16 @@ bool VerifyRect(const PlatformCanvas& canvas,
   const SkBitmap& bitmap = device->accessBitmap(false);
   SkAutoLockPixels lock(bitmap);
 
-  // For masking out the alpha values.
-  uint32_t alpha_mask = 0xFF << SK_A32_SHIFT;
-
   for (int cur_y = 0; cur_y < bitmap.height(); cur_y++) {
     for (int cur_x = 0; cur_x < bitmap.width(); cur_x++) {
       if (cur_x >= x && cur_x < x + w &&
           cur_y >= y && cur_y < y + h) {
         // Inside the square should be rect_color
-        if ((*bitmap.getAddr32(cur_x, cur_y) | alpha_mask) !=
+        if (!IsOfColor(bitmap, cur_x, cur_y, rect_color))
-            (rect_color | alpha_mask))
           return false;
       } else {
         // Outside the square should be canvas_color
-        if ((*bitmap.getAddr32(cur_x, cur_y) | alpha_mask) !=
+        if (!IsOfColor(bitmap, cur_x, cur_y, canvas_color))
-            (canvas_color | alpha_mask))
           return false;
       }
     }
@@ -60,12 +63,6 @@ bool VerifyRect(const PlatformCanvas& canvas,
 }
 
 #if !defined(OS_MACOSX)
-bool IsOfColor(const SkBitmap& bitmap, int x, int y, uint32_t color) {
-  // For masking out the alpha values.
-  static uint32_t alpha_mask = 0xFF << SK_A32_SHIFT;
-  return (*bitmap.getAddr32(x, y) | alpha_mask) == (color | alpha_mask);
-}
-
 // Return true if canvas has something that passes for a rounded-corner
 // rectangle. Basically, we're just checking to make sure that the pixels in the
 // middle are of rect_color and pixels in the corners are of canvas_color.

sql/recovery.cc

@@ -355,7 +355,7 @@ bool Recovery::AutoRecoverTable(const char* table_name,
   // |rowid_decl| stores the ROWID version of the last INTEGER column
   // seen, which is at |rowid_ofs| in |create_column_decls|.
   size_t pk_column_count = 0;
-  size_t rowid_ofs;  // Only valid if rowid_decl is set.
+  size_t rowid_ofs = 0;  // Only valid if rowid_decl is set.
   std::string rowid_decl;  // ROWID version of column |rowid_ofs|.
 
   while (s.Step()) {
@@ -372,7 +372,7 @@ bool Recovery::AutoRecoverTable(const char* table_name,
       // (zero for not in primary key).  I find that it is always 1 for
       // columns in the primary key.  Since this code is very dependent on
       // that pragma, review if the implementation changes.
-      DCHECK_EQ(pk_column, 1);
+      DCHECK_EQ(1, pk_column);
       ++pk_column_count;
     }
 

sync/internal_api/public/base/invalidation.cc

@@ -73,26 +73,25 @@ scoped_ptr<Invalidation> Invalidation::InitFromValue(
         kInvalidVersion,
         std::string(),
         AckHandle::CreateUnique()));
-  } else {
-    int64 version;
-    std::string version_as_string;
-    if (!value.GetString(kVersionKey, &version_as_string)
-        || !base::StringToInt64(version_as_string, &version)) {
-      DLOG(WARNING) << "Failed to parse version";
-      return scoped_ptr<Invalidation>();
-    }
-    std::string payload;
-    if (!value.GetString(kPayloadKey, &payload)) {
-      DLOG(WARNING) << "Failed to parse payload";
-      return scoped_ptr<Invalidation>();
-    }
-    return scoped_ptr<Invalidation>(new Invalidation(
-        id,
-        false,
-        version,
-        payload,
-        AckHandle::CreateUnique()));
   }
+  int64 version = 0;
+  std::string version_as_string;
+  if (!value.GetString(kVersionKey, &version_as_string)
+      || !base::StringToInt64(version_as_string, &version)) {
+    DLOG(WARNING) << "Failed to parse version";
+    return scoped_ptr<Invalidation>();
+  }
+  std::string payload;
+  if (!value.GetString(kPayloadKey, &payload)) {
+    DLOG(WARNING) << "Failed to parse payload";
+    return scoped_ptr<Invalidation>();
+  }
+  return scoped_ptr<Invalidation>(new Invalidation(
+      id,
+      false,
+      version,
+      payload,
+      AckHandle::CreateUnique()));
 }
 
 Invalidation::~Invalidation() {}

sync/internal_api/sync_manager_impl_unittest.cc

@@ -2862,7 +2862,7 @@ class SyncManagerChangeProcessingTest : public SyncManagerTest {
       }
     }
     ADD_FAILURE() << "Failed to find specified change";
-    return -1;
+    return static_cast<size_t>(-1);
   }
 
   // Returns the current size of the change list.

tools/gn/tokenizer.cc

@@ -134,14 +134,13 @@ std::vector<Token> Tokenizer::Run() {
 
 // static
 size_t Tokenizer::ByteOffsetOfNthLine(const base::StringPiece& buf, int n) {
-  int cur_line = 1;
+  DCHECK_GT(n, 0);
-  size_t cur_byte = 0;
-
-  DCHECK(n > 0);
 
   if (n == 1)
     return 0;
 
+  int cur_line = 1;
+  size_t cur_byte = 0;
   while (cur_byte < buf.size()) {
     if (IsNewline(buf, cur_byte)) {
       cur_line++;
@@ -150,7 +149,7 @@ size_t Tokenizer::ByteOffsetOfNthLine(const base::StringPiece& buf, int n) {
     }
     cur_byte++;
   }
-  return -1;
+  return static_cast<size_t>(-1);
 }
 
 // static

tools/json_to_struct/json_to_struct.py

@@ -99,14 +99,14 @@ def _GenerateH(basepath, fileroot, head, namespace, schema, description):
   with open(os.path.join(basepath, h_filename), 'w') as f:
     f.write(head)
 
-    f.write('#include <cstddef>\n')
-    f.write('\n')
-
     header_guard = _GenerateHeaderGuard(h_filename)
     f.write('#ifndef %s\n' % header_guard)
     f.write('#define %s\n' % header_guard)
     f.write('\n')
 
+    f.write('#include <cstddef>\n')
+    f.write('\n')
+
     for header in schema.get('headers', []):
       f.write('#include "%s"\n' % header)
     f.write('\n')

win8/metro_driver/ime/text_store.cc

@@ -765,7 +765,7 @@ bool TextStore::GetCompositionStatus(
   while (true) {
     base::win::ScopedComPtr<ITfRange> range;
     if (ranges->Next(1, range.Receive(), NULL) != S_OK)
-      break;
+      return true;
     base::win::ScopedVariant value;
     base::win::ScopedComPtr<IEnumTfPropertyValue> enum_prop_value;
     if (FAILED(track_property->GetValue(read_only_edit_cookie, range,
@@ -777,16 +777,16 @@ bool TextStore::GetCompositionStatus(
 
     TF_PROPERTYVAL property_value;
     bool is_composition = false;
-    bool has_display_attribute = false;
+    metro_viewer::UnderlineInfo underline;
-    TF_DISPLAYATTRIBUTE display_attribute;
     while (enum_prop_value->Next(1, &property_value, NULL) == S_OK) {
       if (IsEqualGUID(property_value.guidId, GUID_PROP_COMPOSING)) {
         is_composition = (property_value.varValue.lVal == TRUE);
       } else if (IsEqualGUID(property_value.guidId, GUID_PROP_ATTRIBUTE)) {
         TfGuidAtom guid_atom =
             static_cast<TfGuidAtom>(property_value.varValue.lVal);
+        TF_DISPLAYATTRIBUTE display_attribute;
         if (GetDisplayAttribute(guid_atom, &display_attribute))
-          has_display_attribute = true;
+          underline.thick = !!display_attribute.fBoldLine;
       }
       VariantClear(&property_value.varValue);
     }
@@ -795,18 +795,14 @@ bool TextStore::GetCompositionStatus(
     range_acp.QueryFrom(range);
     LONG start_pos, length;
     range_acp->GetExtent(&start_pos, &length);
-    if (!is_composition) {
+    if (is_composition) {
-      if (*committed_size < static_cast<size_t>(start_pos + length))
-        *committed_size = start_pos + length;
-    } else {
-      metro_viewer::UnderlineInfo underline;
       underline.start_offset = start_pos;
       underline.end_offset = start_pos + length;
-      underline.thick = !!display_attribute.fBoldLine;
       undelines->push_back(underline);
+    } else if (*committed_size < static_cast<size_t>(start_pos + length)) {
+      *committed_size = start_pos + length;
     }
   }
-  return true;
 }
 
 bool TextStore::CancelComposition() {