From 1b792d613011265f58bf98ce6a3a9d49687bd37b Mon Sep 17 00:00:00 2001 From: Assaf Passal Date: Tue, 19 Jul 2022 21:20:18 +0300 Subject: [PATCH] Change signture of newCharReader at CharReaderBuilder to return unique_ptr to ensure correct memory management --- doc/jsoncpp.dox | 2 +- example/readFromString/readFromString.cpp | 2 +- include/json/reader.h | 4 +- src/jsontestrunner/main.cpp | 2 +- src/lib_json/json_reader.cpp | 6 +-- src/test_lib_json/fuzz.cpp | 2 +- src/test_lib_json/main.cpp | 60 +++++++++++------------ 7 files changed, 39 insertions(+), 39 deletions(-) diff --git a/doc/jsoncpp.dox b/doc/jsoncpp.dox index f340591..ce4c976 100644 --- a/doc/jsoncpp.dox +++ b/doc/jsoncpp.dox @@ -116,7 +116,7 @@ CharReaders and StreamWriters are not thread-safe, but they are re-usable. \code Json::CharReaderBuilder rbuilder; cfg >> rbuilder.settings_; -std::unique_ptr const reader(rbuilder.newCharReader()); +std::unique_ptr const reader = rbuilder.newCharReader(); reader->parse(start, stop, &value1, &errs); // ... reader->parse(start, stop, &value2, &errs); diff --git a/example/readFromString/readFromString.cpp b/example/readFromString/readFromString.cpp index 0b29a4e..520b96b 100644 --- a/example/readFromString/readFromString.cpp +++ b/example/readFromString/readFromString.cpp @@ -22,7 +22,7 @@ int main() { reader.parse(rawJson, root); } else { Json::CharReaderBuilder builder; - const std::unique_ptr reader(builder.newCharReader()); + const std::unique_ptr reader = builder.newCharReader(); if (!reader->parse(rawJson.c_str(), rawJson.c_str() + rawJsonLength, &root, &err)) { std::cout << "error" << std::endl; diff --git a/include/json/reader.h b/include/json/reader.h index 46975d8..bc3aedb 100644 --- a/include/json/reader.h +++ b/include/json/reader.h @@ -270,7 +270,7 @@ public: /** \brief Allocate a CharReader via operator new(). * \throw std::exception if something goes wrong (e.g. invalid settings) */ - virtual CharReader* newCharReader() const = 0; + virtual std::unique_ptr newCharReader() const = 0; }; // Factory }; // CharReader @@ -337,7 +337,7 @@ public: CharReaderBuilder(); ~CharReaderBuilder() override; - CharReader* newCharReader() const override; + std::unique_ptr newCharReader() const override; /** \return true if 'settings' are legal and consistent; * otherwise, indicate bad settings via 'invalid'. diff --git a/src/jsontestrunner/main.cpp b/src/jsontestrunner/main.cpp index df717ff..60ca445 100644 --- a/src/jsontestrunner/main.cpp +++ b/src/jsontestrunner/main.cpp @@ -138,7 +138,7 @@ static int parseAndSaveValueTree(const Json::String& input, features.allowDroppedNullPlaceholders_; builder.settings_["allowNumericKeys"] = features.allowNumericKeys_; - std::unique_ptr reader(builder.newCharReader()); + std::unique_ptr reader = builder.newCharReader(); Json::String errors; const bool parsingSuccessful = reader->parse(input.data(), input.data() + input.size(), root, &errors); diff --git a/src/lib_json/json_reader.cpp b/src/lib_json/json_reader.cpp index 1ac5e81..6d1650d 100644 --- a/src/lib_json/json_reader.cpp +++ b/src/lib_json/json_reader.cpp @@ -1891,7 +1891,7 @@ public: CharReaderBuilder::CharReaderBuilder() { setDefaults(&settings_); } CharReaderBuilder::~CharReaderBuilder() = default; -CharReader* CharReaderBuilder::newCharReader() const { +std::unique_ptr CharReaderBuilder::newCharReader() const { bool collectComments = settings_["collectComments"].asBool(); OurFeatures features = OurFeatures::all(); features.allowComments_ = settings_["allowComments"].asBool(); @@ -1909,7 +1909,7 @@ CharReader* CharReaderBuilder::newCharReader() const { features.rejectDupKeys_ = settings_["rejectDupKeys"].asBool(); features.allowSpecialFloats_ = settings_["allowSpecialFloats"].asBool(); features.skipBom_ = settings_["skipBom"].asBool(); - return new OurCharReader(collectComments, features); + return std::make_unique(OurCharReader(collectComments, features)); } bool CharReaderBuilder::validate(Json::Value* invalid) const { @@ -1987,7 +1987,7 @@ bool parseFromStream(CharReader::Factory const& fact, IStream& sin, Value* root, char const* begin = doc.data(); char const* end = begin + doc.size(); // Note that we do not actually need a null-terminator. - CharReaderPtr const reader(fact.newCharReader()); + CharReaderPtr const reader = fact.newCharReader(); return reader->parse(begin, end, root, errs); } diff --git a/src/test_lib_json/fuzz.cpp b/src/test_lib_json/fuzz.cpp index 5b75c22..8932779 100644 --- a/src/test_lib_json/fuzz.cpp +++ b/src/test_lib_json/fuzz.cpp @@ -41,7 +41,7 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) { builder.settings_["collectComments"] = hash_settings & (1 << 9); builder.settings_["allowTrailingCommas_"] = hash_settings & (1 << 10); - std::unique_ptr reader(builder.newCharReader()); + std::unique_ptr reader = builder.newCharReader(); Json::Value root; const auto data_str = reinterpret_cast(data); diff --git a/src/test_lib_json/main.cpp b/src/test_lib_json/main.cpp index d0f5364..beae9f5 100644 --- a/src/test_lib_json/main.cpp +++ b/src/test_lib_json/main.cpp @@ -2994,7 +2994,7 @@ struct CharReaderTest : JsonTest::TestCase {}; JSONTEST_FIXTURE_LOCAL(CharReaderTest, parseWithNoErrors) { Json::CharReaderBuilder b; - CharReaderPtr reader(b.newCharReader()); + CharReaderPtr reader = b.newCharReader(); Json::String errs; Json::Value root; char const doc[] = R"({ "property" : "value" })"; @@ -3005,7 +3005,7 @@ JSONTEST_FIXTURE_LOCAL(CharReaderTest, parseWithNoErrors) { JSONTEST_FIXTURE_LOCAL(CharReaderTest, parseWithNoErrorsTestingOffsets) { Json::CharReaderBuilder b; - CharReaderPtr reader(b.newCharReader()); + CharReaderPtr reader = b.newCharReader(); Json::String errs; Json::Value root; char const doc[] = "{ \"property\" : [\"value\", \"value2\"], \"obj\" : " @@ -3018,7 +3018,7 @@ JSONTEST_FIXTURE_LOCAL(CharReaderTest, parseWithNoErrorsTestingOffsets) { JSONTEST_FIXTURE_LOCAL(CharReaderTest, parseNumber) { Json::CharReaderBuilder b; - CharReaderPtr reader(b.newCharReader()); + CharReaderPtr reader = b.newCharReader(); Json::String errs; Json::Value root; { @@ -3034,7 +3034,7 @@ JSONTEST_FIXTURE_LOCAL(CharReaderTest, parseNumber) { JSONTEST_FIXTURE_LOCAL(CharReaderTest, parseString) { Json::CharReaderBuilder b; - CharReaderPtr reader(b.newCharReader()); + CharReaderPtr reader = b.newCharReader(); Json::Value root; Json::String errs; { @@ -3090,7 +3090,7 @@ JSONTEST_FIXTURE_LOCAL(CharReaderTest, parseString) { } { b.settings_["allowSingleQuotes"] = true; - CharReaderPtr charreader(b.newCharReader()); + CharReaderPtr charreader = b.newCharReader(); char const doc[] = R"({'a': 'x\ty', "b":'x\\y'})"; bool ok = charreader->parse(doc, doc + std::strlen(doc), &root, &errs); JSONTEST_ASSERT(ok); @@ -3103,7 +3103,7 @@ JSONTEST_FIXTURE_LOCAL(CharReaderTest, parseString) { JSONTEST_FIXTURE_LOCAL(CharReaderTest, parseComment) { Json::CharReaderBuilder b; - CharReaderPtr reader(b.newCharReader()); + CharReaderPtr reader = b.newCharReader(); Json::Value root; Json::String errs; { @@ -3134,7 +3134,7 @@ JSONTEST_FIXTURE_LOCAL(CharReaderTest, parseComment) { JSONTEST_FIXTURE_LOCAL(CharReaderTest, parseObjectWithErrors) { Json::CharReaderBuilder b; - CharReaderPtr reader(b.newCharReader()); + CharReaderPtr reader = b.newCharReader(); Json::Value root; Json::String errs; { @@ -3157,7 +3157,7 @@ JSONTEST_FIXTURE_LOCAL(CharReaderTest, parseObjectWithErrors) { JSONTEST_FIXTURE_LOCAL(CharReaderTest, parseArrayWithErrors) { Json::CharReaderBuilder b; - CharReaderPtr reader(b.newCharReader()); + CharReaderPtr reader = b.newCharReader(); Json::Value root; Json::String errs; { @@ -3180,7 +3180,7 @@ JSONTEST_FIXTURE_LOCAL(CharReaderTest, parseArrayWithErrors) { JSONTEST_FIXTURE_LOCAL(CharReaderTest, parseWithOneError) { Json::CharReaderBuilder b; - CharReaderPtr reader(b.newCharReader()); + CharReaderPtr reader = b.newCharReader(); Json::String errs; Json::Value root; char const doc[] = R"({ "property" :: "value" })"; @@ -3193,7 +3193,7 @@ JSONTEST_FIXTURE_LOCAL(CharReaderTest, parseWithOneError) { JSONTEST_FIXTURE_LOCAL(CharReaderTest, parseChineseWithOneError) { Json::CharReaderBuilder b; - CharReaderPtr reader(b.newCharReader()); + CharReaderPtr reader = b.newCharReader(); Json::String errs; Json::Value root; char const doc[] = "{ \"pr佐藤erty\" :: \"value\" }"; @@ -3206,7 +3206,7 @@ JSONTEST_FIXTURE_LOCAL(CharReaderTest, parseChineseWithOneError) { JSONTEST_FIXTURE_LOCAL(CharReaderTest, parseWithDetailError) { Json::CharReaderBuilder b; - CharReaderPtr reader(b.newCharReader()); + CharReaderPtr reader = b.newCharReader(); Json::String errs; Json::Value root; char const doc[] = R"({ "property" : "v\alue" })"; @@ -3223,7 +3223,7 @@ JSONTEST_FIXTURE_LOCAL(CharReaderTest, parseWithStackLimit) { char const doc[] = R"({ "property" : "value" })"; { b.settings_["stackLimit"] = 2; - CharReaderPtr reader(b.newCharReader()); + CharReaderPtr reader = b.newCharReader(); Json::String errs; bool ok = reader->parse(doc, doc + std::strlen(doc), &root, &errs); JSONTEST_ASSERT(ok); @@ -3232,7 +3232,7 @@ JSONTEST_FIXTURE_LOCAL(CharReaderTest, parseWithStackLimit) { } { b.settings_["stackLimit"] = 1; - CharReaderPtr reader(b.newCharReader()); + CharReaderPtr reader = b.newCharReader(); Json::String errs; JSONTEST_ASSERT_THROWS( reader->parse(doc, doc + std::strlen(doc), &root, &errs)); @@ -3256,7 +3256,7 @@ JSONTEST_FIXTURE_LOCAL(CharReaderStrictModeTest, dupKeys) { R"({ "property" : "value", "key" : "val1", "key" : "val2" })"; { b.strictMode(&b.settings_); - CharReaderPtr reader(b.newCharReader()); + CharReaderPtr reader = b.newCharReader(); Json::String errs; bool ok = reader->parse(doc, doc + std::strlen(doc), &root, &errs); JSONTEST_ASSERT(!ok); @@ -3275,7 +3275,7 @@ JSONTEST_FIXTURE_LOCAL(CharReaderFailIfExtraTest, issue164) { char const doc[] = R"( "property" : "value" })"; { b.settings_["failIfExtra"] = false; - CharReaderPtr reader(b.newCharReader()); + CharReaderPtr reader = b.newCharReader(); Json::String errs; bool ok = reader->parse(doc, doc + std::strlen(doc), &root, &errs); JSONTEST_ASSERT(ok); @@ -3284,7 +3284,7 @@ JSONTEST_FIXTURE_LOCAL(CharReaderFailIfExtraTest, issue164) { } { b.settings_["failIfExtra"] = true; - CharReaderPtr reader(b.newCharReader()); + CharReaderPtr reader = b.newCharReader(); Json::String errs; bool ok = reader->parse(doc, doc + std::strlen(doc), &root, &errs); JSONTEST_ASSERT(!ok); @@ -3295,7 +3295,7 @@ JSONTEST_FIXTURE_LOCAL(CharReaderFailIfExtraTest, issue164) { } { b.strictMode(&b.settings_); - CharReaderPtr reader(b.newCharReader()); + CharReaderPtr reader = b.newCharReader(); Json::String errs; bool ok = reader->parse(doc, doc + std::strlen(doc), &root, &errs); JSONTEST_ASSERT(!ok); @@ -3307,7 +3307,7 @@ JSONTEST_FIXTURE_LOCAL(CharReaderFailIfExtraTest, issue164) { { b.strictMode(&b.settings_); b.settings_["failIfExtra"] = false; - CharReaderPtr reader(b.newCharReader()); + CharReaderPtr reader = b.newCharReader(); Json::String errs; bool ok = reader->parse(doc, doc + std::strlen(doc), &root, &errs); JSONTEST_ASSERT(!ok); @@ -3325,7 +3325,7 @@ JSONTEST_FIXTURE_LOCAL(CharReaderFailIfExtraTest, issue107) { Json::Value root; char const doc[] = "1:2:3"; b.settings_["failIfExtra"] = true; - CharReaderPtr reader(b.newCharReader()); + CharReaderPtr reader = b.newCharReader(); Json::String errs; bool ok = reader->parse(doc, doc + std::strlen(doc), &root, &errs); JSONTEST_ASSERT(!ok); @@ -3340,7 +3340,7 @@ JSONTEST_FIXTURE_LOCAL(CharReaderFailIfExtraTest, commentAfterObject) { { char const doc[] = "{ \"property\" : \"value\" } //trailing\n//comment\n"; b.settings_["failIfExtra"] = true; - CharReaderPtr reader(b.newCharReader()); + CharReaderPtr reader = b.newCharReader(); Json::String errs; bool ok = reader->parse(doc, doc + std::strlen(doc), &root, &errs); JSONTEST_ASSERT(ok); @@ -3353,7 +3353,7 @@ JSONTEST_FIXTURE_LOCAL(CharReaderFailIfExtraTest, commentAfterArray) { Json::Value root; char const doc[] = "[ \"property\" , \"value\" ] //trailing\n//comment\n"; b.settings_["failIfExtra"] = true; - CharReaderPtr reader(b.newCharReader()); + CharReaderPtr reader = b.newCharReader(); Json::String errs; bool ok = reader->parse(doc, doc + std::strlen(doc), &root, &errs); JSONTEST_ASSERT(ok); @@ -3365,7 +3365,7 @@ JSONTEST_FIXTURE_LOCAL(CharReaderFailIfExtraTest, commentAfterBool) { Json::Value root; char const doc[] = " true /*trailing\ncomment*/"; b.settings_["failIfExtra"] = true; - CharReaderPtr reader(b.newCharReader()); + CharReaderPtr reader = b.newCharReader(); Json::String errs; bool ok = reader->parse(doc, doc + std::strlen(doc), &root, &errs); JSONTEST_ASSERT(ok); @@ -3376,7 +3376,7 @@ JSONTEST_FIXTURE_LOCAL(CharReaderFailIfExtraTest, commentAfterBool) { JSONTEST_FIXTURE_LOCAL(CharReaderFailIfExtraTest, parseComment) { Json::CharReaderBuilder b; b.settings_["failIfExtra"] = true; - CharReaderPtr reader(b.newCharReader()); + CharReaderPtr reader = b.newCharReader(); Json::Value root; Json::String errs; { @@ -3454,7 +3454,7 @@ JSONTEST_FIXTURE_LOCAL(CharReaderAllowDropNullTest, issue178) { for (const auto& spec : specs) { Json::CharReaderBuilder b; b.settings_["allowDroppedNullPlaceholders"] = true; - std::unique_ptr reader(b.newCharReader()); + std::unique_ptr reader = b.newCharReader(); Json::Value root; Json::String errs; @@ -3475,7 +3475,7 @@ JSONTEST_FIXTURE_LOCAL(CharReaderAllowNumericKeysTest, allowNumericKeys) { b.settings_["allowNumericKeys"] = true; Json::Value root; Json::String errs; - CharReaderPtr reader(b.newCharReader()); + CharReaderPtr reader = b.newCharReader(); char const doc[] = "{15:true,-16:true,12.01:true}"; bool ok = reader->parse(doc, doc + std::strlen(doc), &root, &errs); JSONTEST_ASSERT(ok); @@ -3493,7 +3493,7 @@ JSONTEST_FIXTURE_LOCAL(CharReaderAllowSingleQuotesTest, issue182) { b.settings_["allowSingleQuotes"] = true; Json::Value root; Json::String errs; - CharReaderPtr reader(b.newCharReader()); + CharReaderPtr reader = b.newCharReader(); { char const doc[] = "{'a':true,\"b\":true}"; bool ok = reader->parse(doc, doc + std::strlen(doc), &root, &errs); @@ -3521,7 +3521,7 @@ JSONTEST_FIXTURE_LOCAL(CharReaderAllowZeroesTest, issue176) { b.settings_["allowSingleQuotes"] = true; Json::Value root; Json::String errs; - CharReaderPtr reader(b.newCharReader()); + CharReaderPtr reader = b.newCharReader(); { char const doc[] = "{'a':true,\"b\":true}"; bool ok = reader->parse(doc, doc + std::strlen(doc), &root, &errs); @@ -3546,7 +3546,7 @@ struct CharReaderAllowSpecialFloatsTest : JsonTest::TestCase {}; JSONTEST_FIXTURE_LOCAL(CharReaderAllowSpecialFloatsTest, specialFloat) { Json::CharReaderBuilder b; - CharReaderPtr reader(b.newCharReader()); + CharReaderPtr reader = b.newCharReader(); Json::Value root; Json::String errs; { @@ -3574,7 +3574,7 @@ JSONTEST_FIXTURE_LOCAL(CharReaderAllowSpecialFloatsTest, issue209) { b.settings_["allowSpecialFloats"] = true; Json::Value root; Json::String errs; - CharReaderPtr reader(b.newCharReader()); + CharReaderPtr reader = b.newCharReader(); { char const doc[] = R"({"a":NaN,"b":Infinity,"c":-Infinity,"d":+Infinity})"; bool ok = reader->parse(doc, doc + std::strlen(doc), &root, &errs); @@ -3655,7 +3655,7 @@ JSONTEST_FIXTURE_LOCAL(EscapeSequenceTest, readerParseEscapeSequence) { JSONTEST_FIXTURE_LOCAL(EscapeSequenceTest, charReaderParseEscapeSequence) { Json::CharReaderBuilder b; - CharReaderPtr reader(b.newCharReader()); + CharReaderPtr reader = b.newCharReader(); Json::Value root; Json::String errs; char const doc[] = "[\"\\\"\",\"\\/\",\"\\\\\",\"\\b\","