Add help IsValid* methods to string classes

The various string classes, CFX_ByteString, CFX_ByteStringC,
CFX_WideString, and CFX_WideStringC, have many conditionals that are
effectively determining if a value is a valid index or length. This CL
refactors the logic into one place per class, so it only needs to be
changed once if its behaviour needs to change. It also make the some
of the methods stricter on the inputs they will accept.

BUG=pdfium:828

Change-Id: Iadcdaa34a6d862a2804485770027179c89dc6956
Reviewed-on: https://pdfium-review.googlesource.com/12030
Commit-Queue: Ryan Harrison <rharrison@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
diff --git a/core/fxcrt/cfx_bytestring.cpp b/core/fxcrt/cfx_bytestring.cpp
index f6074e9..ac39094 100644
--- a/core/fxcrt/cfx_bytestring.cpp
+++ b/core/fxcrt/cfx_bytestring.cpp
@@ -456,24 +456,39 @@
   m_pData.Swap(pNewData);
 }
 
-CFX_ByteString CFX_ByteString::Mid(FX_STRSIZE nFirst, FX_STRSIZE nCount) const {
-  ASSERT(nCount >= 0);
+CFX_ByteString CFX_ByteString::Mid(FX_STRSIZE first, FX_STRSIZE count) const {
   if (!m_pData)
     return CFX_ByteString();
 
-  nFirst = pdfium::clamp(nFirst, 0, m_pData->m_nDataLength);
-  nCount = pdfium::clamp(nCount, 0, m_pData->m_nDataLength - nFirst);
-  if (nCount == 0)
+  if (!IsValidIndex(first))
     return CFX_ByteString();
 
-  if (nFirst == 0 && nCount == m_pData->m_nDataLength)
+  if (count == 0 || !IsValidLength(count))
+    return CFX_ByteString();
+
+  if (!IsValidIndex(first + count - 1))
+    return CFX_ByteString();
+
+  if (first == 0 && count == m_pData->m_nDataLength)
     return *this;
 
   CFX_ByteString dest;
-  AllocCopy(dest, nCount, nFirst);
+  AllocCopy(dest, count, first);
   return dest;
 }
 
+CFX_ByteString CFX_ByteString::Left(FX_STRSIZE count) const {
+  if (count == 0 || !IsValidLength(count))
+    return CFX_ByteString();
+  return Mid(0, count);
+}
+
+CFX_ByteString CFX_ByteString::Right(FX_STRSIZE count) const {
+  if (count == 0 || !IsValidLength(count))
+    return CFX_ByteString();
+  return Mid(GetLength() - count, count);
+}
+
 void CFX_ByteString::AllocCopy(CFX_ByteString& dest,
                                FX_STRSIZE nCopyLen,
                                FX_STRSIZE nCopyIndex) const {
@@ -519,78 +534,51 @@
 }
 
 void CFX_ByteString::SetAt(FX_STRSIZE index, char c) {
-  ASSERT(index >= 0 && index < GetLength());
+  ASSERT(IsValidIndex(index));
   ReallocBeforeWrite(m_pData->m_nDataLength);
   m_pData->m_String[index] = c;
 }
 
-FX_STRSIZE CFX_ByteString::Insert(FX_STRSIZE index, char ch) {
+FX_STRSIZE CFX_ByteString::Insert(FX_STRSIZE location, char ch) {
   const FX_STRSIZE cur_length = m_pData ? m_pData->m_nDataLength : 0;
-  if (index != pdfium::clamp(index, 0, cur_length))
+  if (!IsValidLength(location))
     return cur_length;
 
   const FX_STRSIZE new_length = cur_length + 1;
   ReallocBeforeWrite(new_length);
-  memmove(m_pData->m_String + index + 1, m_pData->m_String + index,
-          new_length - index);
-  m_pData->m_String[index] = ch;
+  memmove(m_pData->m_String + location + 1, m_pData->m_String + location,
+          new_length - location);
+  m_pData->m_String[location] = ch;
   m_pData->m_nDataLength = new_length;
   return new_length;
 }
 
-CFX_ByteString CFX_ByteString::Right(FX_STRSIZE nCount) const {
-  if (!m_pData)
-    return CFX_ByteString();
-
-  nCount = std::max(nCount, 0);
-  if (nCount >= m_pData->m_nDataLength)
-    return *this;
-
-  CFX_ByteString dest;
-  AllocCopy(dest, nCount, m_pData->m_nDataLength - nCount);
-  return dest;
-}
-
-CFX_ByteString CFX_ByteString::Left(FX_STRSIZE nCount) const {
-  if (!m_pData)
-    return CFX_ByteString();
-
-  nCount = std::max(nCount, 0);
-  if (nCount >= m_pData->m_nDataLength)
-    return *this;
-
-  CFX_ByteString dest;
-  AllocCopy(dest, nCount, 0);
-  return dest;
-}
-
 pdfium::Optional<FX_STRSIZE> CFX_ByteString::Find(char ch,
-                                                  FX_STRSIZE nStart) const {
+                                                  FX_STRSIZE start) const {
   if (!m_pData)
     return pdfium::Optional<FX_STRSIZE>();
 
-  if (nStart < 0 || nStart >= m_pData->m_nDataLength)
+  if (!IsValidIndex(start))
     return pdfium::Optional<FX_STRSIZE>();
 
   const char* pStr = static_cast<const char*>(
-      memchr(m_pData->m_String + nStart, ch, m_pData->m_nDataLength - nStart));
+      memchr(m_pData->m_String + start, ch, m_pData->m_nDataLength - start));
   return pStr ? pdfium::Optional<FX_STRSIZE>(
                     static_cast<FX_STRSIZE>(pStr - m_pData->m_String))
               : pdfium::Optional<FX_STRSIZE>();
 }
 
-pdfium::Optional<FX_STRSIZE> CFX_ByteString::Find(const CFX_ByteStringC& pSub,
-                                                  FX_STRSIZE nStart) const {
+pdfium::Optional<FX_STRSIZE> CFX_ByteString::Find(const CFX_ByteStringC& subStr,
+                                                  FX_STRSIZE start) const {
   if (!m_pData)
     return pdfium::Optional<FX_STRSIZE>();
 
-  FX_STRSIZE nLength = m_pData->m_nDataLength;
-  if (nStart > nLength)
+  if (!IsValidIndex(start))
     return pdfium::Optional<FX_STRSIZE>();
 
   const char* pStr =
-      FX_strstr(m_pData->m_String + nStart, m_pData->m_nDataLength - nStart,
-                pSub.unterminated_c_str(), pSub.GetLength());
+      FX_strstr(m_pData->m_String + start, m_pData->m_nDataLength - start,
+                subStr.unterminated_c_str(), subStr.GetLength());
   return pStr ? pdfium::Optional<FX_STRSIZE>(
                     static_cast<FX_STRSIZE>(pStr - m_pData->m_String))
               : pdfium::Optional<FX_STRSIZE>();
@@ -653,7 +641,7 @@
   }
 
   *pstrDest = 0;
-  FX_STRSIZE nCount = (FX_STRSIZE)(pstrSource - pstrDest);
+  FX_STRSIZE nCount = static_cast<FX_STRSIZE>(pstrSource - pstrDest);
   m_pData->m_nDataLength -= nCount;
   return nCount;
 }
@@ -669,7 +657,7 @@
   const char* pStart = m_pData->m_String;
   char* pEnd = m_pData->m_String + m_pData->m_nDataLength;
   while (1) {
-    const char* pTarget = FX_strstr(pStart, (FX_STRSIZE)(pEnd - pStart),
+    const char* pTarget = FX_strstr(pStart, static_cast<int>(pEnd - pStart),
                                     pOld.unterminated_c_str(), nSourceLen);
     if (!pTarget)
       break;
@@ -692,7 +680,7 @@
   pStart = m_pData->m_String;
   char* pDest = pNewData->m_String;
   for (FX_STRSIZE i = 0; i < nCount; i++) {
-    const char* pTarget = FX_strstr(pStart, (FX_STRSIZE)(pEnd - pStart),
+    const char* pTarget = FX_strstr(pStart, static_cast<int>(pEnd - pStart),
                                     pOld.unterminated_c_str(), nSourceLen);
     memcpy(pDest, pStart, pTarget - pStart);
     pDest += pTarget - pStart;
@@ -708,7 +696,7 @@
 CFX_WideString CFX_ByteString::UTF8Decode() const {
   CFX_UTF8Decoder decoder;
   for (FX_STRSIZE i = 0; i < GetLength(); i++) {
-    decoder.Input((uint8_t)m_pData->m_String[i]);
+    decoder.Input(static_cast<uint8_t>(m_pData->m_String[i]));
   }
   return CFX_WideString(decoder.GetResult());
 }
@@ -722,14 +710,14 @@
   if (!m_pData) {
     return str.IsEmpty() ? 0 : -1;
   }
-  int this_len = m_pData->m_nDataLength;
-  int that_len = str.GetLength();
-  int min_len = this_len < that_len ? this_len : that_len;
-  for (int i = 0; i < min_len; i++) {
-    if ((uint8_t)m_pData->m_String[i] < str[i]) {
+  FX_STRSIZE this_len = m_pData->m_nDataLength;
+  FX_STRSIZE that_len = str.GetLength();
+  FX_STRSIZE min_len = std::min(this_len, that_len);
+  for (FX_STRSIZE i = 0; i < min_len; i++) {
+    if (static_cast<uint8_t>(m_pData->m_String[i]) < str[i]) {
       return -1;
     }
-    if ((uint8_t)m_pData->m_String[i] > str[i]) {
+    if (static_cast<uint8_t>(m_pData->m_String[i]) > str[i]) {
       return 1;
     }
   }
@@ -836,7 +824,7 @@
     return 1;
   }
   char buf2[32];
-  int buf_size = 0;
+  FX_STRSIZE buf_size = 0;
   if (bNegative) {
     buf[buf_size++] = '-';
   }
diff --git a/core/fxcrt/cfx_bytestring.h b/core/fxcrt/cfx_bytestring.h
index 10675be..a8a2816 100644
--- a/core/fxcrt/cfx_bytestring.h
+++ b/core/fxcrt/cfx_bytestring.h
@@ -84,6 +84,14 @@
   }
   bool IsEmpty() const { return !GetLength(); }
 
+  bool IsValidIndex(FX_STRSIZE index) const {
+    return 0 <= index && index < GetLength();
+  }
+
+  bool IsValidLength(FX_STRSIZE length) const {
+    return 0 <= length && length <= GetLength();
+  }
+
   int Compare(const CFX_ByteStringC& str) const;
   bool EqualNoCase(const CFX_ByteStringC& str) const;
 
@@ -109,7 +117,7 @@
   const CFX_ByteString& operator+=(const CFX_ByteStringC& bstrc);
 
   CharType operator[](const FX_STRSIZE index) const {
-    ASSERT(index >= 0 && index < GetLength());
+    ASSERT(IsValidIndex(index));
     return m_pData ? m_pData->m_String[index] : 0;
   }
 
diff --git a/core/fxcrt/cfx_bytestring_unittest.cpp b/core/fxcrt/cfx_bytestring_unittest.cpp
index e728154..497e0b6 100644
--- a/core/fxcrt/cfx_bytestring_unittest.cpp
+++ b/core/fxcrt/cfx_bytestring_unittest.cpp
@@ -291,7 +291,7 @@
 TEST(fxcrt, ByteStringCNull) {
   CFX_ByteStringC null_string;
   EXPECT_FALSE(null_string.raw_str());
-  EXPECT_EQ(null_string.GetLength(), 0);
+  EXPECT_EQ(0, null_string.GetLength());
   EXPECT_TRUE(null_string.IsEmpty());
 
   CFX_ByteStringC another_null_string;
@@ -299,27 +299,27 @@
 
   CFX_ByteStringC copied_null_string(null_string);
   EXPECT_FALSE(copied_null_string.raw_str());
-  EXPECT_EQ(copied_null_string.GetLength(), 0);
+  EXPECT_EQ(0, copied_null_string.GetLength());
   EXPECT_TRUE(copied_null_string.IsEmpty());
   EXPECT_EQ(null_string, copied_null_string);
 
   CFX_ByteStringC empty_string("");  // Pointer to NUL, not NULL pointer.
   EXPECT_TRUE(empty_string.raw_str());
-  EXPECT_EQ(empty_string.GetLength(), 0);
+  EXPECT_EQ(0, empty_string.GetLength());
   EXPECT_TRUE(empty_string.IsEmpty());
   EXPECT_EQ(null_string, empty_string);
 
   CFX_ByteStringC assigned_null_string("initially not nullptr");
   assigned_null_string = null_string;
   EXPECT_FALSE(assigned_null_string.raw_str());
-  EXPECT_EQ(assigned_null_string.GetLength(), 0);
+  EXPECT_EQ(0, assigned_null_string.GetLength());
   EXPECT_TRUE(assigned_null_string.IsEmpty());
   EXPECT_EQ(null_string, assigned_null_string);
 
   CFX_ByteStringC assigned_nullptr_string("initially not nullptr");
   assigned_nullptr_string = nullptr;
   EXPECT_FALSE(assigned_nullptr_string.raw_str());
-  EXPECT_EQ(assigned_nullptr_string.GetLength(), 0);
+  EXPECT_EQ(0, assigned_nullptr_string.GetLength());
   EXPECT_TRUE(assigned_nullptr_string.IsEmpty());
   EXPECT_EQ(null_string, assigned_nullptr_string);
 
@@ -345,13 +345,6 @@
   fred.Concat("DY", 2);
   EXPECT_EQ("FREDDY", fred);
   EXPECT_EQ("FRED", copy);
-
-  // Test invalid arguments.
-  copy = fred;
-  fred.Concat("freddy", -6);
-  CFX_ByteString not_aliased("xxxxxx");
-  EXPECT_EQ("FREDDY", fred);
-  EXPECT_EQ("xxxxxx", not_aliased);
 }
 
 TEST(fxcrt, ByteStringRemove) {
@@ -524,10 +517,10 @@
   EXPECT_EQ("D", fred.Mid(3, 1));
   EXPECT_EQ("FR", fred.Mid(0, 2));
   EXPECT_EQ("FRED", fred.Mid(0, 4));
-  EXPECT_EQ("FRED", fred.Mid(0, 10));
+  EXPECT_EQ("", fred.Mid(0, 10));
 
-  EXPECT_EQ("FR", fred.Mid(-1, 2));
-  EXPECT_EQ("RED", fred.Mid(1, 4));
+  EXPECT_EQ("", fred.Mid(-1, 2));
+  EXPECT_EQ("RED", fred.Mid(1, 3));
   EXPECT_EQ("", fred.Mid(4, 1));
 
   CFX_ByteString empty;
@@ -542,7 +535,7 @@
   EXPECT_EQ("FRE", fred.Left(3));
   EXPECT_EQ("FRED", fred.Left(4));
 
-  EXPECT_EQ("FRED", fred.Left(5));
+  EXPECT_EQ("", fred.Left(5));
   EXPECT_EQ("", fred.Left(-1));
 
   CFX_ByteString empty;
@@ -559,7 +552,7 @@
   EXPECT_EQ("RED", fred.Right(3));
   EXPECT_EQ("FRED", fred.Right(4));
 
-  EXPECT_EQ("FRED", fred.Right(5));
+  EXPECT_EQ("", fred.Right(5));
   EXPECT_EQ("", fred.Right(-1));
 
   CFX_ByteString empty;
@@ -1004,31 +997,31 @@
   EXPECT_EQ(null_string, null_string.Mid(1, 1));
 
   CFX_ByteStringC empty_string("");
-  EXPECT_EQ(empty_string, empty_string.Mid(0, 1));
-  EXPECT_EQ(empty_string, empty_string.Mid(1, 1));
+  EXPECT_EQ("", empty_string.Mid(0, 1));
+  EXPECT_EQ("", empty_string.Mid(1, 1));
 
   CFX_ByteStringC single_character("a");
-  EXPECT_EQ(empty_string, single_character.Mid(0, 0));
+  EXPECT_EQ("", single_character.Mid(0, 0));
   EXPECT_EQ(single_character, single_character.Mid(0, 1));
-  EXPECT_EQ(empty_string, single_character.Mid(1, 0));
-  EXPECT_EQ(empty_string, single_character.Mid(1, 1));
+  EXPECT_EQ("", single_character.Mid(1, 0));
+  EXPECT_EQ("", single_character.Mid(1, 1));
 
   CFX_ByteStringC longer_string("abcdef");
   EXPECT_EQ(longer_string, longer_string.Mid(0, 6));
-  EXPECT_EQ(longer_string, longer_string.Mid(0, 187));
-  EXPECT_EQ(longer_string, longer_string.Mid(-42, 6));
-  EXPECT_EQ(longer_string, longer_string.Mid(-42, 187));
+  EXPECT_EQ("", longer_string.Mid(0, 187));
+  EXPECT_EQ("", longer_string.Mid(-42, 6));
+  EXPECT_EQ("", longer_string.Mid(-42, 187));
 
   CFX_ByteStringC leading_substring("ab");
   EXPECT_EQ(leading_substring, longer_string.Mid(0, 2));
-  EXPECT_EQ(leading_substring, longer_string.Mid(-1, 2));
+  EXPECT_EQ("", longer_string.Mid(-1, 2));
 
   CFX_ByteStringC middle_substring("bcde");
   EXPECT_EQ(middle_substring, longer_string.Mid(1, 4));
 
   CFX_ByteStringC trailing_substring("ef");
   EXPECT_EQ(trailing_substring, longer_string.Mid(4, 2));
-  EXPECT_EQ(trailing_substring, longer_string.Mid(4, 3));
+  EXPECT_EQ("", longer_string.Mid(4, 3));
 }
 
 TEST(fxcrt, ByteStringCElementAccess) {
diff --git a/core/fxcrt/cfx_string_c_template.h b/core/fxcrt/cfx_string_c_template.h
index db8b274..c46b0c4 100644
--- a/core/fxcrt/cfx_string_c_template.h
+++ b/core/fxcrt/cfx_string_c_template.h
@@ -119,15 +119,24 @@
   }
 
   FX_STRSIZE GetLength() const { return m_Length; }
+
   bool IsEmpty() const { return m_Length == 0; }
 
+  bool IsValidIndex(FX_STRSIZE index) const {
+    return 0 <= index && index < GetLength();
+  }
+
+  bool IsValidLength(FX_STRSIZE length) const {
+    return 0 <= length && length <= GetLength();
+  }
+
   const UnsignedType& operator[](const FX_STRSIZE index) const {
-    ASSERT(index >= 0 && index < GetLength());
+    ASSERT(IsValidIndex(index));
     return m_Ptr.Get()[index];
   }
 
   const CharType CharAt(const FX_STRSIZE index) const {
-    ASSERT(index >= 0 && index < GetLength());
+    ASSERT(IsValidIndex(index));
     return static_cast<CharType>(m_Ptr.Get()[index]);
   }
 
@@ -141,33 +150,32 @@
 
   bool Contains(CharType ch) const { return Find(ch).has_value(); }
 
-  CFX_StringCTemplate Mid(FX_STRSIZE index, FX_STRSIZE count) const {
-    ASSERT(count >= 0);
-    if (index > m_Length)
+  CFX_StringCTemplate Mid(FX_STRSIZE first, FX_STRSIZE count) const {
+    if (!m_Ptr.Get())
       return CFX_StringCTemplate();
 
-    index = pdfium::clamp(index, 0, m_Length);
-    count = pdfium::clamp(count, 0, m_Length - index);
-    if (count == 0)
+    if (!IsValidIndex(first))
       return CFX_StringCTemplate();
 
-    return CFX_StringCTemplate(m_Ptr.Get() + index, count);
+    if (count == 0 || !IsValidLength(count))
+      return CFX_StringCTemplate();
+
+    if (!IsValidIndex(first + count - 1))
+      return CFX_StringCTemplate();
+
+    return CFX_StringCTemplate(m_Ptr.Get() + first, count);
   }
 
   CFX_StringCTemplate Left(FX_STRSIZE count) const {
-    count = pdfium::clamp(count, 0, m_Length);
-    if (count == 0)
+    if (count == 0 || !IsValidLength(count))
       return CFX_StringCTemplate();
-
-    return CFX_StringCTemplate(m_Ptr.Get(), count);
+    return Mid(0, count);
   }
 
   CFX_StringCTemplate Right(FX_STRSIZE count) const {
-    count = pdfium::clamp(count, 0, m_Length);
-    if (count == 0)
+    if (count == 0 || !IsValidLength(count))
       return CFX_StringCTemplate();
-
-    return CFX_StringCTemplate(m_Ptr.Get() + m_Length - count, count);
+    return Mid(GetLength() - count, count);
   }
 
   bool operator<(const CFX_StringCTemplate& that) const {
diff --git a/core/fxcrt/cfx_widestring.cpp b/core/fxcrt/cfx_widestring.cpp
index 76fdf24..6e4e82d 100644
--- a/core/fxcrt/cfx_widestring.cpp
+++ b/core/fxcrt/cfx_widestring.cpp
@@ -459,7 +459,7 @@
   if (m_pData && m_pData->CanOperateInPlace(nNewLength))
     return;
 
-  if (nNewLength <= 0) {
+  if (nNewLength == 0) {
     clear();
     return;
   }
@@ -552,7 +552,7 @@
     return old_length;
 
   ReallocBeforeWrite(old_length);
-  int chars_to_copy = old_length - removal_length + 1;
+  FX_STRSIZE chars_to_copy = old_length - removal_length + 1;
   wmemmove(m_pData->m_String + index, m_pData->m_String + removal_length,
            chars_to_copy);
   m_pData->m_nDataLength = old_length - count;
@@ -602,24 +602,39 @@
   return result;
 }
 
-CFX_WideString CFX_WideString::Mid(FX_STRSIZE nFirst, FX_STRSIZE nCount) const {
-  ASSERT(nCount >= 0);
+CFX_WideString CFX_WideString::Mid(FX_STRSIZE first, FX_STRSIZE count) const {
   if (!m_pData)
     return CFX_WideString();
 
-  nFirst = pdfium::clamp(nFirst, 0, m_pData->m_nDataLength);
-  nCount = pdfium::clamp(nCount, 0, m_pData->m_nDataLength - nFirst);
-  if (nCount == 0)
+  if (!IsValidIndex(first))
     return CFX_WideString();
 
-  if (nFirst == 0 && nCount == m_pData->m_nDataLength)
+  if (count == 0 || !IsValidLength(count))
+    return CFX_WideString();
+
+  if (!IsValidIndex(first + count - 1))
+    return CFX_WideString();
+
+  if (first == 0 && count == GetLength())
     return *this;
 
   CFX_WideString dest;
-  AllocCopy(dest, nCount, nFirst);
+  AllocCopy(dest, count, first);
   return dest;
 }
 
+CFX_WideString CFX_WideString::Left(FX_STRSIZE count) const {
+  if (count == 0 || !IsValidLength(count))
+    return CFX_WideString();
+  return Mid(0, count);
+}
+
+CFX_WideString CFX_WideString::Right(FX_STRSIZE count) const {
+  if (count == 0 || !IsValidLength(count))
+    return CFX_WideString();
+  return Mid(GetLength() - count, count);
+}
+
 void CFX_WideString::AllocCopy(CFX_WideString& dest,
                                FX_STRSIZE nCopyLen,
                                FX_STRSIZE nCopyIndex) const {
@@ -651,24 +666,25 @@
   return bSufficientBuffer;
 }
 
-void CFX_WideString::FormatV(const wchar_t* pFormat, va_list argList) {
+void CFX_WideString::FormatV(const wchar_t* format, va_list argList) {
   va_list argListCopy;
   FX_VA_COPY(argListCopy, argList);
-  FX_STRSIZE nMaxLen = vswprintf(nullptr, 0, pFormat, argListCopy);
+  int maxLen = vswprintf(nullptr, 0, format, argListCopy);
   va_end(argListCopy);
-  if (nMaxLen <= 0) {
-    auto guess = GuessSizeForVSWPrintf(pFormat, argListCopy);
+  if (maxLen <= 0) {
+    auto guess = GuessSizeForVSWPrintf(format, argListCopy);
     if (!guess.has_value())
       return;
-    nMaxLen = guess.value();
+    maxLen = guess.value();
   }
-  while (nMaxLen < 32 * 1024) {
+  while (maxLen < 32 * 1024) {
     FX_VA_COPY(argListCopy, argList);
-    bool bSufficientBuffer = TryVSWPrintf(nMaxLen, pFormat, argListCopy);
+    bool bSufficientBuffer =
+        TryVSWPrintf(static_cast<FX_STRSIZE>(maxLen), format, argListCopy);
     va_end(argListCopy);
     if (bSufficientBuffer)
       break;
-    nMaxLen *= 2;
+    maxLen *= 2;
   }
 }
 
@@ -679,73 +695,46 @@
   va_end(argList);
 }
 
-FX_STRSIZE CFX_WideString::Insert(FX_STRSIZE index, wchar_t ch) {
+FX_STRSIZE CFX_WideString::Insert(FX_STRSIZE location, wchar_t ch) {
   const FX_STRSIZE cur_length = m_pData ? m_pData->m_nDataLength : 0;
-  if (index != pdfium::clamp(index, 0, cur_length))
+  if (!IsValidLength(location))
     return cur_length;
 
   const FX_STRSIZE new_length = cur_length + 1;
   ReallocBeforeWrite(new_length);
-  wmemmove(m_pData->m_String + index + 1, m_pData->m_String + index,
-           new_length - index);
-  m_pData->m_String[index] = ch;
+  wmemmove(m_pData->m_String + location + 1, m_pData->m_String + location,
+           new_length - location);
+  m_pData->m_String[location] = ch;
   m_pData->m_nDataLength = new_length;
   return new_length;
 }
 
-CFX_WideString CFX_WideString::Right(FX_STRSIZE nCount) const {
-  if (!m_pData)
-    return CFX_WideString();
-
-  nCount = std::max(nCount, 0);
-  if (nCount >= m_pData->m_nDataLength)
-    return *this;
-
-  CFX_WideString dest;
-  AllocCopy(dest, nCount, m_pData->m_nDataLength - nCount);
-  return dest;
-}
-
-CFX_WideString CFX_WideString::Left(FX_STRSIZE nCount) const {
-  if (!m_pData)
-    return CFX_WideString();
-
-  nCount = std::max(nCount, 0);
-  if (nCount >= m_pData->m_nDataLength)
-    return *this;
-
-  CFX_WideString dest;
-  AllocCopy(dest, nCount, 0);
-  return dest;
-}
-
 pdfium::Optional<FX_STRSIZE> CFX_WideString::Find(wchar_t ch,
-                                                  FX_STRSIZE nStart) const {
+                                                  FX_STRSIZE start) const {
   if (!m_pData)
     return pdfium::Optional<FX_STRSIZE>();
 
-  if (nStart < 0 || nStart >= m_pData->m_nDataLength)
+  if (!IsValidIndex(start))
     return pdfium::Optional<FX_STRSIZE>();
 
   const wchar_t* pStr =
-      wmemchr(m_pData->m_String + nStart, ch, m_pData->m_nDataLength - nStart);
+      wmemchr(m_pData->m_String + start, ch, m_pData->m_nDataLength - start);
   return pStr ? pdfium::Optional<FX_STRSIZE>(
                     static_cast<FX_STRSIZE>(pStr - m_pData->m_String))
               : pdfium::Optional<FX_STRSIZE>();
 }
 
-pdfium::Optional<FX_STRSIZE> CFX_WideString::Find(const CFX_WideStringC& pSub,
-                                                  FX_STRSIZE nStart) const {
+pdfium::Optional<FX_STRSIZE> CFX_WideString::Find(const CFX_WideStringC& subStr,
+                                                  FX_STRSIZE start) const {
   if (!m_pData)
     return pdfium::Optional<FX_STRSIZE>();
 
-  FX_STRSIZE nLength = m_pData->m_nDataLength;
-  if (nStart > nLength)
+  if (!IsValidIndex(start))
     return pdfium::Optional<FX_STRSIZE>();
 
   const wchar_t* pStr =
-      FX_wcsstr(m_pData->m_String + nStart, m_pData->m_nDataLength - nStart,
-                pSub.unterminated_c_str(), pSub.GetLength());
+      FX_wcsstr(m_pData->m_String + start, m_pData->m_nDataLength - start,
+                subStr.unterminated_c_str(), subStr.GetLength());
   return pStr ? pdfium::Optional<FX_STRSIZE>(
                     static_cast<FX_STRSIZE>(pStr - m_pData->m_String))
               : pdfium::Optional<FX_STRSIZE>();
@@ -796,9 +785,9 @@
   }
 
   *pstrDest = 0;
-  FX_STRSIZE nCount = (FX_STRSIZE)(pstrSource - pstrDest);
-  m_pData->m_nDataLength -= nCount;
-  return nCount;
+  FX_STRSIZE count = static_cast<FX_STRSIZE>(pstrSource - pstrDest);
+  m_pData->m_nDataLength -= count;
+  return count;
 }
 
 FX_STRSIZE CFX_WideString::Replace(const CFX_WideStringC& pOld,
@@ -808,7 +797,7 @@
 
   FX_STRSIZE nSourceLen = pOld.GetLength();
   FX_STRSIZE nReplacementLen = pNew.GetLength();
-  FX_STRSIZE nCount = 0;
+  FX_STRSIZE count = 0;
   const wchar_t* pStart = m_pData->m_String;
   wchar_t* pEnd = m_pData->m_String + m_pData->m_nDataLength;
   while (1) {
@@ -817,24 +806,24 @@
     if (!pTarget)
       break;
 
-    nCount++;
+    count++;
     pStart = pTarget + nSourceLen;
   }
-  if (nCount == 0)
+  if (count == 0)
     return 0;
 
   FX_STRSIZE nNewLength =
-      m_pData->m_nDataLength + (nReplacementLen - nSourceLen) * nCount;
+      m_pData->m_nDataLength + (nReplacementLen - nSourceLen) * count;
 
   if (nNewLength == 0) {
     clear();
-    return nCount;
+    return count;
   }
 
   CFX_RetainPtr<StringData> pNewData(StringData::Create(nNewLength));
   pStart = m_pData->m_String;
   wchar_t* pDest = pNewData->m_String;
-  for (FX_STRSIZE i = 0; i < nCount; i++) {
+  for (FX_STRSIZE i = 0; i < count; i++) {
     const wchar_t* pTarget = FX_wcsstr(pStart, (FX_STRSIZE)(pEnd - pStart),
                                        pOld.unterminated_c_str(), nSourceLen);
     wmemcpy(pDest, pStart, pTarget - pStart);
@@ -845,7 +834,7 @@
   }
   wmemcpy(pDest, pStart, pEnd - pStart);
   m_pData.Swap(pNewData);
-  return nCount;
+  return count;
 }
 
 // static
@@ -874,13 +863,13 @@
 // static
 CFX_WideString CFX_WideString::FromUTF16LE(const unsigned short* wstr,
                                            FX_STRSIZE wlen) {
-  if (!wstr || 0 == wlen) {
+  if (!wstr || wlen <= 0) {
     return CFX_WideString();
   }
 
   CFX_WideString result;
   wchar_t* buf = result.GetBuffer(wlen);
-  for (int i = 0; i < wlen; i++) {
+  for (FX_STRSIZE i = 0; i < wlen; i++) {
     buf[i] = wstr[i];
   }
   result.ReleaseBuffer(wlen);
@@ -888,7 +877,7 @@
 }
 
 void CFX_WideString::SetAt(FX_STRSIZE index, wchar_t c) {
-  ASSERT(index >= 0 && index < GetLength());
+  ASSERT(IsValidIndex(index));
   ReallocBeforeWrite(m_pData->m_nDataLength);
   m_pData->m_String[index] = c;
 }
@@ -909,10 +898,10 @@
   if (!str.m_pData) {
     return 1;
   }
-  int this_len = m_pData->m_nDataLength;
-  int that_len = str.m_pData->m_nDataLength;
-  int min_len = this_len < that_len ? this_len : that_len;
-  for (int i = 0; i < min_len; i++) {
+  FX_STRSIZE this_len = m_pData->m_nDataLength;
+  FX_STRSIZE that_len = str.m_pData->m_nDataLength;
+  FX_STRSIZE min_len = std::min(this_len, that_len);
+  for (FX_STRSIZE i = 0; i < min_len; i++) {
     if (m_pData->m_String[i] < str.m_pData->m_String[i]) {
       return -1;
     }
@@ -1035,7 +1024,7 @@
       cc++;
     }
   }
-  fraction += (float)integer;
+  fraction += static_cast<float>(integer);
   return bNegative ? -fraction : fraction;
 }
 
diff --git a/core/fxcrt/cfx_widestring.h b/core/fxcrt/cfx_widestring.h
index a6d0eca..2421388 100644
--- a/core/fxcrt/cfx_widestring.h
+++ b/core/fxcrt/cfx_widestring.h
@@ -81,6 +81,14 @@
   }
   bool IsEmpty() const { return !GetLength(); }
 
+  bool IsValidIndex(FX_STRSIZE index) const {
+    return 0 <= index && index < GetLength();
+  }
+
+  bool IsValidLength(FX_STRSIZE length) const {
+    return 0 <= length && length <= GetLength();
+  }
+
   const CFX_WideString& operator=(const wchar_t* str);
   const CFX_WideString& operator=(const CFX_WideString& stringSrc);
   const CFX_WideString& operator=(const CFX_WideStringC& stringSrc);
@@ -103,7 +111,7 @@
   bool operator<(const CFX_WideString& str) const;
 
   CharType operator[](const FX_STRSIZE index) const {
-    ASSERT(index >= 0 && index < GetLength());
+    ASSERT(IsValidIndex(index));
     return m_pData ? m_pData->m_String[index] : 0;
   }
 
diff --git a/core/fxcrt/cfx_widestring_unittest.cpp b/core/fxcrt/cfx_widestring_unittest.cpp
index e688a53..0005cb3 100644
--- a/core/fxcrt/cfx_widestring_unittest.cpp
+++ b/core/fxcrt/cfx_widestring_unittest.cpp
@@ -304,13 +304,6 @@
   fred.Concat(L"DY", 2);
   EXPECT_EQ(L"FREDDY", fred);
   EXPECT_EQ(L"FRED", copy);
-
-  // Test invalid arguments.
-  copy = fred;
-  fred.Concat(L"freddy", -6);
-  CFX_WideString not_aliased(L"xxxxxx");
-  EXPECT_EQ(L"FREDDY", fred);
-  EXPECT_EQ(L"xxxxxx", not_aliased);
 }
 
 TEST(fxcrt, WideStringRemove) {
@@ -483,10 +476,10 @@
   EXPECT_EQ(L"D", fred.Mid(3, 1));
   EXPECT_EQ(L"FR", fred.Mid(0, 2));
   EXPECT_EQ(L"FRED", fred.Mid(0, 4));
-  EXPECT_EQ(L"FRED", fred.Mid(0, 10));
+  EXPECT_EQ(L"", fred.Mid(0, 10));
 
-  EXPECT_EQ(L"FR", fred.Mid(-1, 2));
-  EXPECT_EQ(L"RED", fred.Mid(1, 4));
+  EXPECT_EQ(L"", fred.Mid(-1, 2));
+  EXPECT_EQ(L"", fred.Mid(1, 4));
   EXPECT_EQ(L"", fred.Mid(4, 1));
 
   CFX_WideString empty;
@@ -501,7 +494,7 @@
   EXPECT_EQ(L"FRE", fred.Left(3));
   EXPECT_EQ(L"FRED", fred.Left(4));
 
-  EXPECT_EQ(L"FRED", fred.Left(5));
+  EXPECT_EQ(L"", fred.Left(5));
   EXPECT_EQ(L"", fred.Left(-1));
 
   CFX_WideString empty;
@@ -518,7 +511,7 @@
   EXPECT_EQ(L"RED", fred.Right(3));
   EXPECT_EQ(L"FRED", fred.Right(4));
 
-  EXPECT_EQ(L"FRED", fred.Right(5));
+  EXPECT_EQ(L"", fred.Right(5));
   EXPECT_EQ(L"", fred.Right(-1));
 
   CFX_WideString empty;
diff --git a/fxbarcode/oned/BC_OnedCode128Writer.cpp b/fxbarcode/oned/BC_OnedCode128Writer.cpp
index 95a95b6..a6dc749 100644
--- a/fxbarcode/oned/BC_OnedCode128Writer.cpp
+++ b/fxbarcode/oned/BC_OnedCode128Writer.cpp
@@ -193,7 +193,9 @@
     int32_t patternIndex;
     char ch = contents[position];
     if (std::isdigit(ch)) {
-      patternIndex = FXSYS_atoi(contents.Mid(position, 2).c_str());
+      patternIndex = FXSYS_atoi(
+          contents.Mid(position, position + 1 < contents.GetLength() ? 2 : 1)
+              .c_str());
       ++position;
       if (position < contents.GetLength() && std::isdigit(contents[position]))
         ++position;