[3112] Don't use boost::optional in libdns++

This has the side-effect of requiring additional copies of RRTTL
to be constructed and is less optimal. See the ticket on why
boost::optional is problematic.

src/lib/dns/master_loader.cc

@@ -224,15 +224,10 @@ private:
             // after the RR class below.
         }
 
-        const MaybeRRClass rrclass =
-            RRClass::createFromText(rrparam_token.getString());
+        boost::scoped_ptr<RRClass> rrclass
+            (RRClass::createFromText(rrparam_token.getString()));
         if (rrclass) {
-            // FIXME: The following code re-parses the rrparam_token to
-            // make an RRClass instead of using the MaybeRRClass above,
-            // because some old versions of boost::optional (that we
-            // still want to support) have a bug (see trac #2593). This
-            // workaround should be removed at some point in the future.
-            if (RRClass(rrparam_token.getString()) != zone_class_) {
+            if (*rrclass != zone_class_) {
                 isc_throw(InternalException, "Class mismatch: " << *rrclass <<
                           " vs. " << zone_class_);
             }
@@ -296,9 +291,9 @@ private:
         // We use the factory version instead of RRTTL constructor as we
         // need to expect cases where ttl_txt does not actually represent a TTL
         // but an RR class or type.
-        const MaybeRRTTL maybe_ttl = RRTTL::createFromText(ttl_txt);
-        if (maybe_ttl) {
-            current_ttl_ = maybe_ttl;
+        RRTTL* rrttl = RRTTL::createFromText(ttl_txt);
+        if (rrttl) {
+            current_ttl_.reset(rrttl);
             limitTTL(*current_ttl_, false);
             return (true);
         }
@@ -329,7 +324,7 @@ private:
                     dynamic_cast<const rdata::generic::SOA&>(*rdata).
                     getMinimum();
                 setDefaultTTL(RRTTL(ttl_val), true);
-                current_ttl_ = *default_ttl_;
+                current_ttl_.reset(new RRTTL(*default_ttl_));
             } else {
                 // On catching the exception we'll try to reach EOL again,
                 // so we need to unget it now.
@@ -338,7 +333,7 @@ private:
                                         "no TTL specified; load rejected");
             }
         } else if (!explicit_ttl && default_ttl_) {
-            current_ttl_ = *default_ttl_;
+            current_ttl_.reset(new RRTTL(*default_ttl_));
         } else if (!explicit_ttl && warn_rfc1035_ttl_) {
             // Omitted (class and) TTL values are default to the last
             // explicitly stated values (RFC 1035, Sec. 5.1).
@@ -407,8 +402,10 @@ private:
     boost::scoped_ptr<RRTTL> default_ttl_; // Default TTL of RRs used when
                                            // unspecified.  If NULL no default
                                            // is known.
-    MaybeRRTTL current_ttl_; // The TTL used most recently.  Initially unset.
-                             // Once set always stores a valid RRTTL.
+    boost::scoped_ptr<RRTTL> current_ttl_; // The TTL used most recently.
+                                           // Initially unset. Once set
+                                           // always stores a valid
+                                           // RRTTL.
     const MasterLoader::Options options_;
     const std::string master_file_;
     std::string string_token_;

src/lib/dns/rrclass-placeholder.h

@@ -35,16 +35,6 @@ namespace dns {
 // forward declarations
 class AbstractMessageRenderer;
 
-class RRClass; // forward declaration to define MaybeRRClass.
-
-/// \brief A shortcut for a compound type to represent RRClass-or-not.
-///
-/// A value of this type can be interpreted in a boolean context, whose
-/// value is \c true if and only if it contains a valid RRClass object.
-/// And, if it contains a valid RRClass object, its value is accessible
-/// using \c operator*, just like a bare pointer to \c RRClass.
-typedef boost::optional<RRClass> MaybeRRClass;
-
 ///
 /// \brief A standard DNS module exception that is thrown if an RRClass object
 /// is being constructed from an unrecognized string.
@@ -148,6 +138,12 @@ public:
     ///
     /// \param buffer A buffer storing the wire format data.
     explicit RRClass(isc::util::InputBuffer& buffer);
+    /// \brief Copy constructor.
+    ///
+    /// This constructor never throws an exception.
+    ///
+    /// \param other The RRClass to copy from.
+    RRClass(const RRClass& other) : classcode_(other.classcode_) {}
 
     /// A separate factory of RRClass from text.
     ///
@@ -163,12 +159,8 @@ public:
     /// <code>RRClass(const std::string&)</code> constructor.
     ///
     /// If the given text represents a valid RRClass, it returns a
-    /// \c MaybeRRClass object that stores a corresponding \c RRClass
-    /// object, which is accessible via \c operator*().  In this case
-    /// the returned object will be interpreted as \c true in a boolean
-    /// context.  If the given text does not represent a valid RRClass,
-    /// it returns a \c MaybeRRClass object which is interpreted as
-    /// \c false in a boolean context.
+    /// pointer to a new \c RRClass object. If the given text does not
+    /// represent a valid RRClass, it returns \c NULL.
     ///
     /// One main purpose of this function is to minimize the overhead
     /// when the given text does not represent a valid RR class.  For
@@ -183,9 +175,9 @@ public:
     /// This function never throws the \c InvalidRRClass exception.
     ///
     /// \param class_str A string representation of the \c RRClass.
-    /// \return A MaybeRRClass object either storing an RRClass object
-    /// for the given text or a \c false value.
-    static MaybeRRClass createFromText(const std::string& class_str);
+    /// \return A new RRClass object for the given text or a \c NULL
+    /// value.
+    static RRClass* createFromText(const std::string& class_str);
 
     ///
     /// We use the default copy constructor intentionally.

src/lib/dns/rrclass.cc

@@ -59,14 +59,14 @@ RRClass::toWire(AbstractMessageRenderer& renderer) const {
     renderer.writeUint16(classcode_);
 }
 
-MaybeRRClass
+RRClass*
 RRClass::createFromText(const string& class_str) {
     uint16_t class_code;
     if (RRParamRegistry::getRegistry().textToClassCode(class_str,
                                                        class_code)) {
-        return (MaybeRRClass(class_code));
+        return (new RRClass(class_code));
     }
-    return (MaybeRRClass());
+    return (NULL);
 }
 
 ostream&

src/lib/dns/rrttl.cc

@@ -166,13 +166,13 @@ RRTTL::RRTTL(const std::string& ttlstr) {
     }
 }
 
-MaybeRRTTL
+RRTTL*
 RRTTL::createFromText(const string& ttlstr) {
     uint32_t ttlval;
     if (parseTTLString(ttlstr, ttlval, NULL)) {
-        return (MaybeRRTTL(ttlval));
+        return (new RRTTL(ttlval));
     }
-    return (MaybeRRTTL());
+    return (NULL);
 }
 
 RRTTL::RRTTL(InputBuffer& buffer) {

src/lib/dns/rrttl.h

@@ -32,16 +32,6 @@ namespace dns {
 // forward declarations
 class AbstractMessageRenderer;
 
-class RRTTL;                    // forward declaration to define MaybeRRTTL
-
-/// \brief A shortcut for a compound type to represent RRTTL-or-not.
-///
-/// A value of this type can be interpreted in a boolean context, whose
-/// value is \c true if and only if it contains a valid RRTTL object.
-/// And, if it contains a valid RRTTL object, its value is accessible
-/// using \c operator*, just like a bare pointer to \c RRTTL.
-typedef boost::optional<RRTTL> MaybeRRTTL;
-
 ///
 /// \brief A standard DNS module exception that is thrown if an RRTTL object
 /// is being constructed from an unrecognized string.
@@ -112,6 +102,12 @@ public:
     ///
     /// \param buffer A buffer storing the wire format data.
     explicit RRTTL(isc::util::InputBuffer& buffer);
+    /// \brief Copy constructor.
+    ///
+    /// This constructor never throws an exception.
+    ///
+    /// \param other The RRTTL to copy from.
+    RRTTL(const RRTTL& other) : ttlval_(other.ttlval_) {}
 
     /// A separate factory of RRTTL from text.
     ///
@@ -123,12 +119,9 @@ public:
     /// possible exception handling.   This version is provided for such
     /// purpose.
     ///
-    /// If the given text represents a valid RRTTL, it returns a \c MaybeRRTTL
-    /// object that stores a corresponding \c RRTTL object, which is
-    /// accessible via \c operator*().  In this case the returned object will
-    /// be interpreted as \c true in a boolean context.  If the given text
-    /// does not represent a valid RRTTL, it returns a \c MaybeRRTTL object
-    /// which is interpreted as \c false in a boolean context.
+    /// If the given text represents a valid RRTTL, it returns a pointer
+    /// to a new RRTTL object. If the given text does not represent a
+    /// valid RRTTL, it returns \c NULL..
     ///
     /// One main purpose of this function is to minimize the overhead
     /// when the given text does not represent a valid RR TTL.  For this
@@ -142,9 +135,8 @@ public:
     /// This function never throws the \c InvalidRRTTL exception.
     ///
     /// \param ttlstr A string representation of the \c RRTTL.
-    /// \return An MaybeRRTTL object either storing an RRTTL object for
-    /// the given text or a \c false value.
-    static MaybeRRTTL createFromText(const std::string& ttlstr);
+    /// \return A new RRTTL object for the given text or a \c NULL value.
+    static RRTTL* createFromText(const std::string& ttlstr);
     ///
     //@}
 

src/lib/dns/tests/rrclass_unittest.cc

@@ -20,10 +20,13 @@
 
 #include <dns/tests/unittest_util.h>
 
+#include <boost/scoped_ptr.hpp>
+
 using namespace std;
 using namespace isc;
 using namespace isc::dns;
 using namespace isc::util;
+using boost::scoped_ptr;
 
 namespace {
 class RRClassTest : public ::testing::Test {
@@ -97,11 +100,11 @@ TEST_F(RRClassTest, toText) {
 }
 
 TEST_F(RRClassTest, createFromText) {
-    const MaybeRRClass rrclass("IN");
-    EXPECT_TRUE(rrclass);
-    EXPECT_EQ("IN", rrclass->toText());
-    EXPECT_TRUE(RRClass::createFromText("CH"));
-    EXPECT_FALSE(RRClass::createFromText("ZZ"));
+    scoped_ptr<RRClass> chclass(RRClass::createFromText("CH"));
+    EXPECT_TRUE(chclass);
+
+    scoped_ptr<RRClass> zzclass(RRClass::createFromText("ZZ"));
+    EXPECT_FALSE(zzclass);
 }
 
 TEST_F(RRClassTest, toWireBuffer) {

src/lib/dns/tests/rrttl_unittest.cc

@@ -26,6 +26,7 @@ using namespace std;
 using namespace isc;
 using namespace isc::dns;
 using namespace isc::util;
+using boost::scoped_ptr;
 
 namespace {
 class RRTTLTest : public ::testing::Test {
@@ -75,6 +76,12 @@ TEST_F(RRTTLTest, getValue) {
     EXPECT_EQ(0xffffffff, ttl_max.getValue());
 }
 
+TEST_F(RRTTLTest, copyConstruct) {
+    const RRTTL ttl1(3600);
+    const RRTTL ttl2(ttl1);
+    EXPECT_EQ(ttl1.getValue(), ttl2.getValue());
+}
+
 TEST_F(RRTTLTest, fromText) {
     // Border cases
     EXPECT_EQ(0, RRTTL("0").getValue());
@@ -88,14 +95,14 @@ TEST_F(RRTTLTest, fromText) {
 }
 
 TEST_F(RRTTLTest, createFromText) {
-    // It returns an actual RRTT iff the given text is recognized as a
+    // It returns an actual RRTTL iff the given text is recognized as a
     // valid RR TTL.
-    MaybeRRTTL maybe_ttl = RRTTL::createFromText("3600");
-    EXPECT_TRUE(maybe_ttl);
-    EXPECT_EQ(RRTTL(3600), *maybe_ttl);
+    scoped_ptr<RRTTL> good_ttl(RRTTL::createFromText("3600"));
+    EXPECT_TRUE(good_ttl);
+    EXPECT_EQ(RRTTL(3600), *good_ttl);
 
-    maybe_ttl = RRTTL::createFromText("bad");
-    EXPECT_FALSE(maybe_ttl);
+    scoped_ptr<RRTTL> bad_ttl(RRTTL::createFromText("bad"));
+    EXPECT_FALSE(bad_ttl);
 }
 
 void