Replace some in/out parameters in cfgas_stringformatter.cpp
Doing so allows the optimizer to remove another bounds check
in GetLiteralText() since there is less aliasing possible.
- small logic tidy in FormatDateTime()
- consolidate FormatNull() and FormatZero() implementations.
Change-Id: If7b2e7acc1b01ccb64792510a64286016c623ad0
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/51850
Commit-Queue: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
diff --git a/xfa/fgas/crt/cfgas_stringformatter.cpp b/xfa/fgas/crt/cfgas_stringformatter.cpp
index ee8321f..2118b08 100644
--- a/xfa/fgas/crt/cfgas_stringformatter.cpp
+++ b/xfa/fgas/crt/cfgas_stringformatter.cpp
@@ -108,64 +108,67 @@
return iKeyValue;
}
-WideString GetLiteralText(pdfium::span<const wchar_t> pStrPattern,
- size_t* iPattern) {
+// Returns {next character position, literal text string}.
+std::pair<size_t, WideString> GetLiteralText(
+ pdfium::span<const wchar_t> pStrPattern,
+ size_t iPattern) {
WideString wsOutput;
- if (*iPattern >= pStrPattern.size() || pStrPattern[*iPattern] != '\'')
- return wsOutput;
+ if (iPattern >= pStrPattern.size() || pStrPattern[iPattern] != '\'')
+ return {iPattern, wsOutput};
- (*iPattern)++;
+ iPattern++;
int32_t iQuote = 1;
- while (*iPattern < pStrPattern.size()) {
- if (pStrPattern[*iPattern] == '\'') {
+ while (iPattern < pStrPattern.size()) {
+ if (pStrPattern[iPattern] == '\'') {
iQuote++;
- if ((*iPattern + 1 >= pStrPattern.size()) ||
- ((pStrPattern[*iPattern + 1] != '\'') && (iQuote % 2 == 0))) {
+ if (iPattern + 1 >= pStrPattern.size() ||
+ ((pStrPattern[iPattern + 1] != '\'') && (iQuote % 2 == 0))) {
break;
}
iQuote++;
- (*iPattern)++;
- } else if (pStrPattern[*iPattern] == '\\' &&
- (*iPattern + 1 < pStrPattern.size()) &&
- pStrPattern[*iPattern + 1] == 'u') {
+ iPattern++;
+ } else if (pStrPattern[iPattern] == '\\' &&
+ iPattern + 1 < pStrPattern.size() &&
+ pStrPattern[iPattern + 1] == 'u') {
int32_t iKeyValue = 0;
- *iPattern += 2;
- for (int32_t i = 0; *iPattern < pStrPattern.size() && i < 4; ++i) {
- wchar_t ch = pStrPattern[(*iPattern)++];
+ iPattern += 2;
+ for (size_t i = 0; iPattern < pStrPattern.size() && i < 4; ++i) {
+ wchar_t ch = pStrPattern[iPattern++];
iKeyValue = ConvertHex(iKeyValue, ch);
}
if (iKeyValue != 0)
wsOutput += static_cast<wchar_t>(iKeyValue & 0x0000FFFF);
-
continue;
}
- wsOutput += pStrPattern[(*iPattern)++];
+ wsOutput += pStrPattern[iPattern++];
}
- return wsOutput;
+ return {iPattern, wsOutput};
}
-WideString GetLiteralTextReverse(pdfium::span<const wchar_t> pStrPattern,
- size_t* iPattern) {
+// Returns {next character position, literal text string}.
+std::pair<size_t, WideString> GetLiteralTextReverse(
+ pdfium::span<const wchar_t> pStrPattern,
+ size_t iPattern) {
WideString wsOutput;
- if (*iPattern >= pStrPattern.size() || pStrPattern[*iPattern] != '\'')
- return wsOutput;
+ if (iPattern >= pStrPattern.size() || pStrPattern[iPattern] != '\'')
+ return {iPattern, wsOutput};
- (*iPattern)--;
+ iPattern--;
int32_t iQuote = 1;
- while (*iPattern < pStrPattern.size()) {
- if (pStrPattern[*iPattern] == '\'') {
+ while (iPattern < pStrPattern.size()) {
+ if (pStrPattern[iPattern] == '\'') {
iQuote++;
- if (*iPattern - 1 >= pStrPattern.size() ||
- ((pStrPattern[*iPattern - 1] != '\'') && (iQuote % 2 == 0))) {
+ if (iPattern - 1 >= pStrPattern.size() ||
+ ((pStrPattern[iPattern - 1] != '\'') && (iQuote % 2 == 0))) {
break;
}
iQuote++;
- (*iPattern)--;
- } else if (pStrPattern[*iPattern] == '\\' &&
- *iPattern + 1 < pStrPattern.size() &&
- pStrPattern[*iPattern + 1] == 'u') {
- (*iPattern)--;
+ iPattern--;
+ } else if (pStrPattern[iPattern] == '\\' &&
+ iPattern + 1 < pStrPattern.size() &&
+ pStrPattern[iPattern + 1] == 'u') {
+ iPattern--;
int32_t iKeyValue = 0;
int32_t iLen = wsOutput.GetLength();
int32_t i = 1;
@@ -179,9 +182,9 @@
}
continue;
}
- wsOutput = pStrPattern[(*iPattern)--] + wsOutput;
+ wsOutput = pStrPattern[iPattern--] + wsOutput;
}
- return wsOutput;
+ return {iPattern, wsOutput};
}
bool GetNumericDotIndex(const WideString& wsNum,
@@ -191,7 +194,7 @@
pdfium::span<const wchar_t> spDotSymbol = wsDotSymbol.AsSpan();
for (size_t ccf = 0; ccf < spNum.size(); ++ccf) {
if (spNum[ccf] == '\'') {
- GetLiteralText(spNum, &ccf);
+ ccf = GetLiteralText(spNum, ccf).first;
continue;
}
if (ccf + spDotSymbol.size() <= spNum.size() &&
@@ -241,7 +244,8 @@
WideStringView wsDateSymbols(gs_wsDateSymbols);
while (*cc < spDate.size() && ccf < spDatePattern.size()) {
if (spDatePattern[ccf] == '\'') {
- WideString wsLiteral = GetLiteralText(spDatePattern, &ccf);
+ WideString wsLiteral;
+ std::tie(ccf, wsLiteral) = GetLiteralText(spDatePattern, ccf);
int32_t iLiteralLen = wsLiteral.GetLength();
if (*cc + iLiteralLen > spDate.size() ||
wcsncmp(spDate.data() + *cc, wsLiteral.c_str(), iLiteralLen) != 0) {
@@ -364,8 +368,9 @@
WideStringView wsTimeSymbols(gs_wsTimeSymbols);
while (*cc < spTime.size() && ccf < spTimePattern.size()) {
if (spTimePattern[ccf] == '\'') {
- WideString wsLiteral = GetLiteralText(spTimePattern, &ccf);
- int32_t iLiteralLen = wsLiteral.GetLength();
+ WideString wsLiteral;
+ std::tie(ccf, wsLiteral) = GetLiteralText(spTimePattern, ccf);
+ size_t iLiteralLen = wsLiteral.GetLength();
if (*cc + iLiteralLen > spTime.size() ||
wcsncmp(spTime.data() + *cc, wsLiteral.c_str(), iLiteralLen) != 0) {
return false;
@@ -576,7 +581,9 @@
WideStringView wsDateSymbols(gs_wsDateSymbols);
while (ccf < spDatePattern.size()) {
if (spDatePattern[ccf] == '\'') {
- wsResult += GetLiteralText(spDatePattern, &ccf);
+ WideString wsPiece;
+ std::tie(ccf, wsPiece) = GetLiteralText(spDatePattern, ccf);
+ wsResult += wsPiece;
ccf++;
continue;
}
@@ -646,7 +653,9 @@
WideStringView wsTimeSymbols(gs_wsTimeSymbols);
while (ccf < spTimePattern.size()) {
if (spTimePattern[ccf] == '\'') {
- wsResult += GetLiteralText(spTimePattern, &ccf);
+ WideString wsPiece;
+ std::tie(ccf, wsPiece) = GetLiteralText(spTimePattern, ccf);
+ wsResult += wsPiece;
ccf++;
continue;
}
@@ -857,7 +866,7 @@
WideStringView wsConstChars(gs_wsConstChars);
while (ccf < spPattern.size()) {
if (spPattern[ccf] == '\'') {
- GetLiteralText(spPattern, &ccf);
+ ccf = GetLiteralText(spPattern, ccf).first;
} else if (!bBraceOpen && !wsConstChars.Contains(spPattern[ccf])) {
WideString wsCategory(spPattern[ccf]);
ccf++;
@@ -913,7 +922,7 @@
while (ccf < spPattern.size()) {
if (spPattern[ccf] == '\'') {
int32_t iCurChar = ccf;
- GetLiteralText(spPattern, &ccf);
+ ccf = GetLiteralText(spPattern, ccf).first;
wsPurgePattern +=
WideStringView(spPattern.data() + iCurChar, ccf - iCurChar + 1);
} else if (!bBrackOpen && !wsConstChars.Contains(spPattern[ccf])) {
@@ -965,7 +974,7 @@
while (ccf < spPattern.size()) {
if (spPattern[ccf] == '\'') {
int32_t iCurChar = ccf;
- GetLiteralText(spPattern, &ccf);
+ ccf = GetLiteralText(spPattern, ccf).first;
*wsPurgePattern +=
WideStringView(spPattern.data() + iCurChar, ccf - iCurChar + 1);
} else if (!bBrackOpen && !wsConstChars.Contains(spPattern[ccf])) {
@@ -1074,8 +1083,9 @@
while (iPattern < spTextFormat.size() && iText < spSrcText.size()) {
switch (spTextFormat[iPattern]) {
case '\'': {
- WideString wsLiteral = GetLiteralText(spTextFormat, &iPattern);
- int32_t iLiteralLen = wsLiteral.GetLength();
+ WideString wsLiteral;
+ std::tie(iPattern, wsLiteral) = GetLiteralText(spTextFormat, iPattern);
+ size_t iLiteralLen = wsLiteral.GetLength();
if (iText + iLiteralLen > spSrcText.size() ||
wcsncmp(spSrcText.data() + iText, wsLiteral.c_str(), iLiteralLen) !=
0) {
@@ -1176,7 +1186,8 @@
while (ccf < spNumFormat.size() && cc < spSrcNum.size()) {
switch (spNumFormat[ccf]) {
case '\'': {
- WideString wsLiteral = GetLiteralTextReverse(spNumFormat, &ccf);
+ WideString wsLiteral;
+ std::tie(ccf, wsLiteral) = GetLiteralTextReverse(spNumFormat, ccf);
int32_t iLiteralLen = wsLiteral.GetLength();
cc -= iLiteralLen - 1;
if (cc >= spSrcNum.size() ||
@@ -1366,8 +1377,9 @@
cc < spSrcNum.size() && ccf < spNumFormat.size(); ++ccf) {
switch (spNumFormat[ccf]) {
case '\'': {
- WideString wsLiteral = GetLiteralText(spNumFormat, &ccf);
- int32_t iLiteralLen = wsLiteral.GetLength();
+ WideString wsLiteral;
+ std::tie(ccf, wsLiteral) = GetLiteralText(spNumFormat, ccf);
+ size_t iLiteralLen = wsLiteral.GetLength();
if (cc + iLiteralLen > spSrcNum.size() ||
wcsncmp(spSrcNum.data() + cc, wsLiteral.c_str(), iLiteralLen) !=
0) {
@@ -1557,7 +1569,7 @@
while (ccf < spPattern.size()) {
if (spPattern[ccf] == '\'') {
int32_t iCurChar = ccf;
- GetLiteralText(spPattern, &ccf);
+ ccf = GetLiteralText(spPattern, ccf).first;
wsTempPattern +=
WideStringView(spPattern.data() + iCurChar, ccf - iCurChar + 1);
} else if (!bBraceOpen && iFindCategory != 3 &&
@@ -1733,8 +1745,9 @@
size_t iPattern = 0;
while (iPattern < spTextFormat.size() && iText < spSrcText.size()) {
if (spTextFormat[iPattern] == '\'') {
- WideString wsLiteral = GetLiteralText(spTextFormat, &iPattern);
- int32_t iLiteralLen = wsLiteral.GetLength();
+ WideString wsLiteral;
+ std::tie(iPattern, wsLiteral) = GetLiteralText(spTextFormat, iPattern);
+ size_t iLiteralLen = wsLiteral.GetLength();
if (iText + iLiteralLen > spSrcText.size() ||
wcsncmp(spSrcText.data() + iText, wsLiteral.c_str(), iLiteralLen)) {
return false;
@@ -1762,8 +1775,9 @@
size_t iPattern = 0;
while (iPattern < spTextFormat.size() && iText < spSrcText.size()) {
if (spTextFormat[iPattern] == '\'') {
- WideString wsLiteral = GetLiteralText(spTextFormat, &iPattern);
- int32_t iLiteralLen = wsLiteral.GetLength();
+ WideString wsLiteral;
+ std::tie(iPattern, wsLiteral) = GetLiteralText(spTextFormat, iPattern);
+ size_t iLiteralLen = wsLiteral.GetLength();
if (iText + iLiteralLen > spSrcText.size() ||
wcsncmp(spSrcText.data() + iText, wsLiteral.c_str(), iLiteralLen)) {
return false;
@@ -1796,7 +1810,9 @@
while (iPattern < spTextFormat.size()) {
switch (spTextFormat[iPattern]) {
case '\'': {
- *wsOutput += GetLiteralText(spTextFormat, &iPattern);
+ WideString wsPiece;
+ std::tie(iPattern, wsPiece) = GetLiteralText(spTextFormat, iPattern);
+ *wsOutput += wsPiece;
iPattern++;
break;
}
@@ -1872,7 +1888,7 @@
for (size_t ccf = 0; ccf < dot_index_f; ++ccf) {
switch (spNumFormat[ccf]) {
case '\'':
- GetLiteralText(spNumFormat, &ccf);
+ ccf = GetLiteralText(spNumFormat, ccf).first;
break;
case '9':
case 'z':
@@ -2040,9 +2056,12 @@
case ')':
wsOutput->InsertAtFront(bNeg ? L')' : L' ');
break;
- case '\'':
- *wsOutput = GetLiteralTextReverse(spNumFormat, &ccf) + *wsOutput;
+ case '\'': {
+ WideString wsPiece;
+ std::tie(ccf, wsPiece) = GetLiteralTextReverse(spNumFormat, ccf);
+ *wsOutput = wsPiece + *wsOutput;
break;
+ }
default:
wsOutput->InsertAtFront(spNumFormat[ccf]);
break;
@@ -2086,9 +2105,12 @@
cc = dot_index.value() + 1;
for (size_t ccf = dot_index_f + 1; ccf < spNumFormat.size(); ++ccf) {
switch (spNumFormat[ccf]) {
- case '\'':
- *wsOutput += GetLiteralText(spNumFormat, &ccf);
+ case '\'': {
+ WideString wsPiece;
+ std::tie(ccf, wsPiece) = GetLiteralText(spNumFormat, ccf);
+ *wsOutput += wsPiece;
break;
+ }
case '9':
if (cc < spSrcNum.size()) {
if (!FXSYS_IsDecimalDigit(spSrcNum[cc]))
@@ -2202,12 +2224,12 @@
return false;
if (eCategory == FX_DATETIMETYPE_Unknown) {
+ if (eDateTimeType == FX_DATETIMETYPE_Unknown)
+ return false;
+
if (eDateTimeType == FX_DATETIMETYPE_Time)
wsTimePattern = std::move(wsDatePattern);
-
eCategory = eDateTimeType;
- if (eCategory == FX_DATETIMETYPE_Unknown)
- return false;
}
CFX_DateTime dt;
@@ -2225,52 +2247,49 @@
pLocale);
return true;
}
- } else {
- pdfium::span<const wchar_t> wsSrcDate =
- wsSrcDateTime.AsSpan().first(iT.value());
- pdfium::span<const wchar_t> wsSrcTime =
- wsSrcDateTime.AsSpan().subspan(iT.value() + 1);
- if (wsSrcDate.empty() || wsSrcTime.empty())
- return false;
+ return false;
+ }
- if (FX_DateFromCanonical(wsSrcDate, &dt) &&
- FX_TimeFromCanonical(pLocale, wsSrcTime, &dt)) {
- *wsOutput = FormatDateTimeInternal(dt, wsDatePattern, wsTimePattern,
- eCategory != FX_DATETIMETYPE_TimeDate,
- pLocale);
- return true;
- }
+ pdfium::span<const wchar_t> wsSrcDate =
+ wsSrcDateTime.AsSpan().first(iT.value());
+ pdfium::span<const wchar_t> wsSrcTime =
+ wsSrcDateTime.AsSpan().subspan(iT.value() + 1);
+ if (wsSrcDate.empty() || wsSrcTime.empty())
+ return false;
+
+ if (FX_DateFromCanonical(wsSrcDate, &dt) &&
+ FX_TimeFromCanonical(pLocale, wsSrcTime, &dt)) {
+ *wsOutput =
+ FormatDateTimeInternal(dt, wsDatePattern, wsTimePattern,
+ eCategory != FX_DATETIMETYPE_TimeDate, pLocale);
+ return true;
}
return false;
}
bool CFGAS_StringFormatter::FormatZero(const WideString& wsPattern,
WideString* wsOutput) const {
- if (wsPattern.IsEmpty())
- return false;
-
- WideString wsTextFormat = GetTextFormat(wsPattern, L"zero");
- pdfium::span<const wchar_t> spTextFormat = wsTextFormat.AsSpan();
- for (size_t iPattern = 0; iPattern < spTextFormat.size(); ++iPattern) {
- if (spTextFormat[iPattern] == '\'') {
- *wsOutput += GetLiteralText(spTextFormat, &iPattern);
- continue;
- }
- *wsOutput += spTextFormat[iPattern];
- }
- return true;
+ return FormatZeroOrNull(L"zero", wsPattern, wsOutput);
}
bool CFGAS_StringFormatter::FormatNull(const WideString& wsPattern,
WideString* wsOutput) const {
+ return FormatZeroOrNull(L"null", wsPattern, wsOutput);
+}
+
+bool CFGAS_StringFormatter::FormatZeroOrNull(const wchar_t* what,
+ const WideString& wsPattern,
+ WideString* wsOutput) const {
if (wsPattern.IsEmpty())
return false;
- WideString wsTextFormat = GetTextFormat(wsPattern, L"null");
+ WideString wsTextFormat = GetTextFormat(wsPattern, what);
pdfium::span<const wchar_t> spTextFormat = wsTextFormat.AsSpan();
for (size_t iPattern = 0; iPattern < spTextFormat.size(); ++iPattern) {
if (spTextFormat[iPattern] == '\'') {
- *wsOutput += GetLiteralText(spTextFormat, &iPattern);
+ WideString wsPiece;
+ std::tie(iPattern, wsPiece) = GetLiteralText(spTextFormat, iPattern);
+ *wsOutput += wsPiece;
continue;
}
*wsOutput += spTextFormat[iPattern];
diff --git a/xfa/fgas/crt/cfgas_stringformatter.h b/xfa/fgas/crt/cfgas_stringformatter.h
index 9583724..cc534f4 100644
--- a/xfa/fgas/crt/cfgas_stringformatter.h
+++ b/xfa/fgas/crt/cfgas_stringformatter.h
@@ -67,6 +67,9 @@
LocaleIface** pLocale,
WideString* wsDatePattern,
WideString* wsTimePattern) const;
+ bool FormatZeroOrNull(const wchar_t* what,
+ const WideString& wsPattern,
+ WideString* wsOutput) const;
UnownedPtr<LocaleMgrIface> const m_pLocaleMgr;
};
