summaryrefslogtreecommitdiff
path: root/sal/qa
AgeCommit message (Collapse)Author
2021-03-09Improve unit test accuracyMike Kaganski
I didn't take clock resolution into account when created the test, and it failed for me occasionally because the value was slightly less than expected. The typical system tick resolution is documented at https://docs.microsoft.com/en-us/windows-hardware/drivers/kernel/high-resolution-timers Change-Id: Ie48b10d15b14f9ac7d292a2cc9916bcbfff44b6f Reviewed-on: https://gerrit.libreoffice.org/c/core/+/111946 Tested-by: Mike Kaganski <mike.kaganski@collabora.com> Reviewed-by: Mike Kaganski <mike.kaganski@collabora.com> (cherry picked from commit eef43192d4c7b2867638c54a2ac31adfc26476c7) Reviewed-on: https://gerrit.libreoffice.org/c/core/+/112076 Tested-by: Jenkins Reviewed-by: Xisco Fauli <xiscofauli@libreoffice.org>
2021-03-09tdf#116983 tdf#136175: retry if failedMike Kaganski
Debugging the test case from the latter bug report shows that indeed the call to OleGetClipboard may fail first time, as jasonkres had suspected in the former bug. So follow the suggestion in tdf#116983, and retry the failing calls several times in case of failure. Many thanks to Telesto for preparing a clear bug report with reliable test case. Co-authored-by: jasonkres Change-Id: Ib3c497da830bc5faac586bcfe1eededa54bfa117 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/111825 Tested-by: Jenkins Reviewed-by: Mike Kaganski <mike.kaganski@collabora.com> (cherry picked from commit cf1c835e8016f8f1eefea6d625a913c0ac343a63) Reviewed-on: https://gerrit.libreoffice.org/c/core/+/112075 Reviewed-by: Xisco Fauli <xiscofauli@libreoffice.org>
2021-02-09report what value was found in messageCaolán McNamara
Change-Id: I512cedcd46f829b97b62a57d90d5a4a81d024d66 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/110598 Tested-by: Jenkins Reviewed-by: Michael Stahl <michael.stahl@allotropia.de>
2020-11-21Relax non-null requirement for some rtl_uString_* functionsStephan Bergmann
...that take a pointer and a length, and where it should be OK that the pointer is null if the length is zero. Those rtl_uString_* functions are targets of OUString member functions that take std::[u16]string_view arguments, and 19926ed35ebb623fc896942b1f232b83edf1fc1e "loplugin:stringview: Flag empty string converted to string view" (which changed some call sites to pass in default- constructed std::[u16]string_view, for which data() returns null) revealed that those rtl_uString_* functions were not prepared for such input. (The guardings of memcpy are necessary because memcpy still requires its pointer arguments to be non-null, even if the corresponding length is zero.) The new sal/qa/rtl/strings/test_strings_defaultstringview.cxx systematically tests all O[U]String[Buffer] member functions taking std::[u16]string_view arguments. It revealed one further issue in IMPL_RTL_STRNAME(compare_WithLength), where UBSan reported a nullptr-with-nonzero-offset > sal/rtl/strtmpl.cxx:149:9: runtime error: applying non-zero offset 18446744073709551614 to null pointer Also, rtl_uString_newReplaceFirstUtf16LUtf16L was found to lack a check for its `from` argument to be non-null. Change-Id: I6a7a712570f7d1e8d52097208c8a43a5a24797af Reviewed-on: https://gerrit.libreoffice.org/c/core/+/106295 Tested-by: Jenkins Reviewed-by: Stephan Bergmann <sbergman@redhat.com>
2020-11-21tdf#123936 Formatting files in module sal with clang-formatPhilipp Hofer
Change-Id: I04a773e8fd565f57dc0eb887fb4714b6edbb35e0 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/105699 Reviewed-by: Christian Lohmaier <lohmaier+LibreOffice@googlemail.com> Tested-by: Jenkins
2020-11-11loplugin:stringviewNoel
Add new methods "subView" to O(U)String to return substring views of the underlying data. Add a clang plugin to warn when replacing existing calls to copy() would be better to use subView(). Change-Id: I03a5732431ce60808946f2ce2c923b22845689ca Reviewed-on: https://gerrit.libreoffice.org/c/core/+/105420 Tested-by: Jenkins Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
2020-11-11disable O(U)String::concat for internal codeNoel Grandin
in favour of the more widely used, and better optimised, operator+ Change-Id: I6a1b37e0f3d253af1f7a0892443f59b620efea63 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/105523 Tested-by: Jenkins Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
2020-10-28Extend loplugin:elidestringvar to OStringStephan Bergmann
(In VisitVarDecl, filtering out AbstractConditionalOperator avoids an unhelpful > ~/lo/core/vcl/source/pdf/XmpMetadata.cxx:63:32: error: replace single use of literal 'rtl::OString' variable with a literal [loplugin:elidestringvar] > aXmlWriter.content(sPdfConformance); > ^~~~~~~~~~~~~~~ > ~/lo/core/vcl/source/pdf/XmpMetadata.cxx:52:21: note: literal 'rtl::OString' variable defined here [loplugin:elidestringvar] > OString sPdfConformance = (mnPDF_A == 1) ? "A" : "B"; > ~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ) Change-Id: I7d0410f04827d79b4b526752917c37d33cad2671 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/104911 Tested-by: Jenkins Reviewed-by: Stephan Bergmann <sbergman@redhat.com>
2020-10-05loplugin:reducevarscope in salNoel
Change-Id: I2ce95de07b8e0952a1e778e65940b30597396aa6 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/103949 Tested-by: Jenkins Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
2020-09-22OUStringLiteral/OStringLiteral coverity PARSE_ERROR workaroundCaolán McNamara
do more like commit 121771e37f7e2de41cd5643475861062bf25627b Date: Mon Sep 21 09:17:54 2020 +0200 Make some OUStringLiteral vars constexpr cause coverity can live with that Change-Id: I9efd7f848289c4865997a44c6780373068422227 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/103147 Tested-by: Jenkins Reviewed-by: Caolán McNamara <caolanm@redhat.com>
2020-09-16Turn OUStringLiteral into a consteval'ed, static-refcound rtl_uStringStephan Bergmann
...from which an OUString can cheaply be instantiated. This is the OUString equivalent of 4b9e440c51be3e40326bc90c33ae69885bfb51e4 "Turn OStringLiteral into a consteval'ed, static-refcound rtl_String". Most remarks about that commit apply here too (this commit is just substantially bigger and a bit more complicated because there were so much more uses of OUStringLiteral than of OStringLiteral): The one downside is that OUStringLiteral now needs to be a template abstracting over the string length. But any uses for which that is a problem (e.g., as the element type of a container that would no longer be homogeneous, or in the signature of a function that shall not be turned into a template for one reason or another) can be replaced with std::u16string_view, without loss of efficiency compared to the original OUStringLiteral, and without loss of expressivity. The new OUStringLiteral ctor code would probably not be very efficient if it were ever executed at runtime, but it is intended to be only executed at compile time. Where available, C++20 "consteval" is used to statically ensure that. The intended use of the new OUStringLiteral is in all cases where an object that shall itself not be an OUString (e.g., because it shall be a global static variable for which the OUString ctor/dtor would be detrimental at library load/unload) must be converted to an OUString instance in at least one place. Other string literal abstractions could use std::u16string_view (or just plain char16_t const[N]), but interestingly OUStringLiteral might be more efficient than constexpr std::u16string_view even for such cases, as it should not need any relocations at library load time. For now, no existing uses of OUStringLiteral have been changed to some other abstraction (unless technically necessary as discussed above), and no additional places that would benefit from OUStringLiteral have been changed to use it. Global constexpr OUStringLiteral variables defined in an included file would be somewhat suboptimal, as each translation unit that uses them would create its own, unshared instance. The envisioned solution is to turn them into static data members of some class (and there may be a loplugin coming to find and fix affected places). Another approach that has been taken here in a few cases where such variables were only used in one .cxx anyway is to move their definitions from the .hxx into that one .cxx (in turn causing some files to become empty and get removed completely)---which also silenced some GCC -Werror=unused-variable if a variable from a .hxx was not used in some .cxx including it. To keep individual commits reasonably manageable, some consumers of OUStringLiteral in rtl/ustrbuf.hxx and rtl/ustring.hxx are left in a somewhat odd state for now, where they don't take advantage of OUStringLiteral's equivalence to rtl_uString, but just keep extracting its contents and copy it elsewhere. In follow-up commits, those consumers should be changed appropriately, making them treat OUStringLiteral like an rtl_uString or dropping the OUStringLiteral overload in favor of an existing (and cheap to use now) OUString overload, etc. In a similar vein, comparison operators between OUString and std::u16string_view have been added to the existing plethora of comparison operator overloads. It would be nice to eventually consolidate them, esp. with the overloads taking OUStringLiteral and/or char16_t const[N] string literals, but that appears tricky to get right without introducing new ambiguities. Also, a handful of places across the code base use comparisons between OUString and OUStringNumber, which are now ambiguous (converting the OUStringNumber to either OUString or std::u16string_view). For simplicity, those few places have manually been fixed for now by adding explicit conversion to std::u16string_view. Also some compilerplugins code needed to be adapted, and some of the compilerplugins/test cases have become irrelevant (and have been removed), as the tested code would no longer compile in the first place. sal/qa/rtl/strings/test_oustring_concat.cxx documents a workaround for GCC bug <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96878> "Failed class template argument deduction in unevaluated, parenthesized context". That place, as well as uses of OUStringLiteral in extensions/source/abpilot/fieldmappingimpl.cxx and i18npool/source/localedata/localedata.cxx, which have been replaced with OUString::Concat (and which is arguably a better choice, anyway), also caused failures with at least Clang 5.0.2 (but would not have caused failures with at least recent Clang 12 trunk, so appear to be bugs in Clang that have meanwhile been fixed). Change-Id: I34174462a28f2000cfeb2d219ffd533a767920b8 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/102222 Tested-by: Jenkins Reviewed-by: Stephan Bergmann <sbergman@redhat.com>
2020-09-08Fix typos in commentsStephan Bergmann
Change-Id: I8a8108ae970613559a68d996dddcee485ddc052f Reviewed-on: https://gerrit.libreoffice.org/c/core/+/102235 Tested-by: Jenkins Reviewed-by: Stephan Bergmann <sbergman@redhat.com>
2020-09-07Make the OUString ctors taking raw sal_Unicode pointer/non-const array explicitStephan Bergmann
...and in turn add OUString::operator = and OUString::operator += overloads that take a std::u16string_view. Without making the ctors explicit, the operator overloads would have caused ambiguities when called with raw sal_Unicode pointers/non-const arrays, as those can convert to both OUString and to std::u16string_view. But the std::u16string_view operator overloads will generally be useful when changing OUStringLiteral similarly to 4b9e440c51be3e40326bc90c33ae69885bfb51e4 "Turn OStringLiteral into a consteval'ed, static-refcound rtl_String", at which point many existing uses of OUStringLiteral will be replaced with uses of std::u16string_view. Implementing this change turned up a need for an operator = overload for OUStringNumber, which has thus been added. No such need turned up for a corresponding operator += overload, but which can easily be added when the need arises. It also revealed that the operator == overloads between an OUString and a raw sal_Unicode pointer/non-const array were implemented rather inefficiently, creating a temporary OUString from the raw argument. Those have been improved. Preceding commits have already taken care of many dubious or simply unnecessary implicit uses of the now-explicit OUString ctors. This commit makes explicit the few remaining reasonable uses. (And in some cases needed to change variable initialization syntax from using parentheses to using curly braces, to avoid the most vexing parse issue. And needed to explicitly add OUString ctors from char16 const[2] string literal lvalues in a conditional expression in writerfilter/source/ooxml/OOXMLFastContextHandler.cxx that are only necessary because MSVC apparently still insists on doing array-to-pointer decay there.) All of this only affects LIBO_INTERNAL_ONLY. Change-Id: I7ce31162e9be1c3ff3c0bd184a34b535ec56be9e Reviewed-on: https://gerrit.libreoffice.org/c/core/+/102098 Tested-by: Jenkins Reviewed-by: Stephan Bergmann <sbergman@redhat.com>
2020-09-05Make OUString(char16_t const[N]) ctor check for embedded NULsStephan Bergmann
...and fix the detected fallout. That ctor only started to get used recently with a1570b6052ae9c9349282027c9007b071589bce6 "Make the OUString ConstCharArrayDetector::TypeUtf16 overloads are actually used", but it turns out that that also gave rise to that ctor being picked in error. To better guard against such erroneous uses, make that ctor assert that the given array does not contain embedded NUL characters, see the new sal/qa/rtl/strings/nonconstarray.cxx tests. The one place where that assert would fire during `make check` is fixed now in SwWW8ImplReader::ImportDopTypography. That assert would also fire for tow OUStringLiteral-related tests in the recently added test::oustring::StringLiterals::checkEmbeddedNul, so drop those for how. They cna presumably be added back (with reversed logic values) when OUStringLiteral is changed similarly to how OStringLiteral was changed in 4b9e440c51be3e40326bc90c33ae69885bfb51e4 "Turn OStringLiteral into a consteval'ed, static-refcound rtl_String". Change-Id: I6056244003a20f77ba0d953538d25edcbd562211 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/102063 Tested-by: Jenkins Reviewed-by: Stephan Bergmann <sbergman@redhat.com>
2020-09-04Make many OUString functions take std::u16string_view parametersStephan Bergmann
...instead of having individual overloads for OUString, OUStringLiteral, and literal char16_t const[N]. (The variants taking OUString are still needed for !LIBO_INTERNAL_ONLY, though. The variants taking ASCII-only literal char const[N] are also left in place.) This nicely reduces the number of needed overloads. std::u16string_view allows to pass as arguments: * OUString * OUStringLiteral * OUStringChar (with the necessary conversion added now) * OUStringNumber * u"..." char16_t string literals * u"..."sv std::u16string_view literals * std::u16string, plain char16_t*, and more A notable exceptions is OUStringConcat, which now needs to be wrapped in OUString(...), see the handful of places that needed to be adapted. One caveat is the treatment of embedded NUL characters, as std::u16string_view(u"x\0y") constructs a view of size 1, while only u"x\0y"sv constructs a view of size 3 (which matches the old behavior of overloads for literal char16_t const[N] via the ConstCharArrayDetector<>::TypeUtf16 machinery). See the new checkEmbeddedNul in sal/qa/rtl/strings/test_oustring_stringliterals.cxx. The functions that have been changed are generally those that: * already take a string of determined length, so that using std::u16string_view, which is always constructed with a determined length, is no pessimization (e.g., there are operator == overloads taking plain pointers, which do not need to determine the string length upfront); * could not benefit from the fact that the passed-in argument is an OUString (e.g., the corresponding operator = overload can reuse the passed-in OUString's rtl_uString pData member); * do not run into overload resolution ambiguity issues, like the comparison operators would do. One inconsistency that showed up is that while the original replaceAll(OUString const &, OUString const &, sal_Int32 fromIndex = 0) overload takes an optional third fromIndex argument, the existing replaceAll overloads taking OUStringLiteral and literal char16_t const[N] arguments did not. Fixing that required a new (LIBO_INTERNAL_ONLY) rtl_uString_newReplaceAllFromIndexUtf16LUtf16L (with test code in sal/qa/rtl/strings/test_strings_replace.cxx). Another issue was posed by test code in sal/qa/rtl/strings/test_oustring_stringliterals.cxx that used the RTL_STRING_UNITTEST-only OUString(Except*CharArrayDetector) ctors to verify that certain function calls should not compile (and would compile under RTL_STRING_UNITTEST by taking those Except*CharArrayDetector converted to OUString as arguments). Those problematic "should fail to compile" tests have been converted into a new CompilerTest_sal_rtl_oustring. Change-Id: Id72e8c4cc338258cadad00ddc6ea5b9da2e1f780 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/102020 Tested-by: Jenkins Reviewed-by: Stephan Bergmann <sbergman@redhat.com>
2020-09-03Fix typoStephan Bergmann
...introduced with b5ad72bbfca85946e352b56d9d2ee5eb71cd2132 "Replace VALID_CONVERSION macro with function". (VALID_CONVERSION of a lambda was trivially false, as neither OUString nor OUStringBuffer have a ctor taking a lambda, anyway.) Change-Id: I1d4f2de0e1b1973d8680f53e03dbbb3d939fcc03 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/101982 Tested-by: Jenkins Reviewed-by: Stephan Bergmann <sbergman@redhat.com>
2020-09-02Turn OStringLiteral into a consteval'ed, static-refcound rtl_StringStephan Bergmann
...from which an OString can cheaply be instantiated. The one downside is that OStringLiteral now needs to be a template abstracting over the string length. But any uses for which that is a problem (e.g., as the element type of a containers that would no longer be homogeneous, or in the signature of a function that shall not be turned into a template for one reason or another) can be replaced with std::string_view, without loss of efficiency compared to the original OStringLiteral, and without loss of expressivity (esp. with the newly introduced OString(std::string_view) ctor). The new OStringLiteral ctor code would probably not be very efficient if it were ever executed at runtime, but it is intended to be only executed at compile time. Where available, C++20 "consteval" is used to statically ensure that. The intended use of the new OStringLiteral is in all cases where an object that shall itself not be an OString (e.g., because it shall be a global static variable for which the OString ctor/dtor would be detrimental at library load/unload) must be converted to an OString instance in at least one place. Other string literal abstractions could use std::string_view (or just plain char const[N]), but interestingly OStringLiteral might be more efficient than constexpr std::string_view even for such cases, as it should not need any relocations at library load time. For now, no existing uses of OUStringLiteral have been changed to some other abstraction (unless technically necessary as discussed above), and no additional places that would benefit from OUStringLiteral have been changed to use it. sal/qa/rtl/strings/test_ostring_concat.cxx documents some workarounds for GCC bug <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96878> "Failed class template argument deduction in unevaluated, parenthesized context". Those places, as well as uses of OStringLiteral in incodemaker/source/javamaker/javaoptions.cxx and i18npool/source/breakiterator/breakiterator_unicode.cxx, which have been replaced with OString::Concat (and which is arguably a better choice, anyway), also caused failures with at least Clang 5.0.2 (but would not have caused failures with at least recent Clang 12 trunk, so appear to be bugs in Clang that have meanwhile been fixed). This change also revealed a bug in at least recent Clang 12 trunk CastExpr::getSubExprAsWritten (still to be reported to LLVM), triggered at least in some calls from loplugin code (for which it can be fixed for now in the existing compat::getSubStringAsWritten). A similar commit for OUStringLiteral is planned, too. Change-Id: Ib192f4ed4c44769512a16364cb55c25627bae6f4 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/101814 Tested-by: Jenkins Reviewed-by: Stephan Bergmann <sbergman@redhat.com>
2020-08-30Goodbye O[U]StringView, welcome O[U]String::ConcatStephan Bergmann
O[U]StringView had an odd mixture of uses. For one, it was used like std::[u16]string_view, for which directly using the latter std types is clearly the better alternative. For another, it was used in concatenation sequences, when neither of the two leading terms were of our rtl string-related types. For that second use case introduce O[U]String::Concat (as std::[u16]string_view can obviously not be used, those not being one of our rtl string-related types). Also, O[U]StringLiteral is occasionally used for this, but the planned changes outlined in the 33ecd0d5c4fff9511a8436513936a3f7044a775a "Change OUStringLiteral from char[] to char16_t[]" commit message will make that no longer work, so O[U]String::Concat will be the preferred solution in such use cases going forward, too. O[U]StringView was also occasionally used to include O[U]StringBuffer values in concatenation sequences, for which a more obvious alternative is to make O[U]StringBuffer participate directly in the ToStringHelper/O[U]StringConcat machinery. Change-Id: I1f0e8d836796c9ae01c45f32c518be5f52976622 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/101586 Tested-by: Jenkins Reviewed-by: Stephan Bergmann <sbergman@redhat.com>
2020-08-29Fix typosAndrea Gelmini
Change-Id: I473956d570feac508e52a3e52cc26cc154f4dc56 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/101627 Reviewed-by: Julien Nabet <serval2412@yahoo.fr> Tested-by: Jenkins
2020-08-28Change OUStringLiteral from char[] to char16_t[]Stephan Bergmann
This is a prerequisite for making conversion from OUStringLiteral to OUString more efficient at least for C++20 (by replacing its internals with a constexpr- generated sal_uString-compatible layout with a SAL_STRING_STATIC_FLAG refCount, conditionally for C++20 for now). For a configure-wise bare-bones build on Linux, size reported by `du -bs instdir` grew by 118792 bytes from 1155636636 to 1155755428. In most places just a u"..." string literal prefix had to be added. In some places char const a[] = "..."; variables have been changed to char16_t, and a few places required even further changes to code (which prompted the addition of include/o3tl/string_view.hxx helper function o3tl::equalsIgnoreAsciiCase and the additional OUString::createFromAscii overload). For all uses of macros expanding to string literals, the relevant uses have been rewritten as u"" MACRO instead of changing the macro definitions. It should be possible to change at least some of those macro definitions (and drop the u"" from their call sites) in follow-up commits. Change-Id: Iec4ef1a057d412d22443312d40c6a8a290dc6144 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/101483 Tested-by: Jenkins Reviewed-by: Stephan Bergmann <sbergman@redhat.com>
2020-08-13loplugin:stringstatic also look for local staticsNoel Grandin
Add some API to O*StringLiteral, to make it easier to use in some places that were using O*String Change-Id: I1fb93bd47ac2065c9220d509aad3f4320326d99e Reviewed-on: https://gerrit.libreoffice.org/c/core/+/100270 Tested-by: Jenkins Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
2020-08-04loplugin:elidestringvar (clang-cl)Stephan Bergmann
...plus ensuing loplugin:unnecessaryparen in vcl/source/treelist/transfer.cxx Change-Id: I1abf2e0c589c2c124399c1337f5dd703ee7d04a9 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/100094 Tested-by: Jenkins Reviewed-by: Stephan Bergmann <sbergman@redhat.com>
2020-07-02Upcoming improved loplugin:staticanonymous -> redundantstatic: salStephan Bergmann
Change-Id: I022f5ed37d25f2c8a8870033bab32ff59d4d8da6 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/97648 Tested-by: Jenkins Reviewed-by: Stephan Bergmann <sbergman@redhat.com>
2020-06-05Upcoming loplugin:elidestringvar: salStephan Bergmann
Change-Id: Ia277fc825d9687fc70d35bc7db92df5cd5d9b2bf Reviewed-on: https://gerrit.libreoffice.org/c/core/+/95529 Tested-by: Jenkins Reviewed-by: Stephan Bergmann <sbergman@redhat.com>
2020-03-14tdf#130975 replace `rtl::math::isNan` with `std::isnan`.Yukio Siraichi
Change-Id: I5d53e6369d35093445b2efd8936bbf8c6775ff47 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/90451 Tested-by: Jenkins Reviewed-by: Mike Kaganski <mike.kaganski@collabora.com>
2020-03-14tdf#130974 replace `rtl::math::isSignBitSet` with `std::signbit`.Yukio Siraichi
Change-Id: I91235eee8c6a9d4a59c1933527b49141f64cd91b Reviewed-on: https://gerrit.libreoffice.org/c/core/+/90478 Tested-by: Jenkins Reviewed-by: Mike Kaganski <mike.kaganski@collabora.com>
2020-03-12tdf#130976 replace `rtl::math::isInf` with `std::inf`.Yukio Siraichi
Change-Id: Id0f0e07b324230d2d69cbf4ab07b0fff5b24474d Reviewed-on: https://gerrit.libreoffice.org/c/core/+/90377 Tested-by: Jenkins Reviewed-by: Mike Kaganski <mike.kaganski@collabora.com>
2020-03-12Revert "loplugin:constfields in reportdesign,sal,sax"Noel Grandin
This reverts commit d4d37662b090cb237585156a47cd8e1f1cbe2656. Now that we know that making fields has negative side effects like disabling assignment operator generation. Change-Id: Idef4937b89a83d2efbfaf0ab87d059a0143c0164 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/90364 Tested-by: Jenkins Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
2020-03-11Get rid of some SAL_MATH_FINITE useStephan Bergmann
Change-Id: I850efb6215c95d356d42e19e639c578c377d1507 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/90316 Tested-by: Jenkins Reviewed-by: Stephan Bergmann <sbergman@redhat.com>
2020-03-04ofz#21036: Avoid UBSan signed-integer-overflow in external/dtoaStephan Bergmann
> workdir/UnpackedTarball/dtoa/src/dtoa.c:3624:12: runtime error: signed integer overflow: 10 * 858993459 cannot be represented in type 'int' > #0 in strtod_nolocale at workdir/UnpackedTarball/dtoa/src/dtoa.c:3624:12 (instdir/program/libuno_sal.so.3 +0x55286d) > #1 in double (anonymous namespace)::stringToDouble<char16_t>(char16_t const*, char16_t const*, char16_t, char16_t, rtl_math_ConversionStatus*, char16_t const**) at sal/rtl/math.cxx:976:20 (instdir/program/libuno_sal.so.3 +0x3b5f0e) > #2 in rtl_math_uStringToDouble at sal/rtl/math.cxx:1028:12 (instdir/program/libuno_sal.so.3 +0x3b1714) [...] Change-Id: If24fca1fb4829ddd763c9920a1af9a90dc2b138c Reviewed-on: https://gerrit.libreoffice.org/c/core/+/89966 Tested-by: Jenkins Reviewed-by: Stephan Bergmann <sbergman@redhat.com>
2020-02-27Related: tdf#130725: use strtod also in rtl::math::stringToDoubleMike Kaganski
Size of buffer on stack is 256 characters. Logging function usage in make check, of >1 100 000 invocations, the longest string was 80 characters, average being 4.6 characters. So heap allocation is unlikely in scenarios with intensive function usage. Several existing unit tests had to be fixed. Usually, the change is either minimal or getting closer to what Excel returns (for Calc tests). But in case of AMORDEGRC, I had to change rate value passed to the function from 0.3 to 0.31. It's because the closest double value for 0.3 is 0.29999999999999999, which is a bit less than 0.3; multiplied by 1.5, this gives 0.44999999999999996, and then rounding the result of multiplication of the latter by cost gave the result 1 less than before, when 0.3 was imported as 0.30000000000000004. Now the function returns a value 1 less than Excel for that set of arguments. I don't see how to fix that. Having rate slightly different gives consistent result between Calc and Excel. Change-Id: Icae5ce374fe0c31a1aa10cee815e65ef0014f382 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/89422 Tested-by: Jenkins Reviewed-by: Mike Kaganski <mike.kaganski@collabora.com>
2020-02-12clang-analyzer-deadcode.DeadStoresNoel Grandin
Change-Id: Ifa384933569b27d0d08eb479bb95b799163ae386 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/88450 Tested-by: Jenkins Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
2020-01-31clang-tidy modernize-concat-nested-namespaceNoel Grandin
Change-Id: Iab35a8b85b3ba1df791c774f40b037f9420a071a Reviewed-on: https://gerrit.libreoffice.org/c/core/+/86708 Tested-by: Jenkins Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
2020-01-26cppcheck: shadowFunction in sal/osl_FileJulien Nabet
Change-Id: I7d264faad080428459e499c786f25b3e06a6700b Reviewed-on: https://gerrit.libreoffice.org/c/core/+/87434 Tested-by: Jenkins Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
2020-01-18Removed redundant semicolonAndrea Gelmini
Change-Id: Ice0c5f413f176ce6eb832dc8b06010595f214857 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/86582 Tested-by: Jenkins Reviewed-by: Stephan Bergmann <sbergman@redhat.com>
2019-12-27Fix typoAndrea Gelmini
Change-Id: Ibd3ece30c6cbb9582904adc5e329f54d41cff33b Reviewed-on: https://gerrit.libreoffice.org/c/core/+/85872 Reviewed-by: Julien Nabet <serval2412@yahoo.fr> Tested-by: Julien Nabet <serval2412@yahoo.fr>
2019-12-27sal_osl_security: Windows: Handle cases with same hostname and usernameMike Kaganski
... as seen in [build CUT] sal_osl_security #Initializing ... # #logonUser function need root/Administrator account to test. #You can test by login with root/Administrator, and execute: #testshl2 -forward "username password" ../../../wntmsci9/bin/Security.dll # where username and password are forwarded account info. #if no text forwarded, this function will be skipped. # #Retrieved system information is below: Computer Name: SOMENAME Current User Name: Somename Current User Home Directory:file:///C:/Users/Somename/Documents Current Config Directory: file:///C:/Users/Somename/AppData/Roaming Current UserID: S-1-5-21-1234567890-123456789-123456789 Current User is: NOT Administrator. # #Initialization Done. osl_Security::ctors::ctors_001 finished in: 0ms osl_Security::UserProfile::loadUserProfile finished in: 0ms osl_Security::UserProfile::unloadUserProfile finished in: 0ms osl_Security::getHandle::getHandle_001 finished in: 0ms osl_Security::loginUserOnFileServer::loginUserOnFileServer_001 finished in: 3ms osl_Security::getConfigDir::getConfigDir_001 finished in: 1ms C:/cygwin/home/Somename/lode/dev/core/sal/qa/osl/security/osl_Security.cxx:139:osl_Security::getUserIdent::getUserIdent_001 equality assertion failed - Expected: S-1-5-21-1234567890-123456789-123456789 - Actual : S-1-5-21-1234567890-123456789-123456789-1001 - strUserID: S-1-5-21-1234567890-123456789-123456789, strID: S-1-5-21-1234567890-123456789-123456789-1001, bRes: true osl_Security::getUserIdent::getUserIdent_001 finished in: 0ms osl_Security::getUserName::getUserName_001 finished in: 1ms osl_Security::isAdministrator::isAdministrator_001 finished in: 0ms C:/cygwin/home/Somename/lode/dev/core/sal/qa/osl/security/osl_Security.cxx(139) : error : Assertion Test name: osl_Security::getUserIdent::getUserIdent_001 equality assertion failed - Expected: S-1-5-21-1234567890-123456789-123456789 - Actual : S-1-5-21-1234567890-123456789-123456789-1001 - strUserID: S-1-5-21-1234567890-123456789-123456789, strID: S-1-5-21-1234567890-123456789-123456789-1001, bRes: true Failures !!! Run: 9 Failure total: 1 Failures: 1 Errors: 0 The problem here is that passing a string equal to hostname (case-insensitive) to LookupAccountNameW without domain qualifier returns data for local system domain, not for user with the same name. So let's try again, this time with fully-qualified user name including local domain part. Change-Id: I15f69c01dddf15782bd11a6ed6678f0a02d79786 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/85859 Tested-by: Jenkins Reviewed-by: Mike Kaganski <mike.kaganski@collabora.com>
2019-12-19sal_Char->char in remotebridges..saxNoel Grandin
Change-Id: I6d32942960a5e997f16eb1301c45495661cd4cea Reviewed-on: https://gerrit.libreoffice.org/85514 Tested-by: Jenkins Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
2019-12-05loplugin:external (clang-cl)Stephan Bergmann
...plus loplugin:consttobool and loplugin:fakebool fallout Change-Id: Ie3d8121815c080b13bea6d9deca1eb138ca56138 Reviewed-on: https://gerrit.libreoffice.org/84515 Tested-by: Jenkins Reviewed-by: Stephan Bergmann <sbergman@redhat.com>
2019-12-05Fix various clang-cl and loplugin warnings in sal/qa/osl/file/osl_File.cxxStephan Bergmann
...after that file got re-enabled for Windows builds recently Change-Id: I67c9e1676344d3754293595ec5a3c6093c0959c1 Reviewed-on: https://gerrit.libreoffice.org/84510 Tested-by: Jenkins Reviewed-by: Stephan Bergmann <sbergman@redhat.com>
2019-12-03Adapt CPPUNIT_ASSERT to C++20 deleted ostream << for sal_Unicode (aka char16_t)Stephan Bergmann
<http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2019/p1423r3.html> "char8_t backward compatibility remediation", as implemented now by <https://gcc.gnu.org/ git/?p=gcc.git;a=commit;h=0c5b35933e5b150df0ab487efb2f11ef5685f713> "libstdc++: P1423R3 char8_t remediation (2/4)" for -std=c++2a, deletes operator << overloads that would print an integer rather than a (presumably expected) character. But for simplicity (and to avoid issues with non-printing characters), keep printing an integer here. Change-Id: I751b99ee32d418eb488131ffa130d6f7d6d38dc7 Reviewed-on: https://gerrit.libreoffice.org/84348 Tested-by: Jenkins Reviewed-by: Stephan Bergmann <sbergman@redhat.com>
2019-12-03WIN map ERROR_DIRECTORY to osl_File_E_NOTDIRJan-Marek Glogowski
The open_004 unit test fails on Windows with E_NOENT. I think the assert is correct, and it's better change the ERROR_DIRECTORY mapping, as FindFirstFileW returns ERROR_PATH_NOT_FOUND, if the path doesn't exists at all. The description of that error code in the MS API docs is "The directory name is invalid.". And obviously there is no way to tell, if this is actually a better mapping. Change-Id: Id122ce39e90f3562e70c96a06752e5b32ed55b41 Reviewed-on: https://gerrit.libreoffice.org/84070 Tested-by: Jenkins Reviewed-by: Mike Kaganski <mike.kaganski@collabora.com> Reviewed-by: Jan-Marek Glogowski <glogow@fbihome.de>
2019-12-03WIN enable osl_File.cxx part of CppUnitTest_sal_oslJan-Marek Glogowski
This just disables open::open_004 test on Windows, as this returns E_NOENT on Windows, but I think the assert is correct. There is also test-getsystempathfromfileurl.cxx, which seems to overlap? The test now uses osl::FileBase::getTempDirURL to find the system temporary directory, insted of hardcoded defaults. This also includes reverts the following commits: * "remove unused Exist from oslCheckMode enum" commit 4ca9e8fa3a040c18221c947a651469a89029282e. * "CppunitTest_sal_osl_file fails on Windows" commit 4603028bc4e2cdacd32da073a82a42ba1fc549a8 Change-Id: Ib7fc4c13f8159a240d4e20c26f61d84204e4261b Reviewed-on: https://gerrit.libreoffice.org/83901 Tested-by: Jenkins Reviewed-by: Jan-Marek Glogowski <glogow@fbihome.de>
2019-11-29WIN enable NoAuthority testJan-Marek Glogowski
Change-Id: I5e8214efbf32a5bc43f002ef244826b9b899c692 Reviewed-on: https://gerrit.libreoffice.org/84071 Tested-by: Jenkins Reviewed-by: Jan-Marek Glogowski <glogow@fbihome.de>
2019-11-27Fix some checkUNXBehaviour_getSystemPathFromFileURL testsStephan Bergmann
Change-Id: I362d277d3ad2b2604107ddb6928c76ebec94efc3 Reviewed-on: https://gerrit.libreoffice.org/83868 Tested-by: Jenkins Reviewed-by: Stephan Bergmann <sbergman@redhat.com>
2019-11-24cppcheck: performing init in init list (oox, package, sal, sax)Julien Nabet
Change-Id: I77e28f02516ab25ff6680970dc13d52cfa90eac1 Reviewed-on: https://gerrit.libreoffice.org/83620 Tested-by: Jenkins Reviewed-by: Julien Nabet <serval2412@yahoo.fr>
2019-11-22Extend loplugin:external to warn about classesStephan Bergmann
...following up on 314f15bff08b76bf96acf99141776ef64d2f1355 "Extend loplugin:external to warn about enums". Cases where free functions were moved into an unnamed namespace along with a class, to not break ADL, are in: filter/source/svg/svgexport.cxx sc/source/filter/excel/xelink.cxx sc/source/filter/excel/xilink.cxx svx/source/sdr/contact/viewobjectcontactofunocontrol.cxx All other free functions mentioning moved classes appear to be harmless and not give rise to (silent, even) ADL breakage. (One remaining TODO in compilerplugins/clang/external.cxx is that derived classes are not covered by computeAffectedTypes, even though they could also be affected by ADL-breakage--- but don't seem to be in any acutal case across the code base.) For friend declarations using elaborate type specifiers, like class C1 {}; class C2 { friend class C1; }; * If C2 (but not C1) is moved into an unnamed namespace, the friend declaration must be changed to not use an elaborate type specifier (i.e., "friend C1;"; see C++17 [namespace.memdef]/3: "If the name in a friend declaration is neither qualified nor a template-id and the declaration is a function or an elaborated-type-specifier, the lookup to determine whether the entity has been previously declared shall not consider any scopes outside the innermost enclosing namespace.") * If C1 (but not C2) is moved into an unnamed namespace, the friend declaration must be changed too, see <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71882> "elaborated-type-specifier friend not looked up in unnamed namespace". Apart from that, to keep changes simple and mostly mechanical (which should help avoid regressions), out-of-line definitions of class members have been left in the enclosing (named) namespace. But explicit specializations of class templates had to be moved into the unnamed namespace to appease <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92598> "explicit specialization of template from unnamed namespace using unqualified-id in enclosing namespace". Also, accompanying declarations (of e.g. typedefs or static variables) that could arguably be moved into the unnamed namespace too have been left alone. And in some cases, mention of affected types in blacklists in other loplugins needed to be adapted. And sc/qa/unit/mark_test.cxx uses a hack of including other .cxx, one of which is sc/source/core/data/segmenttree.cxx where e.g. ScFlatUInt16SegmentsImpl is not moved into an unnamed namespace (because it is declared in sc/inc/segmenttree.hxx), but its base ScFlatSegmentsImpl is. GCC warns about such combinations with enabled-by-default -Wsubobject-linkage, but "The compiler doesn’t give this warning for types defined in the main .C file, as those are unlikely to have multiple definitions." (<https://gcc.gnu.org/onlinedocs/gcc-9.2.0/gcc/Warning-Options.html>) The warned-about classes also don't have multiple definitions in the given test, so disable the warning when including the .cxx. Change-Id: Ib694094c0d8168be68f8fe90dfd0acbb66a3f1e4 Reviewed-on: https://gerrit.libreoffice.org/83239 Tested-by: Jenkins Reviewed-by: Stephan Bergmann <sbergman@redhat.com>
2019-11-17Extend loplugin:external to warn about enumsStephan Bergmann
To mitigate the dangers of silently breaking ADL when moving enums into unnamed namespaces (see the commit message of 206b5b2661be37efdff3c6aedb6f248c4636be79 "New loplugin:external"), note all functions that are affected. (The plan is to extend loplugin:external further to also warn about classes and class templates, and the code to identify affected functions already takes that into account, so some parts of that code are not actually relevant for enums.) But it appears that none of the functions that are actually affected by the changes in this commit relied on being found through ADL, so no adaptions were necessary for them. (clang::DeclContext::collectAllContexts is non-const, which recursively means that External's Visit... functions must take non-const Decl*. Which required compilerplugins/clang/sharedvisitor/analyzer.cxx to be generalized to support such Visit... functions with non-const Decl* parameters.) Change-Id: Ia215291402bf850d43defdab3cff4db5b270d1bd Reviewed-on: https://gerrit.libreoffice.org/83001 Tested-by: Jenkins Reviewed-by: Stephan Bergmann <sbergman@redhat.com>
2019-10-28loplugin:stringadd improve detectionNoel Grandin
if one side of the expression is a compile-time-constant, we don't need to worry about side-effects on the other side Change-Id: Iee71ea51b327ef244bf39f128f921ac325d74e2b Reviewed-on: https://gerrit.libreoffice.org/81589 Tested-by: Jenkins Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
2019-10-20Replace some uses of OUStringChar with string literalsStephan Bergmann
(At least MSVC++ 14.14, aka Visual Studio 2017 version 15.7, apparently requires `"\xDFFF"` to be written with a `u` prefix in the concatenated string literal u"\xD800" "\U000103FF" "\xDFFF" "A" to avoid "error C2022: '57343': too big for character", so prefix all the individual string literals in such concatenations, even if that should be redundant.) Change-Id: Ief69e6c7ae71fe2c4c9c56c38fab0bc782ceb82c Reviewed-on: https://gerrit.libreoffice.org/81142 Tested-by: Jenkins Reviewed-by: Stephan Bergmann <sbergman@redhat.com>