[2429] handled even more minor cases. added some more comments.

src/lib/dns/master_loader.cc

@@ -142,6 +142,7 @@ private:
         pushSource(filename);
     }
 
+    // Set/reset the default TTL.  Either from $TTL or SOA minimum TTL.
     void setDefaultTTL(const RRTTL& ttl) {
         if (!default_ttl_) {
             default_ttl_.reset(new RRTTL(ttl));
@@ -150,11 +151,8 @@ private:
         }
     }
 
-    void setDefaultTTL(const string& ttl_txt) {
-        setDefaultTTL(RRTTL(ttl_txt));
-        eatUntilEOL(true);
-    }
-
+    // Set/reset the TTL currently being used.  This can be used the last
+    // resort TTL when no other TTL is known for an RR.
     void setCurrentTTL(const RRTTL& ttl) {
         if (!current_ttl_) {
             current_ttl_.reset(new RRTTL(ttl));
@@ -163,6 +161,10 @@ private:
         }
     }
 
+    // Try to set/reset the current TTL from a candidate TTL.  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
+    // (and only in which case the current TTL is set).
     bool setCurrentTTL(const string& ttl_txt) {
         try {
             setCurrentTTL(RRTTL(ttl_txt));
@@ -225,7 +227,8 @@ private:
             isc_throw(isc::NotImplemented,
                       "Origin directive not implemented yet");
         } else if (iequals(directive, "TTL")) {
-            setDefaultTTL(getString());
+            setDefaultTTL(RRTTL(getString()));
+            eatUntilEOL(true);
         } else {
             isc_throw(InternalException, "Unknown directive '" <<
                       string(directive, directive + length) << "'");
@@ -267,8 +270,12 @@ private:
     const RRClass zone_class_;
     MasterLoaderCallbacks callbacks_;
     AddRRCallback add_callback_;
-    boost::scoped_ptr<RRTTL> default_ttl_;
-    boost::scoped_ptr<RRTTL> current_ttl_;
+    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.
     const MasterLoader::Options options_;
     const std::string master_file_;
     std::string string_token_;

src/lib/dns/tests/master_loader_unittest.cc

@@ -297,9 +297,27 @@ struct ErrorCase {
         "Include file not found and garbage at the end of line" },
     { "$TTL 100 extra-garbage", "Extra tokens at the end of line",
       "$TTL with extra token" },
+    { "$TTL", "unexpected end of input", "missing TTL" },
+    { "$TTL No-ttl", "Unknown unit used: N in: No-ttl", "bad TTL" },
+    { "$TTL \"100\"", "invalid TTL: \"100\"", "bad TTL, quoted" },
+    { "$TT 100", "Unknown directive 'TT'", "bad directive, too short" },
+    { "$TTLLIKE 100", "Unknown directive 'TTLLIKE'", "bad directive, extra" },
     { NULL, NULL, NULL }
 };
 
+// A commonly used helper to check callback message.
+void
+checkCallbackMessage(const string& actual_msg, const string& expected_msg,
+                     size_t expected_line) {
+    // The actual message should begin with the expected message.
+    EXPECT_EQ(0, actual_msg.find(expected_msg)) << "actual message: "
+                                                << actual_msg;
+
+    // and it should end with "...:<line_num>]"
+    const string line_desc = ":" + lexical_cast<string>(expected_line) + "]";
+    EXPECT_EQ(actual_msg.size() - line_desc.size(), actual_msg.find(line_desc));
+}
+
 // Test a broken zone is handled properly. We test several problems,
 // both in strict and lenient mode.
 TEST_F(MasterLoaderTest, brokenZone) {
@@ -318,7 +336,7 @@ TEST_F(MasterLoaderTest, brokenZone) {
             EXPECT_FALSE(loader_->loadedSucessfully());
             EXPECT_EQ(1, errors_.size());
             if (ec->reason != NULL) {
-                EXPECT_EQ(0, errors_.at(0).find(ec->reason));
+                checkCallbackMessage(errors_.at(0), ec->reason, 2);
             }
             EXPECT_TRUE(warnings_.empty());
 
@@ -397,29 +415,20 @@ TEST_F(MasterLoaderTest, ttlDirective) {
     // Set the default TTL with $TTL followed by an RR omitting the TTL
     zone_stream << "$TTL 1800\nexample.org. IN A 192.0.2.1\n";
     // $TTL can be quoted.  Also testing the case of $TTL being changed.
-    zone_stream << "\"$TTL\" 100\na1.example.org. IN A 192.0.2.2\n";
+    zone_stream << "\"$TTL\" 100\na.example.org. IN A 192.0.2.2\n";
     // Extended TTL form is accepted.
-    zone_stream << "$TTL 1H\na2.example.org. IN A 192.0.2.3\n";
+    zone_stream << "$TTL 1H\nb.example.org. IN A 192.0.2.3\n";
+    // Matching is case insensitive.
+    zone_stream << "$tTl 360\nc.example.org. IN A 192.0.2.3\n";
 
     setLoader(zone_stream, Name("example.org."), RRClass::IN(),
               MasterLoader::DEFAULT);
     loader_->load();
     EXPECT_TRUE(loader_->loadedSucessfully());
     checkRR("example.org", RRType::A(), "192.0.2.1", RRTTL(1800));
-    checkRR("a1.example.org", RRType::A(), "192.0.2.2", RRTTL(100));
-    checkRR("a2.example.org", RRType::A(), "192.0.2.3", RRTTL(3600));
-}
-
-// A commonly used helper to check callback message.
-void
-checkCallbackMessage(const string& actual_msg, const string& expected_msg,
-                     size_t expected_line) {
-    // The actual message should begin with the expected message.
-    EXPECT_EQ(0, actual_msg.find(expected_msg));
-
-    // and it should end with "...:<line_num>]"
-    const string line_desc = ":" + lexical_cast<string>(expected_line) + "]";
-    EXPECT_EQ(actual_msg.size() - line_desc.size(), actual_msg.find(line_desc));
+    checkRR("a.example.org", RRType::A(), "192.0.2.2", RRTTL(100));
+    checkRR("b.example.org", RRType::A(), "192.0.2.3", RRTTL(3600));
+    checkRR("c.example.org", RRType::A(), "192.0.2.3", RRTTL(360));
 }
 
 TEST_F(MasterLoaderTest, ttlFromSOA) {
@@ -441,7 +450,6 @@ TEST_F(MasterLoaderTest, ttlFromSOA) {
                          "no TTL specified; using SOA MINTTL instead", 1);
 }
 
-
 TEST_F(MasterLoaderTest, ttlFromPrevious) {
     // No available default TTL.  2nd and 3rd RR will use the TTL of the
     // 1st RR.  This will result in a warning, but only for the first time.