include/svl/sharedstringpool.hxx | 3 +
sc/qa/unit/ucalc.cxx | 29 ++++++++-------
svl/qa/unit/svl.cxx | 67 +++++++++++++++++++++++++----------
svl/source/misc/sharedstringpool.cxx | 3 +
4 files changed, 69 insertions(+), 33 deletions(-)
New commits:
commit 97715aacd8aa32bbd0fd4e0a3fa0bdb8e7fbeaa5
Author: Luboš Luňák <l.lu...@collabora.com>
AuthorDate: Thu Sep 22 10:34:04 2022 +0200
Commit: Caolán McNamara <caol...@redhat.com>
CommitDate: Wed Oct 5 09:52:25 2022 +0200
make sure SharedString::EMPTY_STRING is interned in pools (tdf#150647)
Without this, it may not actually be there, so interning "" would
use a different string instance, and then comparing with
SharedString::getEmptyString() would actually compare non-equal.
Change-Id: I22660f63aa321e3a8f72cfb96df1db56e08fbb84
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/140402
Tested-by: Jenkins
Reviewed-by: Eike Rathke <er...@redhat.com>
(cherry picked from commit e47e0cb0ad1dc3554e9b57f8562a217cf785edbf)
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/140497
Reviewed-by: Xisco Fauli <xiscofa...@libreoffice.org>
(cherry picked from commit 84dae476666863f3367308cf1c5f80d45a43c3ef)
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/140601
Reviewed-by: Caolán McNamara <caol...@redhat.com>
Tested-by: Caolán McNamara <caol...@redhat.com>
diff --git a/include/svl/sharedstringpool.hxx b/include/svl/sharedstringpool.hxx
index ff270eef5aa6..6880fec2a101 100644
--- a/include/svl/sharedstringpool.hxx
+++ b/include/svl/sharedstringpool.hxx
@@ -53,8 +53,9 @@ public:
*/
void purge();
+ // For unit tests. Note that an "empty" pool may contain some internal
items,
+ // such as SharedString::getEmptyString().
size_t getCount() const;
-
size_t getCountIgnoreCase() const;
};
}
diff --git a/sc/qa/unit/ucalc.cxx b/sc/qa/unit/ucalc.cxx
index db371c46ddc8..df937c50f2d3 100644
--- a/sc/qa/unit/ucalc.cxx
+++ b/sc/qa/unit/ucalc.cxx
@@ -380,6 +380,10 @@ void Test::testSharedStringPool()
{
m_pDoc->InsertTab(0, "foo");
+ svl::SharedStringPool& rPool = m_pDoc->GetSharedStringPool();
+ size_t extraCount = rPool.getCount(); // internal items such as
SharedString::getEmptyString()
+ size_t extraCountIgnoreCase = rPool.getCountIgnoreCase();
+
// Strings that are identical.
m_pDoc->SetString(ScAddress(0,0,0), "Andy"); // A1
m_pDoc->SetString(ScAddress(0,1,0), "Andy"); // A2
@@ -417,40 +421,39 @@ void Test::testSharedStringPool()
}
// Check the string counts after purging. Purging shouldn't remove any
strings in this case.
- svl::SharedStringPool& rPool = m_pDoc->GetSharedStringPool();
rPool.purge();
- CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(5), rPool.getCount());
- CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(2), rPool.getCountIgnoreCase());
+ CPPUNIT_ASSERT_EQUAL(5+extraCount, rPool.getCount());
+ CPPUNIT_ASSERT_EQUAL(2+extraCountIgnoreCase, rPool.getCountIgnoreCase());
// Clear A1 and purge again.
clearRange(m_pDoc, ScAddress(0,0,0));
rPool.purge();
- CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(5), rPool.getCount());
- CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(2), rPool.getCountIgnoreCase());
+ CPPUNIT_ASSERT_EQUAL(5+extraCount, rPool.getCount());
+ CPPUNIT_ASSERT_EQUAL(2+extraCountIgnoreCase, rPool.getCountIgnoreCase());
// Clear A2 and purge again.
clearRange(m_pDoc, ScAddress(0,1,0));
rPool.purge();
- CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(4), rPool.getCount());
- CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(2), rPool.getCountIgnoreCase());
+ CPPUNIT_ASSERT_EQUAL(4+extraCount, rPool.getCount());
+ CPPUNIT_ASSERT_EQUAL(2+extraCountIgnoreCase, rPool.getCountIgnoreCase());
// Clear A3 and purge again.
clearRange(m_pDoc, ScAddress(0,2,0));
rPool.purge();
- CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(3), rPool.getCount());
- CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(2), rPool.getCountIgnoreCase());
+ CPPUNIT_ASSERT_EQUAL(3+extraCount, rPool.getCount());
+ CPPUNIT_ASSERT_EQUAL(2+extraCountIgnoreCase, rPool.getCountIgnoreCase());
// Clear A4 and purge again.
clearRange(m_pDoc, ScAddress(0,3,0));
rPool.purge();
- CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(1), rPool.getCount());
- CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(1), rPool.getCountIgnoreCase());
+ CPPUNIT_ASSERT_EQUAL(1+extraCount, rPool.getCount());
+ CPPUNIT_ASSERT_EQUAL(1+extraCountIgnoreCase, rPool.getCountIgnoreCase());
// Clear A5 and the pool should be completely empty.
clearRange(m_pDoc, ScAddress(0,4,0));
rPool.purge();
- CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(0), rPool.getCount());
- CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(0), rPool.getCountIgnoreCase());
+ CPPUNIT_ASSERT_EQUAL(extraCount, rPool.getCount());
+ CPPUNIT_ASSERT_EQUAL(extraCountIgnoreCase, rPool.getCountIgnoreCase());
// Now, compare string and edit text cells.
m_pDoc->SetString(ScAddress(0,0,0), "Andy and Bruce"); // A1
diff --git a/svl/qa/unit/svl.cxx b/svl/qa/unit/svl.cxx
index f125305783c5..19a8c2b3baab 100644
--- a/svl/qa/unit/svl.cxx
+++ b/svl/qa/unit/svl.cxx
@@ -48,6 +48,15 @@ static std::ostream& operator<<(std::ostream& rStrm, const
Color& rColor)
return rStrm;
}
+namespace svl
+{
+static std::ostream& operator<<(std::ostream& rStrm, const SharedString&
string )
+{
+ return rStrm << "(" << static_cast<const void*>(string.getData()) << ")"
<< string.getString();
+}
+}
+
+
namespace {
class Test : public CppUnit::TestFixture {
@@ -62,6 +71,7 @@ public:
void testSharedStringPool();
void testSharedStringPoolPurge();
void testSharedStringPoolPurgeBug1();
+ void testSharedStringPoolEmptyString();
void testFdo60915();
void testI116701();
void testTdf103060();
@@ -80,6 +90,7 @@ public:
CPPUNIT_TEST(testSharedStringPool);
CPPUNIT_TEST(testSharedStringPoolPurge);
CPPUNIT_TEST(testSharedStringPoolPurgeBug1);
+ CPPUNIT_TEST(testSharedStringPoolEmptyString);
CPPUNIT_TEST(testFdo60915);
CPPUNIT_TEST(testI116701);
CPPUNIT_TEST(testTdf103060);
@@ -363,18 +374,21 @@ void Test::testSharedStringPoolPurge()
{
SvtSysLocale aSysLocale;
svl::SharedStringPool aPool(aSysLocale.GetCharClass());
+ size_t extraCount = aPool.getCount(); // internal items such as
SharedString::getEmptyString()
+ size_t extraCountIgnoreCase = aPool.getCountIgnoreCase();
+
aPool.intern("Andy");
aPool.intern("andy");
aPool.intern("ANDY");
- CPPUNIT_ASSERT_EQUAL_MESSAGE("Wrong string count.",
static_cast<size_t>(3), aPool.getCount());
- CPPUNIT_ASSERT_EQUAL_MESSAGE("Wrong case insensitive string count.",
static_cast<size_t>(1), aPool.getCountIgnoreCase());
+ CPPUNIT_ASSERT_EQUAL_MESSAGE("Wrong string count.", 3+extraCount,
aPool.getCount());
+ CPPUNIT_ASSERT_EQUAL_MESSAGE("Wrong case insensitive string count.",
1+extraCountIgnoreCase, aPool.getCountIgnoreCase());
// Since no string objects referencing the pooled strings exist, purging
- // the pool should empty it.
+ // the pool should empty it (except for internal items).
aPool.purge();
- CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(0), aPool.getCount());
- CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(0), aPool.getCountIgnoreCase());
+ CPPUNIT_ASSERT_EQUAL(extraCount, aPool.getCount());
+ CPPUNIT_ASSERT_EQUAL(extraCountIgnoreCase, aPool.getCountIgnoreCase());
// Now, create string objects using optional so we can clear them
std::optional<svl::SharedString> pStr1 = aPool.intern("Andy");
@@ -382,37 +396,37 @@ void Test::testSharedStringPoolPurge()
std::optional<svl::SharedString> pStr3 = aPool.intern("ANDY");
std::optional<svl::SharedString> pStr4 = aPool.intern("Bruce");
- CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(5), aPool.getCount());
- CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(2), aPool.getCountIgnoreCase());
+ CPPUNIT_ASSERT_EQUAL(5+extraCount, aPool.getCount());
+ CPPUNIT_ASSERT_EQUAL(2+extraCountIgnoreCase, aPool.getCountIgnoreCase());
// This shouldn't purge anything.
aPool.purge();
- CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(5), aPool.getCount());
- CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(2), aPool.getCountIgnoreCase());
+ CPPUNIT_ASSERT_EQUAL(5+extraCount, aPool.getCount());
+ CPPUNIT_ASSERT_EQUAL(2+extraCountIgnoreCase, aPool.getCountIgnoreCase());
// Delete one heap string object, and purge. That should purge one string.
pStr1.reset();
aPool.purge();
- CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(4), aPool.getCount());
- CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(2), aPool.getCountIgnoreCase());
+ CPPUNIT_ASSERT_EQUAL(4+extraCount, aPool.getCount());
+ CPPUNIT_ASSERT_EQUAL(2+extraCountIgnoreCase, aPool.getCountIgnoreCase());
// Nothing changes, because the upper-string is still in the map
pStr3.reset();
aPool.purge();
- CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(4), aPool.getCount());
- CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(2), aPool.getCountIgnoreCase());
+ CPPUNIT_ASSERT_EQUAL(4+extraCount, aPool.getCount());
+ CPPUNIT_ASSERT_EQUAL(2+extraCountIgnoreCase, aPool.getCountIgnoreCase());
// Again.
pStr2.reset();
aPool.purge();
- CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(2), aPool.getCount());
- CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(1), aPool.getCountIgnoreCase());
+ CPPUNIT_ASSERT_EQUAL(2+extraCount, aPool.getCount());
+ CPPUNIT_ASSERT_EQUAL(1+extraCountIgnoreCase, aPool.getCountIgnoreCase());
// Delete 'Bruce' and purge.
pStr4.reset();
aPool.purge();
- CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(0), aPool.getCount());
- CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(0), aPool.getCountIgnoreCase());
+ CPPUNIT_ASSERT_EQUAL(extraCount, aPool.getCount());
+ CPPUNIT_ASSERT_EQUAL(extraCountIgnoreCase, aPool.getCountIgnoreCase());
}
void Test::testSharedStringPoolPurgeBug1()
@@ -421,11 +435,26 @@ void Test::testSharedStringPoolPurgeBug1()
// purge() would de-reference a dangling pointer and consequently cause an
ASAN failure.
SvtSysLocale aSysLocale;
svl::SharedStringPool aPool(aSysLocale.GetCharClass());
+ size_t extraCount = aPool.getCount(); // internal items such as
SharedString::getEmptyString()
+ size_t extraCountIgnoreCase = aPool.getCountIgnoreCase();
aPool.intern("Andy");
aPool.intern("andy");
aPool.purge();
- CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(0), aPool.getCount());
- CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(0), aPool.getCountIgnoreCase());
+ CPPUNIT_ASSERT_EQUAL(extraCount, aPool.getCount());
+ CPPUNIT_ASSERT_EQUAL(extraCountIgnoreCase, aPool.getCountIgnoreCase());
+}
+
+void Test::testSharedStringPoolEmptyString()
+{
+ // Make sure SharedString::getEmptyString() is in the pool and matches
empty strings.
+ SvtSysLocale aSysLocale;
+ svl::SharedStringPool aPool(aSysLocale.GetCharClass());
+ aPool.intern("");
+ CPPUNIT_ASSERT_EQUAL(SharedString::getEmptyString(), aPool.intern(""));
+ CPPUNIT_ASSERT_EQUAL(SharedString::getEmptyString(),
aPool.intern(OUString()));
+ // And it should still work even after purging.
+ aPool.purge();
+ CPPUNIT_ASSERT_EQUAL(SharedString::getEmptyString(),
aPool.intern(OUString()));
}
void Test::checkPreviewString(SvNumberFormatter& aFormatter,
diff --git a/svl/source/misc/sharedstringpool.cxx
b/svl/source/misc/sharedstringpool.cxx
index 2fe8fd8a13ff..377ab5769029 100644
--- a/svl/source/misc/sharedstringpool.cxx
+++ b/svl/source/misc/sharedstringpool.cxx
@@ -70,6 +70,9 @@ struct SharedStringPool::Impl
SharedStringPool::SharedStringPool(const CharClass& rCharClass)
: mpImpl(new Impl(rCharClass))
{
+ // make sure the one empty string instance is shared in this pool as well
+ intern(OUString());
+ assert(intern(OUString()) == SharedString::getEmptyString());
}
SharedStringPool::~SharedStringPool() {}
