Use unique_ptr rather than plain pointer

options.cpp

@@ -22,6 +22,11 @@
 
 #include "config.h"
 
+#include "debug.hpp"
+#include "types.hpp"
+#include "types/casts.hpp"
+
+#include <algorithm>
 #include <cassert>
 #include <cmath>
 #include <cstdint>
@@ -29,7 +34,6 @@
 #include <cstring>
 #include <cstring>
 #include <iostream>
-#include <memory>
 #include <stdexcept>
 
 
@@ -292,21 +296,20 @@ helpoption::execute (void) {
  * Command line processing options
  */
 
-processor::processor ()
+processor::processor () {
-{ add_option (new helpoption (this)); }
+    add_option (make_unique<helpoption> (this));
-
-
-processor::~processor () {
-    for (auto i: m_options)
-        delete i;
 }
 
 
+processor::~processor ()
+{ ; }
+
+
 void
 processor::print_usage (void) {
     cout << "Usage: " << m_command << " [options]" << endl;
 
-    for (const auto i: m_options)
+    for (const auto &i: m_options)
         cout << '\t' << *i << endl;
 }
 
@@ -437,7 +440,7 @@ processor::parse_args (int argc, const char ** argv) {
 
     // Must make sure each option has a chance to reset state (clear flags,
     // etc) between parses
-    for (auto i: m_options)
+    for (auto &i: m_options)
         i->reset ();
 
     const unsigned int FIRST_ARGUMENT = 1;
@@ -468,62 +471,76 @@ processor::parse_args (int argc, const char ** argv) {
         exit (EXIT_FAILURE);
     }
 
-    for (auto i: m_options)
+    for (auto &i: m_options)
         i->finish ();
 }
 
 
 void
-processor::add_option (option *opt) {
+processor::add_option (std::unique_ptr<option> opt) {
     if (m_shortopt.find (opt->shortopt ()) != m_shortopt.end ())
         throw logic_error ("Short option already exists");
     if (m_longopt.find (opt->longopt   ()) != m_longopt.end ())
         throw logic_error ("Long option already exists");
 
-    m_shortopt[opt->shortopt ()] = opt;
+    m_shortopt[opt->shortopt ()] = opt.get ();
-    m_longopt [opt->longopt  ()] = opt;
+    m_longopt [opt->longopt  ()] = opt.get ();
 
-    m_options.push_back (opt);
+    m_options.push_back (move (opt));
 }
 
 
-option*
+std::unique_ptr<option>
 processor::remove_option (char letter) {
     // Locate the option by short name
     const auto s_candidate = m_shortopt.find (letter);
     if (s_candidate == m_shortopt.end ())
         throw logic_error ("Cannot remove an option which is not present");
-    option *opt = (*s_candidate).second;
+    option *target = (*s_candidate).second;
 
     // Locate the long option entry
-    const auto l_candidate = m_longopt.find (opt->longopt ());
+    const auto l_candidate = m_longopt.find (target->longopt ());
     assert (l_candidate != m_longopt.end ());
 
     // Remove all references and return
+    auto prime = std::find_if (
+        m_options.begin (),
+        m_options.end (),
+        [&target] (std::unique_ptr<option> &rhs) { return rhs.get () == target; }
+    );
+    std::unique_ptr<option> opt = move (*prime);
+    
     m_shortopt.erase (s_candidate);
     m_longopt.erase  (l_candidate);
-    m_options.remove (opt);
+    m_options.erase  (prime);
 
     return opt;
 }
 
 
-option*
+std::unique_ptr<option>
 processor::remove_option (const char *name) {
     // Locate the option by long name
     const auto l_candidate = m_longopt.find (name);
     if (l_candidate == m_longopt.end ())
         throw logic_error ("Cannot remove an option which is not present");
-    option * opt = (*l_candidate).second;
+    option *target = (*l_candidate).second;
 
     // Locate the short option entry
-    const auto s_candidate = m_shortopt.find (opt->shortopt ());
+    const auto s_candidate = m_shortopt.find (target->shortopt ());
     assert (s_candidate != m_shortopt.end ());
 
-    // Remove all references and return the option object
+    // Remove all references and return
+    auto prime = std::find_if (
+        m_options.begin (),
+        m_options.end (),
+        [&target] (std::unique_ptr<option> &rhs) { return rhs.get () == target; }
+    );
+    std::unique_ptr<option> opt = move (*prime);
+    
     m_shortopt.erase (s_candidate);
     m_longopt.erase  (l_candidate);
-    m_options.remove (opt);
+    m_options.erase  (prime);
 
     return opt;
 }

options.hpp

@@ -24,6 +24,7 @@
 #include <iostream>
 #include <list>
 #include <map>
+#include <memory>
 #include <sstream>
 #include <stdexcept>
 
@@ -286,7 +287,7 @@ namespace util {
             std::map<char, option *> m_shortopt;
             std::map<std::string, option *> m_longopt;
 
-            std::list<option *> m_options;
+            std::list<std::unique_ptr<option>> m_options;
 
             // The command to execute the application
             std::string m_command;
@@ -308,14 +309,14 @@ namespace util {
             void parse_stream(std::istream & is);
 
 
-            void add_option(option * opt);
+            void add_option(std::unique_ptr<option>);
 
-            option* remove_option(char letter);
+            std::unique_ptr<option> remove_option(char letter);
-            option* remove_option(const char * name);
+            std::unique_ptr<option> remove_option(const char * name);
-            option* remove_option(const std::string& name)
+            std::unique_ptr<option> remove_option(const std::string& name)
-                { return remove_option(name.c_str()); }
+                { return remove_option (name.c_str()); }
-            option* remove_option(const option * opt)
+            std::unique_ptr<option> remove_option(const option *opt)
-                { return remove_option(opt->longopt ()); }
+                { return remove_option (opt->longopt ()); }
     };
 }
 

test/options/success.cpp

@@ -22,12 +22,12 @@ void
 test_null_opt (void) {
     unique_ptr<processor> p(new processor());
 
-    p->add_option(new nulloption('n', "null", "testing null option"));
+    p->add_option(make_unique<nulloption> ('n', "null", "testing null option"));
 
-    const char * argv1[] = { "./foo", "-n", "foo" };
+    static const char *argv1[] = { "./foo", "-n", "foo" };
     p->parse_args(elems(argv1), argv1);
 
-    const char * argv2[] = { "./foo", "--null", "foo" };
+    static const char *argv2[] = { "./foo", "--null", "foo" };
     p->parse_args(elems(argv2), argv2);
 }
 
@@ -39,22 +39,22 @@ void
 test_present_opt (void) {
     unique_ptr<processor> p(new processor());
     bool is_present;
-    p->add_option(new presentoption('p', "present", "option is present", &is_present));
+    p->add_option(make_unique<presentoption> ('p', "present", "option is present", &is_present));
 
     // Short option form
-    const char *argv1[] = { "./foo", "-p" };
+    static const char *argv1[] = { "./foo", "-p" };
     is_present = false;
     p->parse_args(elems(argv1), argv1);
     CHECK (is_present);
 
     // Long option form
-    const char *argv2[] = { "./foo", "--present" };
+    static const char *argv2[] = { "./foo", "--present" };
     is_present = false;
     p->parse_args(elems(argv2), argv2);
     CHECK (is_present);
 
     // Check that value is reset from true if not present
-    const char *argv3[] = { "./foo" };
+    static const char *argv3[] = { "./foo" };
     is_present = true;
     p->parse_args(elems(argv3), argv3);
     CHECK (!is_present);
@@ -67,21 +67,21 @@ test_bool_opt (void) {
     unique_ptr<processor> p(new processor());
     bool value = false;
 
-    p->add_option (new valueoption<bool>('b', "bool", "testing boolean actions", &value));
+    p->add_option (make_unique<valueoption<bool>> ('b', "bool", "testing boolean actions", &value));
 
     // List all legal forms of positive or negative boolean values
-    const char *argv[] = { "./foo", "-b", NULL };
+    static const char *argv[] = { "./foo", "-b", NULL };
-    const char *positive[] = { "1", "true", "yes" };
+    static const char *positive[] = { "1", "true", "yes" };
-    const char *negative[] = { "0", "false", "no" };
+    static const char *negative[] = { "0", "false", "no" };
 
     // For each boolean value, ensure that it returns as expected
-    for (off_t i = 0; i < elems (positive); ++i) {
+    for (size_t i = 0; i < elems (positive); ++i) {
         argv[2] = positive[i];
         p->parse_args (elems (argv), argv);
         CHECK (value == true);
     }
 
-    for (off_t i = 0; i < elems (negative); ++i) {
+    for (size_t i = 0; i < elems (negative); ++i) {
         argv[2] = negative[i];
         p->parse_args (elems (argv), argv);
         CHECK (value == false);
@@ -90,7 +90,7 @@ test_bool_opt (void) {
     // Check that invalid forms of boolean all throw exceptions
     const char* invalid[] = { "foo", "y", "null" };
 
-    for (off_t i = 0; i < elems (invalid); ++i) {
+    for (size_t i = 0; i < elems (invalid); ++i) {
         argv[2] = invalid[i];
         CHECK_THROWS (
             std::domain_error,
@@ -105,7 +105,7 @@ void
 test_numeric_opt (void) {
     unique_ptr<processor> p(new processor ());
     T value;
-    p->add_option (new valueoption<T> ('t', "type", "testing type option", &value));
+    p->add_option (make_unique<valueoption<T>> ('t', "type", "testing type option", &value));
 
     T values[] = {
         // TODO: Enable minimum value testing. Currently disabled as
@@ -120,7 +120,7 @@ test_numeric_opt (void) {
     const char * argv_short[] = { "./foo", "-t", NULL };
     const char *  argv_long[] = { "./foo", NULL };
 
-    for(off_t i = 0; i < elems (values); ++i) {
+    for(size_t i = 0; i < elems (values); ++i) {
         ostringstream out_short, out_long;
         string        str_short, str_long;
 
@@ -147,7 +147,7 @@ void
 test_bytes_opt(void) {
     unique_ptr<processor> p(new processor ());
 
-    struct {
+    static const struct {
         const char                *param;
         bytesoption::bytestype     type;
         bytesoption::bytesmodifier mod;
@@ -183,7 +183,7 @@ test_bytes_opt(void) {
 
     };
 
-    const char *argv[] = { 
+    static const char *argv[] = { 
         "./foo",
         "-b",
         NULL
@@ -191,16 +191,17 @@ test_bytes_opt(void) {
 
     for (unsigned int i = 0; i < elems (commands); ++i) {
         size_t size = 0;
-        option * opt = new bytesoption ('b', "bytes",
+        p->add_option(make_unique<bytesoption> (
+           'b', "bytes",
            "testing sizeof bytes", &size, commands[i].type,
-                commands[i].mod);
+           commands[i].mod 
-        p->add_option(opt);
+        ));
 
         argv[elems (argv) - 1] = commands[i].param;
         p->parse_args (elems (argv), argv);
 
         CHECK_EQ (commands[i].size, size);
-        delete p->remove_option (opt);
+        p->remove_option ('b');
     }
 }
 
@@ -208,28 +209,40 @@ test_bytes_opt(void) {
 void
 test_insert_remove_opt (void) {
     unique_ptr<processor> p(new processor ());
-    nulloption opt ('n', "null-option", "null testing action");
 
-    p->add_option (&opt);
+    {
-    CHECK_EQ (p->remove_option ('n'), (option*)&opt);
+        auto opt = make_unique<nulloption> ('n', "null-option", "null testing action");
+        auto cmp = opt.get ();
+        p->add_option (move (opt));
+        CHECK_EQ (p->remove_option ('n').get (), (option*)cmp);
+    }
 
-    p->add_option (&opt);
+    {
-    CHECK_EQ (p->remove_option ("null-option"), (option*)&opt);
+        auto opt = make_unique<nulloption> ('n', "null-option", "null testing action");
+        auto cmp = opt.get ();
+        p->add_option (move (opt));
+        CHECK_EQ (p->remove_option ("null-option").get (), (option*)cmp);
+    }
 
-    p->add_option (&opt);
+    {
-    CHECK_THROWS (std::logic_error, p->add_option (&opt));
+        auto opt1 = make_unique<nulloption> ('n', "null-option", "null testing action");
-
+        auto opt2 = make_unique<nulloption> ('n', "null-option", "null testing action");
-    p->remove_option (&opt);
+        p->add_option (move (opt1));
+        CHECK_THROWS (std::logic_error, p->add_option (move (opt2)));
+    }
 }
 
 
 void
 test_required (void) {
     unique_ptr<processor> p (new processor ());
-    p->add_option (new nulloption ('n',
+    p->add_option (make_unique <nulloption> (
+        'n',
         "null",
         "null testing",
-                                   true));
+        true
+    ));
+
     static const char *argv[] = {
         "./cpptest",
         "-n",
@@ -242,7 +255,7 @@ test_required (void) {
 
 
 int
-main (int argc, char **argv) {
+main (int, char **) {
     test_null_opt ();
     test_present_opt ();
     test_bool_opt ();