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