fix leaks when using tools::JsonWriter

Specifically in sd/source/core/annotations/Annotation.cxx

We seem to end up fixing leaks here often.
The current tools::JsonWriter API is just very hard to use correctly.

So rather return an OString, which is cheap to copy,
and push that down into the LOK code.

AFAIK that seems to end up requiring less code and less adhoc copying
of data (specifically the queueing code in init.cxx was creating
copies when converting to std::string).

Ideally, we could have some special API to avoid the new strdup()
calls in init.cxx, but not sure how to prevent other people
from accidentally using that.

Change-Id: Ia33437c1bfd9cc2d54dfb99914d1b72db20335f2
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/149963
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>

2 files changed, 18 insertions, 35 deletions

diff --git a/tools/qa/cppunit/test_json_writer.cxx b/tools/qa/cppunit/test_json_writer.cxx
index 2e0706605745..b3d66fe92b03 100644
--- a/tools/qa/cppunit/test_json_writer.cxx
+++ b/tools/qa/cppunit/test_json_writer.cxx
@@ -43,11 +43,11 @@ void JsonWriterTest::test1()
         aJson.put("int", static_cast<sal_Int32>(12));
     }
 
-    std::unique_ptr<char, o3tl::free_delete> result(aJson.extractData());
+    OString result(aJson.finishAndGetAsOString());
 
-    CPPUNIT_ASSERT_EQUAL(std::string("{ \"node\": { \"oustring\": \"val1\", "
-                                     "\"charptr\": \"val3\", \"int\": 12}}"),
-                         std::string(result.get()));
+    CPPUNIT_ASSERT_EQUAL(OString("{ \"node\": { \"oustring\": \"val1\", "
+                                 "\"charptr\": \"val3\", \"int\": 12}}"),
+                         result);
 }
 
 void JsonWriterTest::test2()
@@ -69,12 +69,12 @@ void JsonWriterTest::test2()
         }
     }
 
-    std::unique_ptr<char, o3tl::free_delete> result(aJson.extractData());
+    OString result(aJson.finishAndGetAsOString());
 
-    CPPUNIT_ASSERT_EQUAL(std::string("{ \"node\": { \"field1\": \"val1\", \"field2\": \"val2\", "
-                                     "\"node\": { \"field3\": \"val3\", \"node\": { \"field4\": "
-                                     "\"val4\", \"field5\": \"val5\"}}}}"),
-                         std::string(result.get()));
+    CPPUNIT_ASSERT_EQUAL(OString("{ \"node\": { \"field1\": \"val1\", \"field2\": \"val2\", "
+                                 "\"node\": { \"field3\": \"val3\", \"node\": { \"field4\": "
+                                 "\"val4\", \"field5\": \"val5\"}}}}"),
+                         result);
 }
 
 void JsonWriterTest::testArray()
@@ -86,10 +86,9 @@ void JsonWriterTest::testArray()
         aJson.putSimpleValue("bar");
     }
 
-    std::unique_ptr<char, o3tl::free_delete> aResult(aJson.extractData());
+    OString aResult(aJson.finishAndGetAsOString());
 
-    CPPUNIT_ASSERT_EQUAL(std::string("{ \"items\": [ \"foo\", \"bar\"]}"),
-                         std::string(aResult.get()));
+    CPPUNIT_ASSERT_EQUAL(OString("{ \"items\": [ \"foo\", \"bar\"]}"), aResult);
 }
 
 CPPUNIT_TEST_SUITE_REGISTRATION(JsonWriterTest);
diff --git a/tools/source/misc/json_writer.cxx b/tools/source/misc/json_writer.cxx
index c5e92385904b..e7a0f55fd6c2 100644
--- a/tools/source/misc/json_writer.cxx
+++ b/tools/source/misc/json_writer.cxx
@@ -24,6 +24,7 @@ JsonWriter::JsonWriter()
     , mSpaceAllocated(DEFAULT_BUFFER_SIZE)
     , mStartNodeCount(0)
     , mbFirstFieldInNode(true)
+    , mbClosed(false)
 {
     *mPos = '{';
     ++mPos;
@@ -35,7 +36,7 @@ JsonWriter::JsonWriter()
 
 JsonWriter::~JsonWriter()
 {
-    assert(!mpBuffer && "forgot to extract data?");
+    assert(mbClosed && "forgot to extract data?");
     free(mpBuffer);
 }
 
@@ -314,7 +315,7 @@ void JsonWriter::addCommaBeforeField()
 
 void JsonWriter::ensureSpace(int noMoreBytesRequired)
 {
-    assert(mpBuffer && "already extracted data");
+    assert(!mbClosed && "already extracted data");
     int currentUsed = mPos - mpBuffer;
     if (currentUsed + noMoreBytesRequired >= mSpaceAllocated)
     {
@@ -351,36 +352,19 @@ void JsonWriter::putLiteral(std::string_view propName, std::string_view propValu
     validate();
 }
 
-/** Hands ownership of the underlying storage buffer to the caller,
-  * after this no more document modifications may be written. */
-std::pair<char*, int> JsonWriter::extractDataImpl()
+OString JsonWriter::finishAndGetAsOString()
 {
     assert(mStartNodeCount == 0 && "did not close all nodes");
-    assert(mpBuffer && "data already extracted");
+    assert(!mbClosed && "data already extracted");
     ensureSpace(2);
     // add closing brace
     *mPos = '}';
     ++mPos;
     // null-terminate
     *mPos = 0;
-    const int sz = mPos - mpBuffer;
-    mPos = nullptr;
-    return { std::exchange(mpBuffer, nullptr), sz };
-}
-
-OString JsonWriter::extractAsOString()
-{
-    auto[pChar, sz] = extractDataImpl();
-    OString ret(pChar, sz);
-    free(pChar);
-    return ret;
-}
+    mbClosed = true;
 
-std::string JsonWriter::extractAsStdString()
-{
-    auto[pChar, sz] = extractDataImpl();
-    std::string ret(pChar, sz);
-    free(pChar);
+    OString ret(mpBuffer, mPos - mpBuffer);
     return ret;
 }