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_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;
}
