ake string_util::WriteInto() DCHECK() that the supplied |length_with_null| > 1, meaning that the without-'\0' string is non-empty. This replaces the conditional code added recently that makes this case return NULL. It's easier to understand if it's simply an error to call WriteInto() in this case at all.

Add DCHECK()s or conditionals as appropriate to callers in order to ensure this assertion holds.

BUG=none
TEST=none
Review URL: http://codereview.chromium.org/8418034

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

37 files changed, 214 insertions, 271 deletions

diff --git a/base/base_paths_mac.mm b/base/base_paths_mac.mm
index a9cd471b..aed3ea8 100644
--- a/base/base_paths_mac.mm
+++ b/base/base_paths_mac.mm
@@ -18,24 +18,18 @@
 
 namespace {
 
-bool GetNSExecutablePath(FilePath* path) WARN_UNUSED_RESULT;
-
-bool GetNSExecutablePath(FilePath* path) {
+void GetNSExecutablePath(FilePath* path) {
   DCHECK(path);
   // Executable path can have relative references ("..") depending on
   // how the app was launched.
   uint32_t executable_length = 0;
   _NSGetExecutablePath(NULL, &executable_length);
-  DCHECK_GE(executable_length, 1u);
+  DCHECK_GT(executable_length, 1u);
   std::string executable_path;
-  char* executable_path_c = WriteInto(&executable_path, executable_length);
-  int rv = _NSGetExecutablePath(executable_path_c, &executable_length);
+  int rv = _NSGetExecutablePath(WriteInto(&executable_path, executable_length),
+                                &executable_length);
   DCHECK_EQ(rv, 0);
-  DCHECK(!executable_path.empty());
-  if ((rv != 0) || (executable_path.empty()))
-    return false;
   *path = FilePath(executable_path);
-  return true;
 }
 
 // Returns true if the module for |address| is found. |path| will contain
@@ -58,7 +52,8 @@ namespace base {
 bool PathProviderMac(int key, FilePath* result) {
   switch (key) {
     case base::FILE_EXE:
-      return GetNSExecutablePath(result);
+      GetNSExecutablePath(result);
+      return true;
     case base::FILE_MODULE:
       return GetModulePathForAddress(result,
           reinterpret_cast<const void*>(&base::PathProviderMac));
diff --git a/base/rand_util.cc b/base/rand_util.cc
index fcbccef..a9bc961 100644
--- a/base/rand_util.cc
+++ b/base/rand_util.cc
@@ -71,6 +71,7 @@ void RandBytes(void* output, size_t output_length) {
 }
 
 std::string RandBytesAsString(size_t length) {
+  DCHECK_GT(length, 0u);
   std::string result;
   RandBytes(WriteInto(&result, length + 1), length);
   return result;
diff --git a/base/rand_util.h b/base/rand_util.h
index 4474c02..8d795a3 100644
--- a/base/rand_util.h
+++ b/base/rand_util.h
@@ -38,9 +38,9 @@ BASE_EXPORT double BitsToOpenEndedUnitInterval(uint64 bits);
 BASE_EXPORT void RandBytes(void* output, size_t output_length);
 
 // Fills a string of length |length| with with cryptographically strong random
-// data and returns it.
+// data and returns it.  |length| should be nonzero.
 //
-// Not that this is a variation of |RandBytes| with a different return type.
+// Note that this is a variation of |RandBytes| with a different return type.
 BASE_EXPORT std::string RandBytesAsString(size_t length);
 
 }  // namespace base
diff --git a/base/rand_util_unittest.cc b/base/rand_util_unittest.cc
index a3474ba..e0e85ec 100644
--- a/base/rand_util_unittest.cc
+++ b/base/rand_util_unittest.cc
@@ -41,8 +41,8 @@ TEST(RandUtilTest, RandBytes) {
 }
 
 TEST(RandUtilTest, RandBytesAsString) {
-  std::string random_string = base::RandBytesAsString(0);
-  EXPECT_EQ(0U, random_string.size());
+  std::string random_string = base::RandBytesAsString(1);
+  EXPECT_EQ(1U, random_string.size());
   random_string = base::RandBytesAsString(145);
   EXPECT_EQ(145U, random_string.size());
   char accumulator = 0;
diff --git a/base/string_util.h b/base/string_util.h
index c359e73..a135180 100644
--- a/base/string_util.h
+++ b/base/string_util.h
@@ -457,40 +457,32 @@ BASE_EXPORT void ReplaceSubstringsAfterOffset(
     const std::string& find_this,
     const std::string& replace_with);
 
-// Reserves enough memory in |str| to accommodate |length_with_null|
-// characters, sets the size of |str| to |length_with_null - 1| characters,
-// and returns a pointer to the underlying contiguous array of characters.
+// Reserves enough memory in |str| to accommodate |length_with_null| characters,
+// sets the size of |str| to |length_with_null - 1| characters, and returns a
+// pointer to the underlying contiguous array of characters.  This is typically
+// used when calling a function that writes results into a character array, but
+// the caller wants the data to be managed by a string-like object.  It is
+// convenient in that is can be used inline in the call, and fast in that it
+// avoids copying the results of the call from a char* into a string.
 //
-// This is typically used when calling a function that writes results into a
-// character array, but the caller wants the data to be managed by a
-// string-like object.
+// |length_with_null| must be at least 2, since otherwise the underlying string
+// would have size 0, and trying to access &((*str)[0]) in that case can result
+// in a number of problems.
 //
-// |length_with_null| must be >= 1. In the |length_with_null| == 1 case,
-// NULL is returned rather than a pointer to the array, since there is no way
-// to provide access to the underlying character array of a 0-length
-// string-like object without breaking guarantees provided by the C++
-// standards.
-//
-// Internally, this takes linear time because the underlying array needs to
-// be 0-filled for all |length_with_null - 1| * sizeof(character) bytes.
+// Internally, this takes linear time because the resize() call 0-fills the
+// underlying array for potentially all
+// (|length_with_null - 1| * sizeof(string_type::value_type)) bytes.  Ideally we
+// could avoid this aspect of the resize() call, as we expect the caller to
+// immediately write over this memory, but there is no other way to set the size
+// of the string, and not doing that will mean people who access |str| rather
+// than str.c_str() will get back a string of whatever size |str| had on entry
+// to this function (probably 0).
 template <class string_type>
 inline typename string_type::value_type* WriteInto(string_type* str,
                                                    size_t length_with_null) {
-  DCHECK_NE(0u, length_with_null);
+  DCHECK_GT(length_with_null, 1u);
   str->reserve(length_with_null);
   str->resize(length_with_null - 1);
-
-  // If |length_with_null| is 1, calling (*str)[0] is invalid since the
-  // size() is 0. In some implementations this triggers an assertion.
-  //
-  // Trying to access the underlying byte array by casting away const
-  // when calling str->data() or str->c_str() is also incorrect.
-  // Some implementations of basic_string use a copy-on-write approach and
-  // this could end up mutating the data that is shared across multiple string
-  // objects.
-  if (length_with_null <= 1)
-    return NULL;
-
   return &((*str)[0]);
 }
 
diff --git a/base/string_util_unittest.cc b/base/string_util_unittest.cc
index 1ffc59b..a028882 100644
--- a/base/string_util_unittest.cc
+++ b/base/string_util_unittest.cc
@@ -1113,34 +1113,27 @@ TEST(StringUtilTest, ContainsOnlyChars) {
   EXPECT_FALSE(ContainsOnlyChars("123a", "4321"));
 }
 
-TEST(StringUtilTest, WriteInto) {
+class WriteIntoTest : public testing::Test {
+ protected:
+  static void WritesCorrectly(size_t num_chars) {
+    std::string buffer;
+    char kOriginal[] = "supercali";
+    strncpy(WriteInto(&buffer, num_chars + 1), kOriginal, num_chars);
+    // Using std::string(buffer.c_str()) instead of |buffer| truncates the
+    // string at the first \0.
+    EXPECT_EQ(std::string(kOriginal,
+                          std::min(num_chars, arraysize(kOriginal) - 1)),
+              std::string(buffer.c_str()));
+    EXPECT_EQ(num_chars, buffer.size());
+  }
+};
+
+TEST_F(WriteIntoTest, WriteInto) {
   // Validate that WriteInto reserves enough space and
   // sizes a string correctly.
-  std::string buffer;
-  const char kOriginal[] = "supercali";
-  strncpy(WriteInto(&buffer, 1), kOriginal, 0);
-  EXPECT_STREQ("", buffer.c_str());
-  EXPECT_EQ(0u, buffer.size());
-  strncpy(WriteInto(&buffer, 2), kOriginal, 1);
-  EXPECT_STREQ("s", buffer.c_str());
-  EXPECT_EQ(1u, buffer.size());
-  strncpy(WriteInto(&buffer, 3), kOriginal, 2);
-  EXPECT_STREQ("su", buffer.c_str());
-  EXPECT_EQ(2u, buffer.size());
-  strncpy(WriteInto(&buffer, 5001), kOriginal, 5000);
-  EXPECT_STREQ("supercali", buffer.c_str());
-  EXPECT_EQ(5000u, buffer.size());
-  strncpy(WriteInto(&buffer, 3), kOriginal, 2);
-  EXPECT_STREQ("su", buffer.c_str());
-  EXPECT_EQ(2u, buffer.size());
-  strncpy(WriteInto(&buffer, 1), kOriginal, 0);
-  EXPECT_STREQ("", buffer.c_str());
-  EXPECT_EQ(0u, buffer.size());
-
-  // Validate that WriteInto returns NULL only when
-  // |length_with_null| == 1.
-  EXPECT_TRUE(WriteInto(&buffer, 1) == NULL);
-  EXPECT_TRUE(WriteInto(&buffer, 2) != NULL);
+  WritesCorrectly(1);
+  WritesCorrectly(2);
+  WritesCorrectly(5000);
 
   // Validate that WriteInto doesn't modify other strings
   // when using a Copy-on-Write implementation.
@@ -1149,9 +1142,9 @@ TEST(StringUtilTest, WriteInto) {
   const std::string live = kLive;
   std::string dead = live;
   strncpy(WriteInto(&dead, 5), kDead, 4);
-  EXPECT_STREQ(kDead, dead.c_str());
+  EXPECT_EQ(kDead, dead);
   EXPECT_EQ(4u, dead.size());
-  EXPECT_STREQ(kLive, live.c_str());
+  EXPECT_EQ(kLive, live);
   EXPECT_EQ(4u, live.size());
 }
 
diff --git a/chrome/browser/automation/testing_automation_provider_win.cc b/chrome/browser/automation/testing_automation_provider_win.cc
index 424aa39..21d2137 100644
--- a/chrome/browser/automation/testing_automation_provider_win.cc
+++ b/chrome/browser/automation/testing_automation_provider_win.cc
@@ -86,9 +86,10 @@ void TestingAutomationProvider::SetWindowVisible(int handle,
 
 void TestingAutomationProvider::GetWindowTitle(int handle, string16* text) {
   gfx::NativeWindow window = window_tracker_->GetResource(handle);
-  std::wstring result;
   int length = ::GetWindowTextLength(window) + 1;
-  ::GetWindowText(window, WriteInto(&result, length), length);
-  text->assign(WideToUTF16(result));
+  if (length > 1)
+    ::GetWindowText(window, WriteInto(text, length), length);
+  else
+    text->clear();
 }
 
diff --git a/chrome/browser/browser_focus_uitest.cc b/chrome/browser/browser_focus_uitest.cc
index 0f4fdcc..dada7af 100644
--- a/chrome/browser/browser_focus_uitest.cc
+++ b/chrome/browser/browser_focus_uitest.cc
@@ -90,7 +90,8 @@ bool ChromeInForeground() {
   std::wstring caption;
   std::wstring filename;
   int len = ::GetWindowTextLength(window) + 1;
-  ::GetWindowText(window, WriteInto(&caption, len), len);
+  if (len > 1)
+    ::GetWindowText(window, WriteInto(&caption, len), len);
   bool chrome_window_in_foreground =
       EndsWith(caption, L" - Google Chrome", true) ||
       EndsWith(caption, L" - Chromium", true);
@@ -102,8 +103,8 @@ bool ChromeInForeground() {
     if (base::OpenProcessHandleWithAccess(process_id,
                                           PROCESS_QUERY_LIMITED_INFORMATION,
                                           &process)) {
-      len = MAX_PATH;
-      if (!GetProcessImageFileName(process, WriteInto(&filename, len), len)) {
+      if (!GetProcessImageFileName(process, WriteInto(&filename, MAX_PATH),
+                                   MAX_PATH)) {
         int error = GetLastError();
         filename = std::wstring(L"Unable to read filename for process id '" +
                                 base::IntToString16(process_id) +
diff --git a/chrome/browser/mac/install_from_dmg.mm b/chrome/browser/mac/install_from_dmg.mm
index b263491..6f276b8 100644
--- a/chrome/browser/mac/install_from_dmg.mm
+++ b/chrome/browser/mac/install_from_dmg.mm
@@ -141,6 +141,10 @@ bool MediaResidesOnDiskImage(io_service_t media, std::string* image_path) {
     CFDataRef image_path_data = static_cast<CFDataRef>(
         image_path_cftyperef.get());
     CFIndex length = CFDataGetLength(image_path_data);
+    if (length <= 0) {
+      LOG(ERROR) << "image_path_data is unexpectedly empty";
+      return true;
+    }
     char* image_path_c = WriteInto(image_path, length + 1);
     CFDataGetBytes(image_path_data,
                    CFRangeMake(0, length),
diff --git a/chrome/browser/ui/views/omnibox/omnibox_view_win.cc b/chrome/browser/ui/views/omnibox/omnibox_view_win.cc
index fd0ca98..379d94d 100644
--- a/chrome/browser/ui/views/omnibox/omnibox_view_win.cc
+++ b/chrome/browser/ui/views/omnibox/omnibox_view_win.cc
@@ -645,11 +645,9 @@ void OmniboxViewWin::OpenMatch(const AutocompleteMatch& match,
 
 string16 OmniboxViewWin::GetText() const {
   const int len = GetTextLength() + 1;
-  if (len <= 1)
-    return string16();
-
   string16 str;
-  GetWindowText(WriteInto(&str, len), len);
+  if (len > 1)
+    GetWindowText(WriteInto(&str, len), len);
   return str;
 }
 
@@ -2128,15 +2126,11 @@ void OmniboxViewWin::GetSelection(CHARRANGE& sel) const {
 }
 
 string16 OmniboxViewWin::GetSelectedText() const {
-  // Figure out the length of the selection.
   CHARRANGE sel;
   GetSel(sel);
-  if (sel.cpMin == sel.cpMax)  // GetSelText() crashes on NULL input.
-    return string16();
-
-  // Grab the selected text.
   string16 str;
-  GetSelText(WriteInto(&str, sel.cpMax - sel.cpMin + 1));
+  if (sel.cpMin != sel.cpMax)
+    GetSelText(WriteInto(&str, sel.cpMax - sel.cpMin + 1));
   return str;
 }
 
diff --git a/chrome/common/metrics_log_manager.cc b/chrome/common/metrics_log_manager.cc
index 37a9e4b..960672a 100644
--- a/chrome/common/metrics_log_manager.cc
+++ b/chrome/common/metrics_log_manager.cc
@@ -105,7 +105,7 @@ void MetricsLogManager::LoadPersistedUnsentLogs() {
 void MetricsLogManager::CompressStagedLog() {
   int text_size = staged_log_->GetEncodedLogSize();
   std::string staged_log_text;
-  // Leave room for the NULL terminator.
+  DCHECK_GT(text_size, 0);
   staged_log_->GetEncodedLog(WriteInto(&staged_log_text, text_size + 1),
                              text_size);
 
diff --git a/chrome/common/time_format.cc b/chrome/common/time_format.cc
index 75407a9..c4cf9ca 100644
--- a/chrome/common/time_format.cc
+++ b/chrome/common/time_format.cc
@@ -296,9 +296,9 @@ static string16 FormatTimeImpl(const TimeDelta& delta, FormatType format_type) {
   // With the fallback added, this should never fail.
   DCHECK(U_SUCCESS(error));
   int capacity = time_string.length() + 1;
+  DCHECK_GT(capacity, 1);
   string16 result;
-  time_string.extract(static_cast<UChar*>(
-                      WriteInto(&result, capacity)),
+  time_string.extract(static_cast<UChar*>(WriteInto(&result, capacity)),
                       capacity, error);
   DCHECK(U_SUCCESS(error));
   return result;
diff --git a/chrome_frame/chrome_tab.cc b/chrome_frame/chrome_tab.cc
index b394a11..b984e73 100644
--- a/chrome_frame/chrome_tab.cc
+++ b/chrome_frame/chrome_tab.cc
@@ -467,8 +467,9 @@ class SecurityDescBackup {
     DWORD reg_type = REG_NONE;
     if (backup_key.ReadValue(NULL, NULL, &len, &reg_type) != ERROR_SUCCESS)
       return false;
+    DCHECK_EQ(0u, len % sizeof(wchar_t));
 
-    if (reg_type != REG_SZ)
+    if ((len == 0) || (reg_type != REG_SZ))
       return false;
 
     size_t wchar_count = 1 + len / sizeof(wchar_t);
diff --git a/chrome_frame/test/win_event_receiver.cc b/chrome_frame/test/win_event_receiver.cc
index 045a48c..fbc21f2 100644
--- a/chrome_frame/test/win_event_receiver.cc
+++ b/chrome_frame/test/win_event_receiver.cc
@@ -169,8 +169,8 @@ void WindowWatchdog::RemoveObserver(WindowObserver* observer) {
 std::string WindowWatchdog::GetWindowCaption(HWND hwnd) {
   std::string caption;
   int len = ::GetWindowTextLength(hwnd) + 1;
-  ::GetWindowTextA(hwnd, WriteInto(&caption, len), len);
-
+  if (len > 1)
+    ::GetWindowTextA(hwnd, WriteInto(&caption, len), len);
   return caption;
 }
 
diff --git a/chrome_frame/utils.cc b/chrome_frame/utils.cc
index 7f48648..9003109 100644
--- a/chrome_frame/utils.cc
+++ b/chrome_frame/utils.cc
@@ -1328,6 +1328,7 @@ std::string Bscf2Str(DWORD flags) {
 // Reads data from a stream into a string.
 HRESULT ReadStream(IStream* stream, size_t size, std::string* data) {
   DCHECK(stream);
+  DCHECK_GT(size, 0u);
   DCHECK(data);
 
   DWORD read = 0;
diff --git a/crypto/encryptor.h b/crypto/encryptor.h
index a407266..6b23619 100644
--- a/crypto/encryptor.h
+++ b/crypto/encryptor.h
@@ -61,13 +61,15 @@ class CRYPTO_EXPORT Encryptor {
   // Initializes the encryptor using |key| and |iv|. Returns false if either the
   // key or the initialization vector cannot be used.
   //
-  // When |mode| is CTR then |iv| should be empty.
+  // If |mode| is CBC, |iv| must not be empty; if it is CTR, then |iv| must be
+  // empty.
   bool Init(SymmetricKey* key, Mode mode, const base::StringPiece& iv);
 
-  // Encrypts |plaintext| into |ciphertext|.
+  // Encrypts |plaintext| into |ciphertext|.  |plaintext| may only be empty if
+  // the mode is CBC.
   bool Encrypt(const base::StringPiece& plaintext, std::string* ciphertext);
 
-  // Decrypts |ciphertext| into |plaintext|.
+  // Decrypts |ciphertext| into |plaintext|.  |ciphertext| must not be empty.
   bool Decrypt(const base::StringPiece& ciphertext, std::string* plaintext);
 
   // Sets the counter value when in CTR mode. Currently only 128-bits
diff --git a/crypto/encryptor_mac.cc b/crypto/encryptor_mac.cc
index a08d09e..6be373a 100644
--- a/crypto/encryptor_mac.cc
+++ b/crypto/encryptor_mac.cc
@@ -49,17 +49,19 @@ bool Encryptor::Crypt(int /*CCOperation*/ op,
   // length is never larger than the input length plus the block size."
 
   size_t output_size = input.size() + iv_.size();
+  CHECK_GT(output_size, 0u);
+  CHECK_GT(output_size + 1, input.size());
   CCCryptorStatus err = CCCrypt(op,
                                 kCCAlgorithmAES128,
                                 kCCOptionPKCS7Padding,
                                 raw_key.Data, raw_key.Length,
                                 iv_.data(),
                                 input.data(), input.size(),
-                                WriteInto(output, output_size+1),
+                                WriteInto(output, output_size + 1),
                                 output_size,
                                 &output_size);
   if (err) {
-    output->resize(0);
+    output->clear();
     LOG(ERROR) << "CCCrypt returned " << err;
     return false;
   }
@@ -69,11 +71,13 @@ bool Encryptor::Crypt(int /*CCOperation*/ op,
 
 bool Encryptor::Encrypt(const base::StringPiece& plaintext,
                         std::string* ciphertext) {
+  CHECK(!plaintext.empty() || (mode_ == CBC));
   return Crypt(kCCEncrypt, plaintext, ciphertext);
 }
 
 bool Encryptor::Decrypt(const base::StringPiece& ciphertext,
                         std::string* plaintext) {
+  CHECK(!ciphertext.empty());
   return Crypt(kCCDecrypt, ciphertext, plaintext);
 }
 
diff --git a/crypto/encryptor_nss.cc b/crypto/encryptor_nss.cc
index c4610903..cf4fa2a 100644
--- a/crypto/encryptor_nss.cc
+++ b/crypto/encryptor_nss.cc
@@ -72,13 +72,12 @@ bool Encryptor::Init(SymmetricKey* key,
       break;
   }
 
-  if (!param_.get())
-    return false;
-  return true;
+  return param_ != NULL;
 }
 
 bool Encryptor::Encrypt(const base::StringPiece& plaintext,
                         std::string* ciphertext) {
+  CHECK(!plaintext.empty() || (mode_ == CBC));
   ScopedPK11Context context(PK11_CreateContextBySymKey(GetMechanism(mode_),
                                                        CKA_ENCRYPT,
                                                        key_->key(),
@@ -86,34 +85,30 @@ bool Encryptor::Encrypt(const base::StringPiece& plaintext,
   if (!context.get())
     return false;
 
-  if (mode_ == CTR)
-    return CryptCTR(context.get(), plaintext, ciphertext);
-  else
-    return Crypt(context.get(), plaintext, ciphertext);
+  return (mode_ == CTR) ?
+      CryptCTR(context.get(), plaintext, ciphertext) :
+      Crypt(context.get(), plaintext, ciphertext);
 }
 
 bool Encryptor::Decrypt(const base::StringPiece& ciphertext,
                         std::string* plaintext) {
-  if (ciphertext.empty())
-    return false;
-
+  CHECK(!ciphertext.empty());
   ScopedPK11Context context(PK11_CreateContextBySymKey(
       GetMechanism(mode_), (mode_ == CTR ? CKA_ENCRYPT : CKA_DECRYPT),
       key_->key(), param_.get()));
   if (!context.get())
     return false;
 
-  if (mode_ == CTR)
-    return CryptCTR(context.get(), ciphertext, plaintext);
-  else
-    return Crypt(context.get(), ciphertext, plaintext);
+  return (mode_ == CTR) ?
+      CryptCTR(context.get(), ciphertext, plaintext) :
+      Crypt(context.get(), ciphertext, plaintext);
 }
 
 bool Encryptor::Crypt(PK11Context* context,
                       const base::StringPiece& input,
                       std::string* output) {
   size_t output_len = input.size() + AES_BLOCK_SIZE;
-  CHECK(output_len > input.size()) << "Output size overflow";
+  CHECK_GT(output_len, input.size());
 
   output->resize(output_len);
   uint8* output_data =
@@ -160,7 +155,7 @@ bool Encryptor::CryptCTR(PK11Context* context,
 
   size_t output_len = ((input.size() + AES_BLOCK_SIZE - 1) / AES_BLOCK_SIZE) *
       AES_BLOCK_SIZE;
-  CHECK(output_len >= input.size()) << "Output size overflow";
+  CHECK_GE(output_len, input.size());
   output->resize(output_len);
   uint8* output_data =
       reinterpret_cast<uint8*>(const_cast<char*>(output->data()));
@@ -180,7 +175,7 @@ bool Encryptor::CryptCTR(PK11Context* context,
                                mask_len);
   if (SECSuccess != rv)
     return false;
-  CHECK(op_len == static_cast<int>(mask_len));
+  CHECK_EQ(static_cast<int>(mask_len), op_len);
 
   unsigned int digest_len;
   rv = PK11_DigestFinal(context,
diff --git a/crypto/encryptor_openssl.cc b/crypto/encryptor_openssl.cc
index 6513181..cb26e22 100644
--- a/crypto/encryptor_openssl.cc
+++ b/crypto/encryptor_openssl.cc
@@ -73,11 +73,13 @@ bool Encryptor::Init(SymmetricKey* key,
 
 bool Encryptor::Encrypt(const base::StringPiece& plaintext,
                         std::string* ciphertext) {
+  CHECK(!plaintext.empty() || (mode_ == CBC));
   return Crypt(true, plaintext, ciphertext);
 }
 
 bool Encryptor::Decrypt(const base::StringPiece& ciphertext,
                         std::string* plaintext) {
+  CHECK(!ciphertext.empty());
   return Crypt(false, ciphertext, plaintext);
 }
 
@@ -88,7 +90,7 @@ bool Encryptor::Crypt(bool do_encrypt,
   // Work on the result in a local variable, and then only transfer it to
   // |output| on success to ensure no partial data is returned.
   std::string result;
-  output->swap(result);
+  output->clear();
 
   const EVP_CIPHER* cipher = GetCipherForKey(key_);
   DCHECK(cipher);  // Already handled in Init();
@@ -106,6 +108,8 @@ bool Encryptor::Crypt(bool do_encrypt,
 
   // When encrypting, add another block size of space to allow for any padding.
   const size_t output_size = input.size() + (do_encrypt ? iv_.size() : 0);
+  CHECK_GT(output_size, 0u);
+  CHECK_GT(output_size + 1, input.size());
   uint8* out_ptr = reinterpret_cast<uint8*>(WriteInto(&result,
                                                       output_size + 1));
   int out_len;
diff --git a/crypto/encryptor_unittest.cc b/crypto/encryptor_unittest.cc
index 2d710bc..7f2caf9 100644
--- a/crypto/encryptor_unittest.cc
+++ b/crypto/encryptor_unittest.cc
@@ -281,21 +281,3 @@ TEST(EncryptorTest, EmptyEncrypt) {
   EXPECT_EQ(expected_ciphertext_hex, base::HexEncode(ciphertext.data(),
                                                      ciphertext.size()));
 }
-
-TEST(EncryptorTest, EmptyDecrypt) {
-  std::string key = "128=SixteenBytes";
-  std::string iv = "Sweet Sixteen IV";
-
-  scoped_ptr<crypto::SymmetricKey> sym_key(crypto::SymmetricKey::Import(
-      crypto::SymmetricKey::AES, key));
-  ASSERT_TRUE(NULL != sym_key.get());
-
-  crypto::Encryptor encryptor;
-  // The IV must be exactly as long a the cipher block size.
-  EXPECT_EQ(16U, iv.size());
-  EXPECT_TRUE(encryptor.Init(sym_key.get(), crypto::Encryptor::CBC, iv));
-
-  std::string decrypted;
-  EXPECT_FALSE(encryptor.Decrypt("", &decrypted));
-  EXPECT_EQ("", decrypted);
-}
diff --git a/crypto/encryptor_win.cc b/crypto/encryptor_win.cc
index fae1f82..dc59519 100644
--- a/crypto/encryptor_win.cc
+++ b/crypto/encryptor_win.cc
@@ -83,8 +83,10 @@ bool Encryptor::Init(SymmetricKey* key,
 bool Encryptor::Encrypt(const base::StringPiece& plaintext,
                         std::string* ciphertext) {
   DWORD data_len = plaintext.size();
+  CHECK((data_len > 0u) || (mode_ == CBC));
   DWORD total_len = data_len + block_size_;
-  CHECK_GT(total_len, data_len);
+  CHECK_GT(total_len, 0u);
+  CHECK_GT(total_len + 1, data_len);
 
   // CryptoAPI encrypts/decrypts in place.
   char* ciphertext_data = WriteInto(ciphertext, total_len + 1);
@@ -105,8 +107,8 @@ bool Encryptor::Encrypt(const base::StringPiece& plaintext,
 bool Encryptor::Decrypt(const base::StringPiece& ciphertext,
                         std::string* plaintext) {
   DWORD data_len = ciphertext.size();
-  if (data_len == 0 || (data_len + 1) < data_len)
-    return false;
+  CHECK_GT(data_len, 0u);
+  CHECK_GT(data_len + 1, data_len);
 
   // CryptoAPI encrypts/decrypts in place.
   char* plaintext_data = WriteInto(plaintext, data_len + 1);
diff --git a/crypto/symmetric_key.h b/crypto/symmetric_key.h
index 500324e..3c25fab 100644
--- a/crypto/symmetric_key.h
+++ b/crypto/symmetric_key.h
@@ -35,15 +35,16 @@ class CRYPTO_EXPORT SymmetricKey {
   virtual ~SymmetricKey();
 
   // Generates a random key suitable to be used with |algorithm| and of
-  // |key_size_in_bits| bits.
+  // |key_size_in_bits| bits. |key_size_in_bits| must be a multiple of 8.
   // The caller is responsible for deleting the returned SymmetricKey.
   static SymmetricKey* GenerateRandomKey(Algorithm algorithm,
                                          size_t key_size_in_bits);
 
   // Derives a key from the supplied password and salt using PBKDF2, suitable
   // for use with specified |algorithm|. Note |algorithm| is not the algorithm
-  // used to derive the key from the password. The caller is responsible for
-  // deleting the returned SymmetricKey.
+  // used to derive the key from the password. |key_size_in_bits| must be a
+  // multiple of 8. The caller is responsible for deleting the returned
+  // SymmetricKey.
   static SymmetricKey* DeriveKeyFromPassword(Algorithm algorithm,
                                              const std::string& password,
                                              const std::string& salt,
diff --git a/crypto/symmetric_key_openssl.cc b/crypto/symmetric_key_openssl.cc
index 1d1ad23..899a942 100644
--- a/crypto/symmetric_key_openssl.cc
+++ b/crypto/symmetric_key_openssl.cc
@@ -24,10 +24,10 @@ SymmetricKey::~SymmetricKey() {
 SymmetricKey* SymmetricKey::GenerateRandomKey(Algorithm algorithm,
                                               size_t key_size_in_bits) {
   DCHECK_EQ(AES, algorithm);
-  int key_size_in_bytes = key_size_in_bits / 8;
-  DCHECK_EQ(static_cast<int>(key_size_in_bits), key_size_in_bytes * 8);
+  size_t key_size_in_bytes = key_size_in_bits / 8;
+  DCHECK_EQ(key_size_in_bits, key_size_in_bytes * 8);
 
-  if (key_size_in_bits == 0)
+  if (key_size_in_bytes == 0)
     return NULL;
 
   OpenSSLErrStackTracer err_tracer(FROM_HERE);
@@ -35,7 +35,7 @@ SymmetricKey* SymmetricKey::GenerateRandomKey(Algorithm algorithm,
   uint8* key_data =
       reinterpret_cast<uint8*>(WriteInto(&key->key_, key_size_in_bytes + 1));
 
-  int rv = RAND_bytes(key_data, key_size_in_bytes);
+  int rv = RAND_bytes(key_data, static_cast<int>(key_size_in_bytes));
   return rv == 1 ? key.release() : NULL;
 }
 
@@ -46,8 +46,11 @@ SymmetricKey* SymmetricKey::DeriveKeyFromPassword(Algorithm algorithm,
                                                   size_t iterations,
                                                   size_t key_size_in_bits) {
   DCHECK(algorithm == AES || algorithm == HMAC_SHA1);
-  int key_size_in_bytes = key_size_in_bits / 8;
-  DCHECK_EQ(static_cast<int>(key_size_in_bits), key_size_in_bytes * 8);
+  size_t key_size_in_bytes = key_size_in_bits / 8;
+  DCHECK_EQ(key_size_in_bits, key_size_in_bytes * 8);
+
+  if (key_size_in_bytes == 0)
+    return NULL;
 
   OpenSSLErrStackTracer err_tracer(FROM_HERE);
   scoped_ptr<SymmetricKey> key(new SymmetricKey);
@@ -56,7 +59,8 @@ SymmetricKey* SymmetricKey::DeriveKeyFromPassword(Algorithm algorithm,
   int rv = PKCS5_PBKDF2_HMAC_SHA1(password.data(), password.length(),
                                   reinterpret_cast<const uint8*>(salt.data()),
                                   salt.length(), iterations,
-                                  key_size_in_bytes, key_data);
+                                  static_cast<int>(key_size_in_bytes),
+                                  key_data);
   return rv == 1 ? key.release() : NULL;
 }
 
diff --git a/media/audio/win/audio_manager_win.cc b/media/audio/win/audio_manager_win.cc
index 92f59d8..80d0d9d 100644
--- a/media/audio/win/audio_manager_win.cc
+++ b/media/audio/win/audio_manager_win.cc
@@ -211,12 +211,14 @@ string16 AudioManagerWin::GetAudioInputDeviceModel() {
   waveInMessage(reinterpret_cast<HWAVEIN>(device_id),
                 DRV_QUERYDEVICEINTERFACESIZE,
                 reinterpret_cast<DWORD_PTR>(&device_interface_name_size), 0);
-  if (device_interface_name_size == 0)  // No audio capture device?
-    return string16();
+  size_t bytes_in_char16 = sizeof(string16::value_type);
+  DCHECK_EQ(0u, device_interface_name_size % bytes_in_char16);
+  if (device_interface_name_size <= bytes_in_char16)
+    return string16();  // No audio capture device.
 
   string16 device_interface_name;
   string16::value_type* name_ptr = WriteInto(&device_interface_name,
-      device_interface_name_size / sizeof(string16::value_type));
+      device_interface_name_size / bytes_in_char16);
   waveInMessage(reinterpret_cast<HWAVEIN>(device_id),
                 DRV_QUERYDEVICEINTERFACE,
                 reinterpret_cast<DWORD_PTR>(name_ptr),
diff --git a/net/base/net_util.cc b/net/base/net_util.cc
index 204e768..7ae4363d 100644
--- a/net/base/net_util.cc
+++ b/net/base/net_util.cc
@@ -162,13 +162,9 @@ static const int kAllowedFtpPorts[] = {
 std::string::size_type CountTrailingChars(
     const std::string input,
     const std::string::value_type trailing_chars[]) {
-  const std::string::size_type last_good_char =
-      input.find_last_not_of(trailing_chars);
-
-  if (last_good_char == std::string::npos)
-    return input.length();
-  else
-    return input.length() - last_good_char - 1;
+  const size_t last_good_char = input.find_last_not_of(trailing_chars);
+  return (last_good_char == std::string::npos) ?
+      input.length() : (input.length() - last_good_char - 1);
 }
 
 // Similar to Base64Decode. Decodes a Q-encoded string to a sequence
@@ -176,71 +172,68 @@ std::string::size_type CountTrailingChars(
 bool QPDecode(const std::string& input, std::string* output) {
   std::string temp;
   temp.reserve(input.size());
-  std::string::const_iterator it = input.begin();
-  while (it != input.end()) {
+  for (std::string::const_iterator it = input.begin(); it != input.end();
+       ++it) {
     if (*it == '_') {
       temp.push_back(' ');
     } else if (*it == '=') {
-      if (input.end() - it < 3) {
-        return false;
-      }
-      if (IsHexDigit(static_cast<unsigned char>(*(it + 1))) &&
-          IsHexDigit(static_cast<unsigned char>(*(it + 2)))) {
-        unsigned char ch = HexDigitToInt(*(it + 1)) * 16 +
-                           HexDigitToInt(*(it + 2));
-        temp.push_back(static_cast<char>(ch));
-        ++it;
-        ++it;
-      } else {
+      if ((input.end() - it < 3) ||
+          !IsHexDigit(static_cast<unsigned char>(*(it + 1))) ||
+          !IsHexDigit(static_cast<unsigned char>(*(it + 2))))
         return false;
-      }
+      unsigned char ch = HexDigitToInt(*(it + 1)) * 16 +
+                         HexDigitToInt(*(it + 2));
+      temp.push_back(static_cast<char>(ch));
+      ++it;
+      ++it;
     } else if (0x20 < *it && *it < 0x7F) {
       // In a Q-encoded word, only printable ASCII characters
       // represent themselves. Besides, space, '=', '_' and '?' are
       // not allowed, but they're already filtered out.
-      DCHECK(*it != 0x3D && *it != 0x5F && *it != 0x3F);
+      DCHECK_NE('=', *it);
+      DCHECK_NE('?', *it);
+      DCHECK_NE('_', *it);
       temp.push_back(*it);
     } else {
       return false;
     }
-    ++it;
   }
   output->swap(temp);
   return true;
 }
 
 enum RFC2047EncodingType {Q_ENCODING, B_ENCODING};
-bool DecodeBQEncoding(const std::string& part, RFC2047EncodingType enc_type,
-                       const std::string& charset, std::string* output) {
+bool DecodeBQEncoding(const std::string& part,
+                      RFC2047EncodingType enc_type,
+                      const std::string& charset,
+                      std::string* output) {
   std::string decoded;
-  if (enc_type == B_ENCODING) {
-    if (!base::Base64Decode(part, &decoded)) {
-      return false;
-    }
-  } else {
-    if (!QPDecode(part, &decoded)) {
-      return false;
-    }
+  if (!((enc_type == B_ENCODING) ?
+      base::Base64Decode(part, &decoded) : QPDecode(part, &decoded)))
+    return false;
+
+  if (decoded.empty()) {
+    output->clear();
+    return true;
   }
 
   UErrorCode err = U_ZERO_ERROR;
   UConverter* converter(ucnv_open(charset.c_str(), &err));
-  if (U_FAILURE(err)) {
+  if (U_FAILURE(err))
     return false;
-  }
 
   // A single byte in a legacy encoding can be expanded to 3 bytes in UTF-8.
   // A 'two-byte character' in a legacy encoding can be expanded to 4 bytes
-  // in UTF-8. Therefore, the expansion ratio is 3 at most.
-  int length = static_cast<int>(decoded.length());
-  char* buf = WriteInto(output, length * 3);
-  length = ucnv_toAlgorithmic(UCNV_UTF8, converter, buf, length * 3,
-      decoded.data(), length, &err);
+  // in UTF-8. Therefore, the expansion ratio is 3 at most. Add one for a
+  // trailing '\0'.
+  size_t output_length = decoded.length() * 3 + 1;
+  char* buf = WriteInto(output, output_length);
+  output_length = ucnv_toAlgorithmic(UCNV_UTF8, converter, buf, output_length,
+                                     decoded.data(), decoded.length(), &err);
   ucnv_close(converter);
-  if (U_FAILURE(err)) {
+  if (U_FAILURE(err))
     return false;
-  }
-  output->resize(length);
+  output->resize(output_length);
   return true;
 }
 
diff --git a/net/base/x509_certificate_win.cc b/net/base/x509_certificate_win.cc
index 7309021..12acdaf 100644
--- a/net/base/x509_certificate_win.cc
+++ b/net/base/x509_certificate_win.cc
@@ -557,18 +557,22 @@ void X509Certificate::Initialize() {
                             &cert_handle_->pCertInfo->Subject,
                             CERT_X500_NAME_STR | CERT_NAME_STR_CRLF_FLAG,
                             NULL, 0);
-  name_size = CertNameToStr(cert_handle_->dwCertEncodingType,
-                            &cert_handle_->pCertInfo->Subject,
-                            CERT_X500_NAME_STR | CERT_NAME_STR_CRLF_FLAG,
-                            WriteInto(&subject_info, name_size), name_size);
+  if (name_size > 1) {
+    CertNameToStr(cert_handle_->dwCertEncodingType,
+                  &cert_handle_->pCertInfo->Subject,
+                  CERT_X500_NAME_STR | CERT_NAME_STR_CRLF_FLAG,
+                  WriteInto(&subject_info, name_size), name_size);
+  }
   name_size = CertNameToStr(cert_handle_->dwCertEncodingType,
                             &cert_handle_->pCertInfo->Issuer,
                             CERT_X500_NAME_STR | CERT_NAME_STR_CRLF_FLAG,
                             NULL, 0);
-  name_size = CertNameToStr(cert_handle_->dwCertEncodingType,
-                            &cert_handle_->pCertInfo->Issuer,
-                            CERT_X500_NAME_STR | CERT_NAME_STR_CRLF_FLAG,
-                            WriteInto(&issuer_info, name_size), name_size);
+  if (name_size > 1) {
+    CertNameToStr(cert_handle_->dwCertEncodingType,
+                  &cert_handle_->pCertInfo->Issuer,
+                  CERT_X500_NAME_STR | CERT_NAME_STR_CRLF_FLAG,
+                  WriteInto(&issuer_info, name_size), name_size);
+  }
   ParsePrincipal(WideToUTF8(subject_info), &subject_);
   ParsePrincipal(WideToUTF8(issuer_info), &issuer_);
 
diff --git a/net/disk_cache/entry_impl.cc b/net/disk_cache/entry_impl.cc
index 976e16f..0a6a2d2 100644
--- a/net/disk_cache/entry_impl.cc
+++ b/net/disk_cache/entry_impl.cc
@@ -773,11 +773,11 @@ std::string EntryImpl::GetKey() const {
   File* key_file = const_cast<EntryImpl*>(this)->GetBackingFile(address,
                                                                 kKeyFileIndex);
 
-  if (!offset && key_file->GetLength() != static_cast<size_t>(key_len + 1))
+  ++key_len;  // We store a trailing \0 on disk that we read back below.
+  if (!offset && key_file->GetLength() != static_cast<size_t>(key_len))
     return std::string();
 
-  if (!key_file ||
-      !key_file->Read(WriteInto(&key_, key_len + 1), key_len + 1, offset))
+  if (!key_file || !key_file->Read(WriteInto(&key_, key_len), key_len, offset))
     key_.clear();
   return key_;
 }
diff --git a/net/http/http_mac_signature.cc b/net/http/http_mac_signature.cc
index 93498ef..e445165 100644
--- a/net/http/http_mac_signature.cc
+++ b/net/http/http_mac_signature.cc
@@ -158,9 +158,12 @@ bool HttpMacSignature::GenerateMAC(const std::string& age,
 
   std::string signature;
   size_t length = hmac.DigestLength();
-  char* buffer = WriteInto(&signature, length);
-  if (!hmac.Sign(request, reinterpret_cast<unsigned char*>(buffer),
-                 length)) {
+  DCHECK_GT(length, 0u);
+  if (!hmac.Sign(request,
+          // We need the + 1 here not because the call will write a trailing \0,
+          // but so that signature.length() is correctly set to |length|.
+          reinterpret_cast<unsigned char*>(WriteInto(&signature, length + 1)),
+          length)) {
     NOTREACHED();
     return false;
   }
diff --git a/net/http/http_mac_signature_unittest.cc b/net/http/http_mac_signature_unittest.cc
index 74064dd..130ce39 100644
--- a/net/http/http_mac_signature_unittest.cc
+++ b/net/http/http_mac_signature_unittest.cc
@@ -59,7 +59,7 @@ TEST(HttpMacSignatureTest, GenerateHeaderString) {
   EXPECT_TRUE(signature.GenerateHeaderString(age, nonce, &header_string));
   EXPECT_EQ("MAC id=\"dfoi30j0qnf\", "
             "nonce=\"239034:mn4302j0n+32r2/f3r=\", "
-            "mac=\"GrkHtPKzB1m1dCHfa7OCWOw6EQ==\"",
+            "mac=\"GrkHtPKzB1m1dCHfa7OCWOw6Ecw=\"",
             header_string);
 }
 
@@ -104,6 +104,6 @@ TEST(HttpMacSignatureTest, GenerateMAC) {
 
   std::string mac;
   EXPECT_TRUE(signature.GenerateMAC(age, nonce, &mac));
-  EXPECT_EQ("GrkHtPKzB1m1dCHfa7OCWOw6EQ==", mac);
+  EXPECT_EQ("GrkHtPKzB1m1dCHfa7OCWOw6Ecw=", mac);
 }
 }
diff --git a/sandbox/src/process_policy_test.cc b/sandbox/src/process_policy_test.cc
index 8681671..1bcfa88 100644
--- a/sandbox/src/process_policy_test.cc
+++ b/sandbox/src/process_policy_test.cc
@@ -5,11 +5,11 @@
 #include <memory>
 #include <string>
 
+#include "base/sys_string_conversions.h"
 #include "base/win/scoped_handle.h"
 #include "sandbox/src/sandbox.h"
 #include "sandbox/src/sandbox_policy.h"
 #include "sandbox/src/sandbox_factory.h"
-#include "sandbox/src/sandbox_utils.h"
 #include "sandbox/tests/common/controller.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -67,9 +67,9 @@ sandbox::SboxTestResult CreateProcessHelper(const std::wstring &exe,
 
   std::string narrow_cmd_line;
   if (cmd_line)
-    narrow_cmd_line = sandbox::WideToMultiByte(cmd_line);
+    narrow_cmd_line = base::SysWideToMultiByte(cmd_line, CP_UTF8);
   if (!::CreateProcessA(
-        exe_name ? sandbox::WideToMultiByte(exe_name).c_str() : NULL,
+        exe_name ? base::SysWideToMultiByte(exe_name, CP_UTF8).c_str() : NULL,
         cmd_line ? const_cast<char*>(narrow_cmd_line.c_str()) : NULL,
         NULL, NULL, FALSE, 0, NULL, NULL, &sia, &pi)) {
     DWORD last_error = GetLastError();
diff --git a/sandbox/src/sandbox_utils.cc b/sandbox/src/sandbox_utils.cc
index b93434c..3bfa696 100644
--- a/sandbox/src/sandbox_utils.cc
+++ b/sandbox/src/sandbox_utils.cc
@@ -76,21 +76,4 @@ void InitObjectAttribs(const std::wstring& name, ULONG attributes, HANDLE root,
   InitializeObjectAttributes(obj_attr, uni_name, attributes, root, NULL);
 }
 
-std::string WideToMultiByte(const std::wstring& wide) {
-  if (wide.length() == 0)
-    return std::string();
-
-  // compute the length of the buffer we'll need
-  int charcount = WideCharToMultiByte(CP_UTF8, 0, wide.c_str(), -1,
-                                      NULL, 0, NULL, NULL);
-  if (charcount == 0)
-    return std::string();
-
-  std::string mb;
-  WideCharToMultiByte(CP_UTF8, 0, wide.c_str(), -1,
-                      WriteInto(&mb, charcount), charcount, NULL, NULL);
-
-  return mb;
-}
-
 };  // namespace sandbox
diff --git a/sandbox/src/sandbox_utils.h b/sandbox/src/sandbox_utils.h
index e6cac5d..314330f 100644
--- a/sandbox/src/sandbox_utils.h
+++ b/sandbox/src/sandbox_utils.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2006-2008 The Chromium Authors. All rights reserved.
+// Copyright (c) 2011 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.
 
@@ -33,18 +33,6 @@ bool IsXPSP2OrLater();
 void InitObjectAttribs(const std::wstring& name, ULONG attributes, HANDLE root,
                        OBJECT_ATTRIBUTES* obj_attr, UNICODE_STRING* uni_name);
 
-// The next 2 functions are copied from base\string_util.h and have been
-// slighty modified because we don't want to depend on ICU.
-template <class string_type>
-inline typename string_type::value_type* WriteInto(string_type* str,
-                                                   size_t length_with_null) {
-  str->reserve(length_with_null);
-  str->resize(length_with_null - 1);
-  return &((*str)[0]);
-}
-
-std::string WideToMultiByte(const std::wstring& wide);
-
 };  // namespace sandbox
 
 #endif  // SANDBOX_SRC_SANDBOX_UTILS_H__
diff --git a/sandbox/tests/common/controller.cc b/sandbox/tests/common/controller.cc
index b618069..a221ee1 100644
--- a/sandbox/tests/common/controller.cc
+++ b/sandbox/tests/common/controller.cc
@@ -6,6 +6,7 @@
 
 #include <string>
 
+#include "base/sys_string_conversions.h"
 #include "base/win/windows_version.h"
 #include "sandbox/src/sandbox_factory.h"
 #include "sandbox/src/sandbox_utils.h"
@@ -261,7 +262,7 @@ int DispatchCall(int argc, wchar_t **argv) {
                              &module))
     return SBOX_TEST_FAILED_TO_EXECUTE_COMMAND;
 
-  std::string command_name = WideToMultiByte(argv[3]);
+  std::string command_name = base::SysWideToMultiByte(argv[3], CP_UTF8);
   CommandFunction command = reinterpret_cast<CommandFunction>(
                                 ::GetProcAddress(module, command_name.c_str()));
   if (!command)
diff --git a/ui/base/clipboard/clipboard_win.cc b/ui/base/clipboard/clipboard_win.cc
index 492dd31..a543fe6 100644
--- a/ui/base/clipboard/clipboard_win.cc
+++ b/ui/base/clipboard/clipboard_win.cc
@@ -604,7 +604,8 @@ void Clipboard::ReadFiles(std::vector<FilePath>* files) const {
 
   if (count) {
     for (int i = 0; i < count; ++i) {
-      int size = ::DragQueryFile(drop, i, NULL, 0) + 1;
+      UINT size = ::DragQueryFile(drop, i, NULL, 0) + 1;
+      DCHECK_GT(size, 1u);
       std::wstring file;
       ::DragQueryFile(drop, i, WriteInto(&file, size), size);
       files->push_back(FilePath(file));
diff --git a/ui/base/l10n/l10n_util.cc b/ui/base/l10n/l10n_util.cc
index 1c2aca0..28dbc43 100644
--- a/ui/base/l10n/l10n_util.cc
+++ b/ui/base/l10n/l10n_util.cc
@@ -478,12 +478,12 @@ string16 GetDisplayNameForLocale(const std::string& locale,
     locale_code = "zh-Hant";
 
   UErrorCode error = U_ZERO_ERROR;
-  const int buffer_size = 1024;
+  const int kBufferSize = 1024;
 
   string16 display_name;
   int actual_size = uloc_getDisplayName(locale_code.c_str(),
       display_locale.c_str(),
-      WriteInto(&display_name, buffer_size + 1), buffer_size, &error);
+      WriteInto(&display_name, kBufferSize), kBufferSize - 1, &error);
   DCHECK(U_SUCCESS(error));
   display_name.resize(actual_size);
   // Add an RTL mark so parentheses are properly placed.
diff --git a/ui/base/win/ime_input.cc b/ui/base/win/ime_input.cc
index 50bf3ea..049ab02 100644
--- a/ui/base/win/ime_input.cc
+++ b/ui/base/win/ime_input.cc
@@ -339,22 +339,19 @@ void ImeInput::GetCompositionInfo(HIMC imm_context, LPARAM lparam,
   }
 }
 
-bool ImeInput::GetString(HIMC imm_context, WPARAM lparam, int type,
+bool ImeInput::GetString(HIMC imm_context,
+                         WPARAM lparam,
+                         int type,
                          string16* result) {
-  bool ret = false;
-  if (lparam & type) {
-    int string_size = ::ImmGetCompositionString(imm_context, type, NULL, 0);
-    if (string_size > 0) {
-      int string_length = string_size / sizeof(wchar_t);
-      wchar_t *string_data = WriteInto(result, string_length + 1);
-      if (string_data) {
-        // Fill the given result object.
-        ::ImmGetCompositionString(imm_context, type, string_data, string_size);
-        ret = true;
-      }
-    }
-  }
-  return ret;
+  if (!(lparam & type))
+    return false;
+  LONG string_size = ::ImmGetCompositionString(imm_context, type, NULL, 0);
+  if (string_size <= 0)
+    return false;
+  DCHECK_EQ(0u, string_size % sizeof(wchar_t));
+  ::ImmGetCompositionString(imm_context, type,
+      WriteInto(result, (string_size / sizeof(wchar_t)) + 1), string_size);
+  return true;
 }
 
 bool ImeInput::GetResult(HWND window_handle, LPARAM lparam, string16* result) {
diff --git a/ui/views/controls/textfield/native_textfield_win.cc b/ui/views/controls/textfield/native_textfield_win.cc
index 20cae12..3b598e1 100644
--- a/ui/views/controls/textfield/native_textfield_win.cc
+++ b/ui/views/controls/textfield/native_textfield_win.cc
@@ -173,18 +173,16 @@ void NativeTextfieldWin::AttachHack() {
 
 string16 NativeTextfieldWin::GetText() const {
   int len = GetTextLength() + 1;
-  if (len <= 1)
-    return string16();
-
   string16 str;
-  GetWindowText(WriteInto(&str, len), len);
+  if (len > 1)
+    GetWindowText(WriteInto(&str, len), len);
   // The text get from GetWindowText() might be wrapped with explicit bidi
   // control characters. Refer to UpdateText() for detail. Without such
   // wrapping, in RTL chrome, a pure LTR string ending with parenthesis will
   // not be displayed correctly in a textfield. For example, "Yahoo!" will be
   // displayed as "!Yahoo", and "Google (by default)" will be displayed as
   // "(Google (by default".
-  return base::i18n::StripWrappingBidiControlCharacters(WideToUTF16(str));
+  return base::i18n::StripWrappingBidiControlCharacters(str);
 }
 
 void NativeTextfieldWin::UpdateText() {
@@ -206,15 +204,11 @@ void NativeTextfieldWin::AppendText(const string16& text) {
 }
 
 string16 NativeTextfieldWin::GetSelectedText() const {
-  // Figure out the length of the selection.
   CHARRANGE sel;
   GetSel(sel);
-  if (sel.cpMin == sel.cpMax)  // GetSelText() crashes on NULL input.
-    return string16();
-
-  // Grab the selected text.
   string16 str;
-  GetSelText(WriteInto(&str, sel.cpMax - sel.cpMin + 1));
+  if (sel.cpMin != sel.cpMax)
+    GetSelText(WriteInto(&str, sel.cpMax - sel.cpMin + 1));
   return str;
 }