From c0c22305d6b31b86633d7a1041b23bf1b1150f28 Mon Sep 17 00:00:00 2001 From: cfrandsen Date: Mon, 28 Jun 2021 11:20:23 -0600 Subject: [PATCH 1/3] Update value_semantic to return a boost shared pointer instead of a raw pointer This will return a managed pointer to the user instead of an unmanaged pointer. This helps static code analyzers from flagged false positive memory leaks It is becoming harder and harder for projects to use this project because some customers require a static code analyzer. By returning a managed pointer, those projects can continue to use this amazing library. --- .../program_options/detail/value_semantic.hpp | 13 ++-- .../program_options/options_description.hpp | 13 +++- .../boost/program_options/value_semantic.hpp | 56 +++++++-------- src/options_description.cpp | 28 ++++++-- src/value_semantic.cpp | 7 +- test/cmdline_test.cpp | 2 +- test/options_description_test.cpp | 68 ++++++++++++------- test/parsers_test.cpp | 28 ++++---- test/unicode_test.cpp | 6 +- test/variable_map_test.cpp | 9 +-- 10 files changed, 139 insertions(+), 91 deletions(-) diff --git a/include/boost/program_options/detail/value_semantic.hpp b/include/boost/program_options/detail/value_semantic.hpp index 9531339a34..bd49921510 100644 --- a/include/boost/program_options/detail/value_semantic.hpp +++ b/include/boost/program_options/detail/value_semantic.hpp @@ -6,6 +6,7 @@ // This file defines template functions that are declared in // ../value_semantic.hpp. +#include #include // forward declaration @@ -185,7 +186,7 @@ namespace boost { namespace program_options { } template - typed_value* + shared_ptr> value() { // Explicit qualification is vc6 workaround. @@ -193,26 +194,26 @@ namespace boost { namespace program_options { } template - typed_value* + shared_ptr> value(T* v) { - typed_value* r = new typed_value(v); + shared_ptr> r = boost::make_shared>(v); return r; } template - typed_value* + shared_ptr> wvalue() { return wvalue(0); } template - typed_value* + shared_ptr> wvalue(T* v) { - typed_value* r = new typed_value(v); + shared_ptr> r = boost::make_shared>(v); return r; } diff --git a/include/boost/program_options/options_description.hpp b/include/boost/program_options/options_description.hpp index 90d913d3e3..9cbde9dbb7 100644 --- a/include/boost/program_options/options_description.hpp +++ b/include/boost/program_options/options_description.hpp @@ -75,12 +75,12 @@ namespace program_options { after \-- short name. */ option_description(const char* name, - const value_semantic* s); + shared_ptr s); /** Initializes the class with the passed data. */ option_description(const char* name, - const value_semantic* s, + shared_ptr s, const char* description); virtual ~option_description(); @@ -167,6 +167,15 @@ namespace program_options { operator()(const char* name, const char* description); + options_description_easy_init& + operator()(const char* name, + shared_ptr s); + + options_description_easy_init& + operator()(const char* name, + shared_ptr s, + const char* description); + options_description_easy_init& operator()(const char* name, const value_semantic* s); diff --git a/include/boost/program_options/value_semantic.hpp b/include/boost/program_options/value_semantic.hpp index ac9dbc663b..5aec831ca6 100644 --- a/include/boost/program_options/value_semantic.hpp +++ b/include/boost/program_options/value_semantic.hpp @@ -12,6 +12,7 @@ #include #include #include +#include #include #include @@ -178,7 +179,8 @@ namespace boost { namespace program_options { /** Class which handles value of a specific type. */ template - class typed_value : public value_semantic_codecvt_helper + class typed_value : public enable_shared_from_this>, + public value_semantic_codecvt_helper #ifndef BOOST_NO_RTTI , public typed_value_base #endif @@ -196,11 +198,11 @@ namespace boost { namespace program_options { if none is explicitly specified. The type 'T' should provide operator<< for ostream. */ - typed_value* default_value(const T& v) + shared_ptr default_value(const T& v) { m_default_value = boost::any(v); m_default_value_as_text = boost::lexical_cast(v); - return this; + return this->shared_from_this(); } /** Specifies default value, which will be used @@ -209,30 +211,30 @@ namespace boost { namespace program_options { but textual representation of default value must be provided by the user. */ - typed_value* default_value(const T& v, const std::string& textual) + shared_ptr default_value(const T& v, const std::string& textual) { m_default_value = boost::any(v); m_default_value_as_text = textual; - return this; + return this->shared_from_this(); } /** Specifies an implicit value, which will be used if the option is given, but without an adjacent value. Using this implies that an explicit value is optional, */ - typed_value* implicit_value(const T &v) + shared_ptr implicit_value(const T &v) { m_implicit_value = boost::any(v); m_implicit_value_as_text = boost::lexical_cast(v); - return this; + return this->shared_from_this(); } /** Specifies the name used to to the value in help message. */ - typed_value* value_name(const std::string& name) + shared_ptr value_name(const std::string& name) { m_value_name = name; - return this; + return this->shared_from_this(); } /** Specifies an implicit value, which will be used @@ -245,36 +247,36 @@ namespace boost { namespace program_options { operator<< for ostream, but textual representation of default value must be provided by the user. */ - typed_value* implicit_value(const T &v, const std::string& textual) + shared_ptr implicit_value(const T &v, const std::string& textual) { m_implicit_value = boost::any(v); m_implicit_value_as_text = textual; - return this; + return this->shared_from_this(); } /** Specifies a function to be called when the final value is determined. */ - typed_value* notifier(function1 f) + shared_ptr notifier(function1 f) { m_notifier = f; - return this; + return this->shared_from_this(); } /** Specifies that the value is composing. See the 'is_composing' method for explanation. */ - typed_value* composing() + shared_ptr composing() { m_composing = true; - return this; + return this->shared_from_this(); } /** Specifies that the value can span multiple tokens. */ - typed_value* multitoken() + shared_ptr multitoken() { m_multitoken = true; - return this; + return this->shared_from_this(); } /** Specifies that no tokens may be provided as the value of @@ -284,17 +286,17 @@ namespace boost { namespace program_options { 'implicit_value' method should be also used. In most cases, you can use the 'bool_switch' function instead of using this method. */ - typed_value* zero_tokens() + shared_ptr zero_tokens() { m_zero_tokens = true; - return this; + return this->shared_from_this(); } /** Specifies that the value must occur. */ - typed_value* required() + shared_ptr required() { m_required = true; - return this; + return this->shared_from_this(); } public: // value semantic overrides @@ -381,13 +383,13 @@ namespace boost { namespace program_options { value of option into program variable. */ template - typed_value* + shared_ptr> value(); /** @overload */ template - typed_value* + shared_ptr> value(T* v); /** Creates a typed_value instance. This function is the primary @@ -395,25 +397,25 @@ namespace boost { namespace program_options { can later be passed to 'option_description' constructor. */ template - typed_value* + shared_ptr> wvalue(); /** @overload */ template - typed_value* + shared_ptr> wvalue(T* v); /** Works the same way as the 'value' function, but the created value_semantic won't accept any explicit value. So, if the option is present on the command line, the value will be 'true'. */ - BOOST_PROGRAM_OPTIONS_DECL typed_value* + BOOST_PROGRAM_OPTIONS_DECL shared_ptr> bool_switch(); /** @overload */ - BOOST_PROGRAM_OPTIONS_DECL typed_value* + BOOST_PROGRAM_OPTIONS_DECL shared_ptr> bool_switch(bool* v); }} diff --git a/src/options_description.cpp b/src/options_description.cpp index dc0eae8744..a49e05406d 100644 --- a/src/options_description.cpp +++ b/src/options_description.cpp @@ -11,6 +11,7 @@ // FIXME: this is only to get multiple_occurrences class // should move that to a separate headers. #include +#include #include @@ -50,7 +51,7 @@ namespace boost { namespace program_options { option_description:: option_description(const char* names, - const value_semantic* s) + shared_ptr s) : m_value_semantic(s) { this->set_names(names); @@ -59,7 +60,7 @@ namespace boost { namespace program_options { option_description:: option_description(const char* names, - const value_semantic* s, + shared_ptr s, const char* description) : m_description(description), m_value_semantic(s) { @@ -266,7 +267,7 @@ namespace boost { namespace program_options { // no value can be specified on command line. // FIXME: does not look exception-safe shared_ptr d( - new option_description(name, new untyped_value(true), description)); + new option_description(name, boost::make_shared(true), description)); owner->add(d); return *this; @@ -275,7 +276,7 @@ namespace boost { namespace program_options { options_description_easy_init& options_description_easy_init:: operator()(const char* name, - const value_semantic* s) + shared_ptr s) { shared_ptr d(new option_description(name, s)); owner->add(d); @@ -285,7 +286,7 @@ namespace boost { namespace program_options { options_description_easy_init& options_description_easy_init:: operator()(const char* name, - const value_semantic* s, + shared_ptr s, const char* description) { shared_ptr d(new option_description(name, s, description)); @@ -293,6 +294,23 @@ namespace boost { namespace program_options { owner->add(d); return *this; } + + options_description_easy_init& + options_description_easy_init:: + operator()(const char* name, + const value_semantic* s) + { + return (*this)(name, shared_ptr(s)); + } + + options_description_easy_init& + options_description_easy_init:: + operator()(const char* name, + const value_semantic* s, + const char* description) + { + return (*this)(name, shared_ptr(s), description); + } const unsigned options_description::m_default_line_length = 80; diff --git a/src/value_semantic.cpp b/src/value_semantic.cpp index a7366d4386..ed7eefaa83 100644 --- a/src/value_semantic.cpp +++ b/src/value_semantic.cpp @@ -8,6 +8,7 @@ #include #include #include +#include #include #include @@ -121,16 +122,16 @@ namespace boost { namespace program_options { value_store = new_tokens.empty() ? std::string("") : new_tokens.front(); } - BOOST_PROGRAM_OPTIONS_DECL typed_value* + BOOST_PROGRAM_OPTIONS_DECL shared_ptr> bool_switch() { return bool_switch(0); } - BOOST_PROGRAM_OPTIONS_DECL typed_value* + BOOST_PROGRAM_OPTIONS_DECL shared_ptr> bool_switch(bool* v) { - typed_value* r = new typed_value(v); + shared_ptr> r = boost::make_shared>(v); r->default_value(0); r->zero_tokens(); diff --git a/test/cmdline_test.cpp b/test/cmdline_test.cpp index 1fc0af83c9..fb14193b95 100644 --- a/test/cmdline_test.cpp +++ b/test/cmdline_test.cpp @@ -71,7 +71,7 @@ void apply_syntax(options_description& desc, stringstream ss; ss << syntax; while(ss >> s) { - value_semantic* v = 0; + boost::shared_ptr v = 0; if (*(s.end()-1) == '=') { v = value(); diff --git a/test/options_description_test.cpp b/test/options_description_test.cpp index d0df7ef397..7d0084228f 100644 --- a/test/options_description_test.cpp +++ b/test/options_description_test.cpp @@ -8,6 +8,7 @@ using namespace boost::program_options; #include +#include using namespace boost; #include @@ -42,12 +43,12 @@ void test_approximation() { options_description desc; desc.add_options() - ("foo", new untyped_value()) - ("fee", new untyped_value()) - ("baz", new untyped_value()) - ("all-chroots", new untyped_value()) - ("all-sessions", new untyped_value()) - ("all", new untyped_value()) + ("foo", boost::make_shared()) + ("fee", boost::make_shared()) + ("baz", boost::make_shared()) + ("all-chroots", boost::make_shared()) + ("all-sessions", boost::make_shared()) + ("all", boost::make_shared()) ; BOOST_CHECK_EQUAL(desc.find("fo", true).long_name(), "foo"); @@ -78,13 +79,13 @@ void test_approximation_with_multiname_options() { options_description desc; desc.add_options() - ("foo", new untyped_value()) - ("fee", new untyped_value()) - ("fe,baz", new untyped_value()) - ("chroots,all-chroots", new untyped_value()) - ("sessions,all-sessions", new untyped_value()) - ("everything,all", new untyped_value()) - ("qux,fo", new untyped_value()) + ("foo", boost::make_shared()) + ("fee", boost::make_shared()) + ("fe,baz", boost::make_shared()) + ("chroots,all-chroots", boost::make_shared()) + ("sessions,all-sessions", boost::make_shared()) + ("everything,all", boost::make_shared()) + ("qux,fo", boost::make_shared()) ; BOOST_CHECK_EQUAL(desc.find("fo", true).long_name(), "qux"); @@ -108,12 +109,12 @@ void test_long_names_for_option_description() { options_description desc; desc.add_options() - ("foo", new untyped_value()) - ("fe,baz", new untyped_value()) - ("chroots,all-chroots", new untyped_value()) - ("sessions,all-sessions", new untyped_value()) - ("everything,all", new untyped_value()) - ("qux,fo,q", new untyped_value()) + ("foo", boost::make_shared()) + ("fe,baz", boost::make_shared()) + ("chroots,all-chroots", boost::make_shared()) + ("sessions,all-sessions", boost::make_shared()) + ("everything,all", boost::make_shared()) + ("qux,fo,q", boost::make_shared()) ; BOOST_CHECK_EQUAL(desc.find("foo", false, false, false).long_names().second, 1u); @@ -134,16 +135,16 @@ void test_formatting() // Long option descriptions used to crash on MSVC-8.0. options_description desc; desc.add_options()( - "test", new untyped_value(), + "test", boost::make_shared(), "foo foo foo foo foo foo foo foo foo foo foo foo foo foo" "foo foo foo foo foo foo foo foo foo foo foo foo foo foo" "foo foo foo foo foo foo foo foo foo foo foo foo foo foo" "foo foo foo foo foo foo foo foo foo foo foo foo foo foo") - ("list", new untyped_value(), + ("list", boost::make_shared(), "a list:\n \t" "item1, item2, item3, item4, item5, item6, item7, item8, item9, " "item10, item11, item12, item13, item14, item15, item16, item17, item18") - ("well_formated", new untyped_value(), + ("well_formated", boost::make_shared(), "As you can see this is a very well formatted option description.\n" "You can do this for example:\n\n" "Values:\n" @@ -183,7 +184,7 @@ void test_multiname_option_formatting() { options_description desc; desc.add_options() - ("foo,bar", new untyped_value(), "a multiple-name option") + ("foo,bar", boost::make_shared(), "a multiple-name option") ; stringstream ss; @@ -201,9 +202,9 @@ void test_formatting_description_length() options_description::m_default_line_length, options_description::m_default_line_length / 2U); desc.add_options() - ("an-option-that-sets-the-max", new untyped_value(), // > 40 available for desc + ("an-option-that-sets-the-max", boost::make_shared(), // > 40 available for desc "this description sits on the same line, but wrapping should still work correctly") - ("a-long-option-that-would-leave-very-little-space-for-description", new untyped_value(), + ("a-long-option-that-would-leave-very-little-space-for-description", boost::make_shared(), "the description of the long opt, but placed on the next line\n" " \talso ensure that the tabulation works correctly when a" " description size has been set"); @@ -229,7 +230,7 @@ void test_formatting_description_length() options_description::m_default_line_length, options_description::m_default_line_length - 10U); // leaves < 23 (default option space) desc.add_options() - ("an-option-that-encroaches-description", new untyped_value(), + ("an-option-that-encroaches-description", boost::make_shared(), "this description should always be placed on the next line, and wrapping should continue as normal"); stringstream ss; @@ -315,6 +316,20 @@ void test_value_name() ); } +void test_backwards_compatibility_with_raw_pointer() +{ + options_description desc; + desc.add_options() + ("foo", new untyped_value(), "a raw pointer option") + ; + + stringstream ss; + ss << desc; + BOOST_CHECK_EQUAL(ss.str(), +" --foo arg a raw pointer option\n" + ); +} + void test_multiname_key_and_switch_selection() { // cases: @@ -342,6 +357,7 @@ int main(int, char* []) test_word_wrapping(); test_default_values(); test_value_name(); + test_backwards_compatibility_with_raw_pointer(); return 0; } diff --git a/test/parsers_test.cpp b/test/parsers_test.cpp index 8f01731dcd..14f1d17e7c 100644 --- a/test/parsers_test.cpp +++ b/test/parsers_test.cpp @@ -104,13 +104,13 @@ void test_parsing_without_specifying_options() { void test_many_different_options() { options_description desc; desc.add_options() - ("foo,f", new untyped_value(), "") + ("foo,f", boost::make_shared(), "") ( // Explicit qualification is a workaround for vc6 "bar,b", po::value(), "") - ("car,voiture", new untyped_value()) - ("dog,dawg", new untyped_value()) - ("baz", new untyped_value()) - ("plug*", new untyped_value()); + ("car,voiture", boost::make_shared()) + ("dog,dawg", boost::make_shared()) + ("baz", boost::make_shared()) + ("plug*", boost::make_shared()); const char* cmdline3_[] = { "--foo=12", "-f4", "--bar=11", "-b4", "--voiture=15", "--dawg=16", "--dog=17", "--plug3=10" }; vector cmdline3 = sv(cmdline3_, @@ -255,13 +255,13 @@ void test_config_file(const char* config_file) { options_description desc; desc.add_options() - ("gv1", new untyped_value) - ("gv2", new untyped_value) - ("empty_value", new untyped_value) - ("plug*", new untyped_value) - ("m1.v1", new untyped_value) - ("m1.v2", new untyped_value) - ("m1.v3,alias3", new untyped_value) + ("gv1", boost::shared_ptr()) + ("gv2", boost::shared_ptr()) + ("empty_value", boost::shared_ptr()) + ("plug*", boost::shared_ptr()) + ("m1.v1", boost::shared_ptr()) + ("m1.v2", boost::shared_ptr()) + ("m1.v3,alias3", boost::shared_ptr()) ("b", bool_switch()) ; @@ -304,8 +304,8 @@ void test_environment() { options_description desc; desc.add_options() - ("foo", new untyped_value, "") - ("bar", new untyped_value, "") + ("foo", boost::shared_ptr(), "") + ("bar", boost::shared_ptr(), "") ; #if (defined(_WIN32) && ! defined(BOOST_BORLANDC) && ! defined(BOOST_EMBTC)) || (defined(__CYGWIN__)) diff --git a/test/unicode_test.cpp b/test/unicode_test.cpp index 97d19f0b40..6c86b54e4d 100644 --- a/test/unicode_test.cpp +++ b/test/unicode_test.cpp @@ -107,11 +107,11 @@ void test_command_line() { options_description desc; desc.add_options() - ("foo,f", new untyped_value(), "") + ("foo,f", boost::make_shared(), "") // Explicit qualification is a workaround for vc6 ("bar,b", po::value(), "") - ("baz", new untyped_value()) - ("qux,plug*", new untyped_value()) + ("baz", boost::make_shared()) + ("qux,plug*", boost::make_shared()) ; const wchar_t* cmdline4_[] = { L"--foo=1\u0FF52", L"-f4", L"--bar=11", diff --git a/test/variable_map_test.cpp b/test/variable_map_test.cpp index 04b43473d6..54e3cad871 100644 --- a/test/variable_map_test.cpp +++ b/test/variable_map_test.cpp @@ -13,6 +13,7 @@ using namespace boost::program_options; namespace po = boost::program_options; #include +#include using namespace boost; #include @@ -32,11 +33,11 @@ void test_variable_map() { options_description desc; desc.add_options() - ("foo,f", new untyped_value) + ("foo,f", boost::make_shared()) ("bar,b", po::value()) ("biz,z", po::value()) - ("baz", new untyped_value()) - ("output,o", new untyped_value(), "") + ("baz", boost::make_shared()) + ("output,o", boost::make_shared(), "") ; const char* cmdline3_[] = { "--foo='12'", "--bar=11", "-z3", "-ofoo" }; vector cmdline3 = sv(cmdline3_, @@ -124,7 +125,7 @@ void test_semantic_values() { options_description desc; desc.add_options() - ("foo", new untyped_value()) + ("foo", boost::make_shared()) ("bar", po::value()) ("biz", po::value< vector >()) ("baz", po::value< vector >()->multitoken()) From 4477671a0e1c531c900f22bfe1dc46f9803b74bf Mon Sep 17 00:00:00 2001 From: cfrandsen Date: Thu, 1 Jul 2021 13:31:58 -0600 Subject: [PATCH 2/3] Fix cygwin errors: change >> to > > --- .../program_options/detail/value_semantic.hpp | 12 ++++++------ include/boost/program_options/value_semantic.hpp | 14 +++++++------- src/value_semantic.cpp | 6 +++--- 3 files changed, 16 insertions(+), 16 deletions(-) diff --git a/include/boost/program_options/detail/value_semantic.hpp b/include/boost/program_options/detail/value_semantic.hpp index bd49921510..30746f64a1 100644 --- a/include/boost/program_options/detail/value_semantic.hpp +++ b/include/boost/program_options/detail/value_semantic.hpp @@ -186,7 +186,7 @@ namespace boost { namespace program_options { } template - shared_ptr> + shared_ptr > value() { // Explicit qualification is vc6 workaround. @@ -194,26 +194,26 @@ namespace boost { namespace program_options { } template - shared_ptr> + shared_ptr > value(T* v) { - shared_ptr> r = boost::make_shared>(v); + shared_ptr > r = boost::make_shared >(v); return r; } template - shared_ptr> + shared_ptr > wvalue() { return wvalue(0); } template - shared_ptr> + shared_ptr > wvalue(T* v) { - shared_ptr> r = boost::make_shared>(v); + shared_ptr > r = boost::make_shared >(v); return r; } diff --git a/include/boost/program_options/value_semantic.hpp b/include/boost/program_options/value_semantic.hpp index 5aec831ca6..bb68214169 100644 --- a/include/boost/program_options/value_semantic.hpp +++ b/include/boost/program_options/value_semantic.hpp @@ -179,7 +179,7 @@ namespace boost { namespace program_options { /** Class which handles value of a specific type. */ template - class typed_value : public enable_shared_from_this>, + class typed_value : public enable_shared_from_this >, public value_semantic_codecvt_helper #ifndef BOOST_NO_RTTI , public typed_value_base @@ -383,13 +383,13 @@ namespace boost { namespace program_options { value of option into program variable. */ template - shared_ptr> + shared_ptr > value(); /** @overload */ template - shared_ptr> + shared_ptr > value(T* v); /** Creates a typed_value instance. This function is the primary @@ -397,25 +397,25 @@ namespace boost { namespace program_options { can later be passed to 'option_description' constructor. */ template - shared_ptr> + shared_ptr > wvalue(); /** @overload */ template - shared_ptr> + shared_ptr > wvalue(T* v); /** Works the same way as the 'value' function, but the created value_semantic won't accept any explicit value. So, if the option is present on the command line, the value will be 'true'. */ - BOOST_PROGRAM_OPTIONS_DECL shared_ptr> + BOOST_PROGRAM_OPTIONS_DECL shared_ptr > bool_switch(); /** @overload */ - BOOST_PROGRAM_OPTIONS_DECL shared_ptr> + BOOST_PROGRAM_OPTIONS_DECL shared_ptr > bool_switch(bool* v); }} diff --git a/src/value_semantic.cpp b/src/value_semantic.cpp index ed7eefaa83..49204314c4 100644 --- a/src/value_semantic.cpp +++ b/src/value_semantic.cpp @@ -122,16 +122,16 @@ namespace boost { namespace program_options { value_store = new_tokens.empty() ? std::string("") : new_tokens.front(); } - BOOST_PROGRAM_OPTIONS_DECL shared_ptr> + BOOST_PROGRAM_OPTIONS_DECL shared_ptr > bool_switch() { return bool_switch(0); } - BOOST_PROGRAM_OPTIONS_DECL shared_ptr> + BOOST_PROGRAM_OPTIONS_DECL shared_ptr > bool_switch(bool* v) { - shared_ptr> r = boost::make_shared>(v); + shared_ptr > r = boost::make_shared >(v); r->default_value(0); r->zero_tokens(); From 7601fe731656c98c615f517d000b897d15db0025 Mon Sep 17 00:00:00 2001 From: cfrandsen Date: Thu, 1 Jul 2021 14:31:10 -0600 Subject: [PATCH 3/3] Fix cygwin failure: setting shared pointer to 0 results in error: conversion from 'int' to non-scalar type --- test/cmdline_test.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/cmdline_test.cpp b/test/cmdline_test.cpp index fb14193b95..d3e56b9313 100644 --- a/test/cmdline_test.cpp +++ b/test/cmdline_test.cpp @@ -71,7 +71,7 @@ void apply_syntax(options_description& desc, stringstream ss; ss << syntax; while(ss >> s) { - boost::shared_ptr v = 0; + boost::shared_ptr v; if (*(s.end()-1) == '=') { v = value();