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 <erack@redhat.com>
(cherry picked from commit e47e0cb0ad1dc3554e9b57f8562a217cf785edbf)
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/140497
Reviewed-by: Xisco Fauli <xiscofauli@libreoffice.org>
(cherry picked from commit 84dae476666863f3367308cf1c5f80d45a43c3ef)
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/140601
Reviewed-by: Caolán McNamara <caolanm@redhat.com>
Tested-by: Caolán McNamara <caolanm@redhat.com>

2 files changed, 51 insertions, 19 deletions

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() {}