[2429] use boost::optional instead of homebrew and less clean interface

at the cost of increasing dependency, it will provide a cleaner and
more intuitive interface without sacrificing efficiency (much).

src/lib/dns/master_loader.cc

@@ -180,20 +180,6 @@ private:
         limitTTL(*default_ttl_, post_parsing);
     }
 
-    // Set/reset the TTL currently being used to the default TTL.
-    // This can be used as the last resort TTL when no other TTL is known for
-    // an RR.  The caller should guarantee that the default TTL has been
-    // defined by the time of this method call.  Note that this method doesn't
-    // have to call limitTTL(); it was already applied to the stored default
-    // TTL.
-    void setCurrentTTLToDefault() {
-        if (!current_ttl_) {
-            current_ttl_.reset(new RRTTL(*default_ttl_));
-        } else {
-            *current_ttl_ = *default_ttl_;
-        }
-    }
-
     // Try to set/reset the current TTL from candidate TTL text.  It's possible
     // it does not actually represent a TTL (which is not immediately
     // considered an error).  Return true iff it's recognized as a valid TTL
@@ -202,11 +188,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.
-        RRTTL* ttl = RRTTL::createFromText(ttl_txt, current_ttl_.get());
-        if (ttl != NULL) {
-            if (!current_ttl_) {
-                current_ttl_.reset(ttl);
-            }
+        const MaybeRRTTL maybe_ttl = RRTTL::createFromText(ttl_txt);
+        if (maybe_ttl) {
+            current_ttl_ = maybe_ttl;
             limitTTL(*current_ttl_, false);
             return (true);
         }
@@ -237,7 +221,7 @@ private:
                     dynamic_cast<const rdata::generic::SOA&>(*rdata).
                     getMinimum();
                 setDefaultTTL(RRTTL(ttl_val), true);
-                setCurrentTTLToDefault();
+                current_ttl_ = *default_ttl_;
             } else {
                 // On catching the exception we'll try to reach EOL again,
                 // so we need to unget it now.
@@ -246,7 +230,7 @@ private:
                                         "no TTL specified; load rejected");
             }
         } else if (!explicit_ttl && default_ttl_) {
-            setCurrentTTLToDefault();
+            current_ttl_ = *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).
@@ -313,9 +297,8 @@ private:
     boost::scoped_ptr<RRTTL> default_ttl_; // Default TTL of RRs used when
                                            // unspecified.  If NULL no default
                                            // is known.
-    boost::scoped_ptr<RRTTL> current_ttl_; // The TTL used most recently.
-                                           // Initially set to NULL.  Once set
-                                           // always non NULL.
+    MaybeRRTTL 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/rrttl.cc

@@ -59,7 +59,7 @@ namespace dns {
 
 namespace {
 bool
-parseTTLStr(const string& ttlstr, uint32_t& ttlval, string* error_txt) {
+parseTTLString(const string& ttlstr, uint32_t& ttlval, string* error_txt) {
     if (ttlstr.empty()) {
         if (error_txt != NULL) {
             *error_txt = "Empty TTL string";
@@ -161,22 +161,18 @@ parseTTLStr(const string& ttlstr, uint32_t& ttlval, string* error_txt) {
 
 RRTTL::RRTTL(const std::string& ttlstr) {
     string error_txt;
-    if (!parseTTLStr(ttlstr, ttlval_, &error_txt)) {
+    if (!parseTTLString(ttlstr, ttlval_, &error_txt)) {
         isc_throw(InvalidRRTTL, error_txt);
     }
 }
 
-RRTTL*
-RRTTL::createFromText(const string& ttlstr, RRTTL* placeholder) {
+MaybeRRTTL
+RRTTL::createFromText(const string& ttlstr) {
     uint32_t ttlval;
-    if (parseTTLStr(ttlstr, ttlval, NULL)) {
-        if (placeholder != NULL) {
-            *placeholder = RRTTL(ttlval);
-            return (placeholder);
-        }
-        return (new RRTTL(ttlval));
+    if (parseTTLString(ttlstr, ttlval, NULL)) {
+        return (MaybeRRTTL(ttlval));
     }
-    return (NULL);
+    return (MaybeRRTTL());
 }
 
 RRTTL::RRTTL(InputBuffer& buffer) {

src/lib/dns/rrttl.h

@@ -15,10 +15,12 @@
 #ifndef RRTTL_H
 #define RRTTL_H 1
 
-#include <stdint.h>
-
 #include <exceptions/exceptions.h>
 
+#include <boost/optional.hpp>
+
+#include <stdint.h>
+
 namespace isc {
 namespace util {
 class InputBuffer;
@@ -30,6 +32,16 @@ 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.
@@ -104,31 +116,35 @@ public:
     /// A separate factory of RRTTL from text.
     ///
     /// This static method is similar to the constructor that takes a string
-    /// object, but works as a factory and reports parsing failure in return
-    /// value.  Normally the constructor version should suffice, but in some
-    /// cases the caller may have to expect mixture of valid and invalid input,
-    /// and may want to minimize the overhead of possible exception handling.
-    /// This version is provided for such purpose.
-    ///
-    /// When the \c placeholder parameter is NULL, it creates a new RRTTL
-    /// object, allocating memory for it; the caller is responsible for
-    /// releasing the memory using the \c delete operator.  If \c placeholder
-    /// is non NULL, it will override the placeholder object with an RRTTL
-    /// corresponding to the given text and return a pointer to the placeholder
-    /// object.  This way, the caller can also minimize the overhead of memory
-    /// allocation if it needs to call this method many times.
-    ///
-    /// If the given text does not represent a valid RRTTL, it returns NULL;
-    /// if \c placeholder is non NULL, it will be intact.
+    /// object, but works as a factory and reports parsing failure in the
+    /// form of the return value.  Normally the constructor version should
+    /// suffice, but in some cases the caller may have to expect mixture of
+    /// valid and invalid input, and may want to minimize the overhead of
+    /// 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.
+    ///
+    /// One main purpose of this function is to minimize the overhead
+    /// when the given text does not represent a valid RR TTL.  For this
+    /// reason this function intentionally omits the capability of delivering
+    /// details reason for the parse failure, such as in the \c want()
+    /// string when exception is thrown from the constructor (it will
+    /// internally require a creation of string object, which is relatively
+    /// expensive).  If such detailed information is necessary, the constructor
+    /// version should be used to catch the resulting exception.
     ///
     /// This function never throws the \c InvalidRRTTL exception.
     ///
     /// \param ttlstr A string representation of the \c RRTTL.
-    /// \param placeholder If non NULL, an RRTTL object to be overridden
-    /// with an RRTTL for \c ttlstr.
-    /// \return A pointer to the created or overridden RRTTL object.
-    static RRTTL* createFromText(const std::string& ttlstr,
-                                 RRTTL* placeholder);
+    /// \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);
     ///
     //@}
 

src/lib/dns/tests/rrttl_unittest.cc

@@ -88,22 +88,14 @@ TEST_F(RRTTLTest, fromText) {
 }
 
 TEST_F(RRTTLTest, createFromText) {
-    // If placeholder is NULL, a new RRTTL object is allocated
-    boost::scoped_ptr<RRTTL> ttl_ptr;
-    ttl_ptr.reset(RRTTL::createFromText("3600", NULL));
-    ASSERT_TRUE(ttl_ptr);
-    EXPECT_EQ(RRTTL(3600), *ttl_ptr);
-
-    // If placeholder is non NULL, it will be overwritten
-    RRTTL ttl(3600);
-    EXPECT_NE(static_cast<RRTTL*>(NULL), RRTTL::createFromText("1800", &ttl));
-    EXPECT_EQ(RRTTL(1800), ttl);
-
-    // If text parsing fails, NULL is returned; if placeholder is given,
-    // it will be intact.
-    EXPECT_EQ(static_cast<RRTTL*>(NULL), RRTTL::createFromText("bad", NULL));
-    EXPECT_EQ(static_cast<RRTTL*>(NULL), RRTTL::createFromText("bad", &ttl));
-    EXPECT_EQ(RRTTL(1800), ttl);
+    // It returns an actual RRTT 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);
+
+    maybe_ttl = RRTTL::createFromText("bad");
+    EXPECT_FALSE(maybe_ttl);
 }
 
 void