ODFstudio/odfstudio
2
0
Fork 0
Code Issues Packages Activity

make sure SharedString::EMPTY_STRING is interned in pools (tdf#150647)

Browse Source 
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 <erack@redhat.com>
This commit is contained in:
Luboš Luňák
2022-09-23 11:51:24 +02:00
committed by Eike Rathke
parent 31a8de10e8
commit e47e0cb0ad
4 changed files with 61 additions and 25 deletions
Download Patch File Download Diff File Expand all files Collapse all files
include/svl/sharedstringpool.hxx
+2 -1
View File
@@ -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;
};
}
sc/qa/unit/ucalc.cxx
+8 -5
View File
@@ -383,6 +383,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
@@ -420,10 +424,9 @@ 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
clearRange(m_pDoc, ScAddress(0,0,0));
@@ -436,8 +439,8 @@ void Test::testSharedStringPool()
// 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
svl/qa/unit/svl.cxx
+48 -19
View File
@@ -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(SharedString::EMPTY_STRING));
// And it should still work even after purging.
aPool.purge();
CPPUNIT_ASSERT_EQUAL(SharedString::getEmptyString(), aPool.intern(SharedString::EMPTY_STRING));
}
void Test::checkPreviewString(SvNumberFormatter& aFormatter,
svl/source/misc/sharedstringpool.cxx
+3
View File
@@ -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(SharedString::EMPTY_STRING);
assert(intern(SharedString::EMPTY_STRING) == SharedString::getEmptyString());
}
SharedStringPool::~SharedStringPool() {}