From 231b5a40a6af1c84acfaaf85f28cdfe243b191c2 Mon Sep 17 00:00:00 2001 From: Kai Blaschke Date: Mon, 31 Jan 2022 15:16:21 +0100 Subject: [PATCH] #3507: Added regression tests and fix another issue in binding(). --- Util/src/Option.cpp | 7 +- Util/testsuite/src/OptionTest.cpp | 204 ++++++++++++++++++++++++++---- Util/testsuite/src/OptionTest.h | 4 + 3 files changed, 189 insertions(+), 26 deletions(-) diff --git a/Util/src/Option.cpp b/Util/src/Option.cpp index 1aafe06b07..c8be30c49a 100644 --- a/Util/src/Option.cpp +++ b/Util/src/Option.cpp @@ -201,8 +201,11 @@ Option& Option::binding(const std::string& propertyName) Option& Option::binding(const std::string& propertyName, AbstractConfiguration* pConfig) { _binding = propertyName; - _pConfig = pConfig; - if (_pConfig) _pConfig->duplicate(); + if (pConfig != _pConfig) + { + _pConfig = pConfig; + if (_pConfig) _pConfig->duplicate(); + } return *this; } diff --git a/Util/testsuite/src/OptionTest.cpp b/Util/testsuite/src/OptionTest.cpp index b08d08aea2..4cdaf3e5ee 100644 --- a/Util/testsuite/src/OptionTest.cpp +++ b/Util/testsuite/src/OptionTest.cpp @@ -11,6 +11,7 @@ #include "OptionTest.h" #include "CppUnit/TestCaller.h" #include "CppUnit/TestSuite.h" +#include "Poco/Util/MapConfiguration.h" #include "Poco/Util/Option.h" #include "Poco/Util/OptionException.h" @@ -34,11 +35,11 @@ void OptionTest::testOption() .required(false) .repeatable(true) .argument("path"); - + Option libOpt = Option("library-dir", "L", "specify a library search path", false) .repeatable(true) .argument("path"); - + Option outOpt = Option("output", "o", "specify the output file", true) .argument("file", true); @@ -46,13 +47,13 @@ void OptionTest::testOption() .description("enable verbose mode") .required(false) .repeatable(false); - + Option optOpt = Option("optimize", "O") .description("enable optimization") .required(false) .repeatable(false) .argument("level", false); - + assertTrue (incOpt.shortName() == "I"); assertTrue (incOpt.fullName() == "include-dir"); assertTrue (incOpt.repeatable()); @@ -60,7 +61,7 @@ void OptionTest::testOption() assertTrue (incOpt.argumentName() == "path"); assertTrue (incOpt.argumentRequired()); assertTrue (incOpt.takesArgument()); - + assertTrue (libOpt.shortName() == "L"); assertTrue (libOpt.fullName() == "library-dir"); assertTrue (libOpt.repeatable()); @@ -100,7 +101,7 @@ void OptionTest::testMatches1() .required(false) .repeatable(true) .argument("path"); - + assertTrue (incOpt.matchesShort("Iinclude")); assertTrue (incOpt.matchesPartial("include:include")); assertTrue (incOpt.matchesPartial("include-dir:include")); @@ -109,12 +110,12 @@ void OptionTest::testMatches1() assertTrue (incOpt.matchesPartial("include")); assertTrue (incOpt.matchesShort("I")); assertTrue (incOpt.matchesPartial("i")); - + assertTrue (incOpt.matchesFull("include-dir:include")); assertTrue (incOpt.matchesFull("INClude-dir:include")); assertTrue (!incOpt.matchesFull("include:include")); assertTrue (!incOpt.matchesFull("include-dir2:include")); - + assertTrue (!incOpt.matchesPartial("include-dir2=include")); assertTrue (!incOpt.matchesShort("linclude")); } @@ -126,7 +127,7 @@ void OptionTest::testMatches2() .required(false) .repeatable(true) .argument("path"); - + assertTrue (!incOpt.matchesShort("Iinclude")); assertTrue (incOpt.matchesPartial("include:include")); assertTrue (incOpt.matchesPartial("include-dir:include")); @@ -134,12 +135,12 @@ void OptionTest::testMatches2() assertTrue (incOpt.matchesPartial("INCLUDE=include")); assertTrue (incOpt.matchesPartial("I")); assertTrue (incOpt.matchesPartial("i")); - + assertTrue (incOpt.matchesFull("include-dir:include")); assertTrue (incOpt.matchesFull("INClude-dir:include")); assertTrue (!incOpt.matchesFull("include:include")); assertTrue (!incOpt.matchesFull("include-dir2:include")); - + assertTrue (!incOpt.matchesFull("include-dir2=include")); assertTrue (!incOpt.matchesShort("linclude")); } @@ -165,7 +166,7 @@ void OptionTest::testProcess1() assertTrue (arg == "/usr/include"); incOpt.process("Include-dir:/proj/include", arg); assertTrue (arg == "/proj/include"); - + try { incOpt.process("I", arg); @@ -183,7 +184,7 @@ void OptionTest::testProcess1() catch (Poco::Util::MissingArgumentException&) { } - + try { incOpt.process("Llib", arg); @@ -192,17 +193,17 @@ void OptionTest::testProcess1() catch (Poco::Util::UnknownOptionException&) { } - + Option vrbOpt = Option("verbose", "v") .description("enable verbose mode") .required(false) .repeatable(false); - + vrbOpt.process("v", arg); assertTrue (arg.empty()); vrbOpt.process("verbose", arg); assertTrue (arg.empty()); - + try { vrbOpt.process("v2", arg); @@ -220,13 +221,13 @@ void OptionTest::testProcess1() catch (Poco::Util::UnexpectedArgumentException&) { } - + Option optOpt = Option("optimize", "O") .description("enable optimization") .required(false) .repeatable(false) .argument("level", false); - + optOpt.process("O", arg); assertTrue (arg.empty()); optOpt.process("O2", arg); @@ -258,7 +259,7 @@ void OptionTest::testProcess2() assertTrue (arg == "/usr/include"); incOpt.process("Include-dir:/proj/include", arg); assertTrue (arg == "/proj/include"); - + try { incOpt.process("Iinclude", arg); @@ -267,7 +268,7 @@ void OptionTest::testProcess2() catch (Poco::Util::UnknownOptionException&) { } - + try { incOpt.process("I", arg); @@ -285,7 +286,7 @@ void OptionTest::testProcess2() catch (Poco::Util::MissingArgumentException&) { } - + try { incOpt.process("Llib", arg); @@ -294,17 +295,17 @@ void OptionTest::testProcess2() catch (Poco::Util::UnknownOptionException&) { } - + Option vrbOpt = Option("verbose", "") .description("enable verbose mode") .required(false) .repeatable(false); - + vrbOpt.process("v", arg); assertTrue (arg.empty()); vrbOpt.process("verbose", arg); assertTrue (arg.empty()); - + try { vrbOpt.process("v2", arg); @@ -324,6 +325,157 @@ void OptionTest::testProcess2() } } +void OptionTest::testBindingRefCountingSingleBind() +{ + Poco::Util::MapConfiguration::Ptr mapConfiguration = Poco::makeAuto(); + + // Increment refcounter by 4 to prevent crashes- + mapConfiguration.duplicate(); + mapConfiguration.duplicate(); + mapConfiguration.duplicate(); + mapConfiguration.duplicate(); + + assertTrue(mapConfiguration->referenceCount() == 5); + + { + Option incOpt = Option("include-dir", "I", "specify an include search path") + .binding("includeDir", mapConfiguration); + + assertTrue(mapConfiguration->referenceCount() == 6); + } + + assertTrue(mapConfiguration->referenceCount() == 5); + + while (mapConfiguration->referenceCount() > 1) + { + mapConfiguration->release(); + } + + mapConfiguration.reset(); +} + +void OptionTest::testBindingRefCountingCopyWithBinding() +{ + Poco::Util::MapConfiguration::Ptr mapConfiguration = Poco::makeAuto(); + + // Increment refcounter by 4 to prevent crashes- + mapConfiguration.duplicate(); + mapConfiguration.duplicate(); + mapConfiguration.duplicate(); + mapConfiguration.duplicate(); + + assertTrue(mapConfiguration->referenceCount() == 5); + + { + Option incOpt = Option("include-dir", "I", "specify an include search path") + .binding("includeDir", mapConfiguration); + + assertTrue(mapConfiguration->referenceCount() == 6); + + { + Option incOpt2 = incOpt; + + assertTrue(mapConfiguration->referenceCount() == 7); + + Option incOpt3 = incOpt; + + assertTrue(mapConfiguration->referenceCount() == 8); + } + + assertTrue(mapConfiguration->referenceCount() == 6); + } + + assertTrue(mapConfiguration->referenceCount() == 5); + + while (mapConfiguration->referenceCount() > 1) + { + mapConfiguration->release(); + } + + mapConfiguration.reset(); +} + +void OptionTest::testBindingRefCountingMultipleBinds1() +{ + + Poco::Util::MapConfiguration::Ptr mapConfiguration = Poco::makeAuto(); + + // Increment refcounter by 4 to prevent crashes + mapConfiguration.duplicate(); + mapConfiguration.duplicate(); + mapConfiguration.duplicate(); + mapConfiguration.duplicate(); + + assertTrue(mapConfiguration->referenceCount() == 5); + + { + Option incOpt = Option("include-dir", "I", "specify an include search path") + .binding("includeDir", mapConfiguration); + + assertTrue(mapConfiguration->referenceCount() == 6); + + incOpt.binding("includeDir", mapConfiguration); + + assertTrue(mapConfiguration->referenceCount() == 6); + } + + assertTrue(mapConfiguration->referenceCount() == 5); + + while (mapConfiguration->referenceCount() > 1) + { + mapConfiguration->release(); + } + + mapConfiguration.reset(); +} + + +void OptionTest::testBindingRefCountingMultipleBinds2() +{ + + Poco::Util::MapConfiguration::Ptr mapConfiguration1 = Poco::makeAuto(); + Poco::Util::MapConfiguration::Ptr mapConfiguration2 = Poco::makeAuto(); + + // Increment refcounters by 4 to prevent crashes + for (int i = 0; i < 4; i++) + { + mapConfiguration1.duplicate(); + mapConfiguration2.duplicate(); + } + + assertTrue(mapConfiguration1->referenceCount() == 5); + assertTrue(mapConfiguration2->referenceCount() == 5); + + { + Option incOpt = Option("include-dir", "I", "specify an include search path") + .binding("includeDir", mapConfiguration1); + + assertTrue(mapConfiguration1->referenceCount() == 6); + assertTrue(mapConfiguration2->referenceCount() == 5); + + incOpt.binding("includeDir", mapConfiguration2); + + assertTrue(mapConfiguration1->referenceCount() == 5); + assertTrue(mapConfiguration2->referenceCount() == 6); + } + + assertTrue(mapConfiguration1->referenceCount() == 5); + assertTrue(mapConfiguration2->referenceCount() == 5); + + while (mapConfiguration1->referenceCount() > 1) + { + mapConfiguration1->release(); + } + + while (mapConfiguration2->referenceCount() > 1) + { + mapConfiguration2->release(); + } + + mapConfiguration1.reset(); + mapConfiguration2.reset(); +} + void OptionTest::setUp() { @@ -344,6 +496,10 @@ CppUnit::Test* OptionTest::suite() CppUnit_addTest(pSuite, OptionTest, testMatches2); CppUnit_addTest(pSuite, OptionTest, testProcess1); CppUnit_addTest(pSuite, OptionTest, testProcess2); + CppUnit_addTest(pSuite, OptionTest, testBindingRefCountingSingleBind); + CppUnit_addTest(pSuite, OptionTest, testBindingRefCountingCopyWithBinding); + CppUnit_addTest(pSuite, OptionTest, testBindingRefCountingMultipleBinds1); + CppUnit_addTest(pSuite, OptionTest, testBindingRefCountingMultipleBinds2); return pSuite; } diff --git a/Util/testsuite/src/OptionTest.h b/Util/testsuite/src/OptionTest.h index 6054dc68dd..25fb98eea2 100644 --- a/Util/testsuite/src/OptionTest.h +++ b/Util/testsuite/src/OptionTest.h @@ -29,6 +29,10 @@ class OptionTest: public CppUnit::TestCase void testMatches2(); void testProcess1(); void testProcess2(); + void testBindingRefCountingSingleBind(); + void testBindingRefCountingCopyWithBinding(); + void testBindingRefCountingMultipleBinds1(); + void testBindingRefCountingMultipleBinds2(); void setUp(); void tearDown();