BIND 10 trac2377, updated. f4f1c1f0006d665f2e2216e3872d82b5ac25334b [2377] Allow checking if there was an error

BIND 10 source code commits bind10-changes at lists.isc.org
Mon Dec 10 12:52:44 UTC 2012


The branch, trac2377 has been updated
       via  f4f1c1f0006d665f2e2216e3872d82b5ac25334b (commit)
       via  f77ed1ab54bc60e08292d58fbc4cfc635cd1ef42 (commit)
       via  9e8813efff273e390680ccfd1e8893c06a4b5461 (commit)
      from  85c5f47252f0db064ece1a00c121cca9519d3b42 (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

- Log -----------------------------------------------------------------
commit f4f1c1f0006d665f2e2216e3872d82b5ac25334b
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date:   Mon Dec 10 13:52:05 2012 +0100

    [2377] Allow checking if there was an error

commit f77ed1ab54bc60e08292d58fbc4cfc635cd1ef42
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date:   Mon Dec 10 13:36:08 2012 +0100

    [2377] Make the construction exception safe

commit 9e8813efff273e390680ccfd1e8893c06a4b5461
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date:   Mon Dec 10 13:26:05 2012 +0100

    [2377] Reject load incremental of 0 RRs
    
    As it makes little sense.

-----------------------------------------------------------------------

Summary of changes:
 src/lib/dns/master_loader.cc                |   47 +++++++++++++++++++--------
 src/lib/dns/master_loader.h                 |   10 ++++++
 src/lib/dns/tests/master_loader_unittest.cc |   21 ++++++++++++
 3 files changed, 65 insertions(+), 13 deletions(-)

-----------------------------------------------------------------------
diff --git a/src/lib/dns/master_loader.cc b/src/lib/dns/master_loader.cc
index 8b91dd7..6fd40d8 100644
--- a/src/lib/dns/master_loader.cc
+++ b/src/lib/dns/master_loader.cc
@@ -21,8 +21,10 @@
 #include <dns/rdata.h>
 
 #include <string>
+#include <memory>
 
 using std::string;
+using std::auto_ptr;
 
 namespace isc {
 namespace dns {
@@ -45,12 +47,14 @@ public:
         initialized_(false),
         ok_(true),
         many_errors_((options & MANY_ERRORS) != 0),
-        complete_(false)
+        complete_(false),
+        seen_error_(false)
     {}
 
     void reportError(const std::string& filename, size_t line,
                      const std::string& reason)
     {
+        seen_error_ = true;
         callbacks_.error(filename, line, reason);
         if (!many_errors_) {
             // In case we don't have the lenient mode, every error is fatal
@@ -100,14 +104,20 @@ private:
     const std::string master_file_;
     std::string string_token_;
     bool initialized_;
-    bool ok_;
-    const bool many_errors_;
+    bool ok_;                   // Is it OK to continue loading?
+    const bool many_errors_;    // Are many errors allowed (or should we abort
+                                // on the first)
 public:
-    bool complete_;
+    bool complete_;             // All work done.
+    bool seen_error_;           // Was there at least one error during the
+                                // load?
 };
 
 bool
 MasterLoader::MasterLoaderImpl::loadIncremental(size_t count_limit) {
+    if (count_limit == 0) {
+        isc_throw(isc::InvalidParameter, "Count limit set to 0");
+    }
     if (complete_) {
         isc_throw(isc::InvalidOperation,
                   "Trying to load when already loaded");
@@ -167,12 +177,15 @@ MasterLoader::MasterLoaderImpl::loadIncremental(size_t count_limit) {
 
                 // Good, we loaded another one
                 ++count;
-            } else if (!many_errors_) {
-                ok_ = false;
-                complete_ = true;
-                // We don't have the exact error here, but it was reported
-                // by the error callback.
-                isc_throw(MasterLoaderError, "Invalid RR data");
+            } else {
+                seen_error_ = true;
+                if (!many_errors_) {
+                    ok_ = false;
+                    complete_ = true;
+                    // We don't have the exact error here, but it was reported
+                    // by the error callback.
+                    isc_throw(MasterLoaderError, "Invalid RR data");
+                }
             }
         } catch (const MasterLoaderError&) {
             // This is a hack. We exclude the MasterLoaderError from the
@@ -233,9 +246,12 @@ MasterLoader::MasterLoader(std::istream& stream,
     if (add_callback.empty()) {
         isc_throw(isc::InvalidParameter, "Empty add RR callback");
     }
-    impl_ = new MasterLoaderImpl("", zone_origin, zone_class, callbacks,
-                                 add_callback, options);
-    impl_->pushStreamSource(stream);
+    auto_ptr<MasterLoaderImpl> impl(new MasterLoaderImpl("", zone_origin,
+                                                         zone_class, callbacks,
+                                                         add_callback,
+                                                         options));
+    impl->pushStreamSource(stream);
+    impl_ = impl.release();
 }
 
 MasterLoader::~MasterLoader() {
@@ -249,5 +265,10 @@ MasterLoader::loadIncremental(size_t count_limit) {
     return (result);
 }
 
+bool
+MasterLoader::loadedSucessfully() const {
+    return (impl_->complete_ && !impl_->seen_error_);
+}
+
 } // end namespace dns
 } // end namespace isc
diff --git a/src/lib/dns/master_loader.h b/src/lib/dns/master_loader.h
index 659e885..9d8a755 100644
--- a/src/lib/dns/master_loader.h
+++ b/src/lib/dns/master_loader.h
@@ -132,6 +132,16 @@ public:
         }
     }
 
+    /// \brief Was the loading successful?
+    ///
+    /// \return true if and only if the loading was complete (after a call of
+    ///     load or after loadIncremental returned true) and there was no
+    ///     error. In other cases, return false.
+    /// \note While this method works even before the loading is complete (by
+    ///     returning false in that case), it is meant to be called only after
+    ///     finishing the load.
+    bool loadedSucessfully() const;
+
 private:
     class MasterLoaderImpl;
     MasterLoaderImpl* impl_;
diff --git a/src/lib/dns/tests/master_loader_unittest.cc b/src/lib/dns/tests/master_loader_unittest.cc
index fea7664..a80c055 100644
--- a/src/lib/dns/tests/master_loader_unittest.cc
+++ b/src/lib/dns/tests/master_loader_unittest.cc
@@ -128,7 +128,9 @@ TEST_F(MasterLoaderTest, basicLoad) {
     setLoader(TEST_DATA_SRCDIR "/example.org", Name("example.org."),
               RRClass::IN(), MasterLoader::MANY_ERRORS);
 
+    EXPECT_FALSE(loader_->loadedSucessfully());
     loader_->load();
+    EXPECT_TRUE(loader_->loadedSucessfully());
 
     EXPECT_TRUE(errors_.empty());
     EXPECT_TRUE(warnings_.empty());
@@ -146,7 +148,9 @@ TEST_F(MasterLoaderTest, streamConstructor) {
     setLoader(zone_stream, Name("example.org."), RRClass::IN(),
               MasterLoader::MANY_ERRORS);
 
+    EXPECT_FALSE(loader_->loadedSucessfully());
     loader_->load();
+    EXPECT_TRUE(loader_->loadedSucessfully());
 
     EXPECT_TRUE(errors_.empty());
     EXPECT_TRUE(warnings_.empty());
@@ -160,7 +164,9 @@ TEST_F(MasterLoaderTest, incrementalLoad) {
     setLoader(TEST_DATA_SRCDIR "/example.org", Name("example.org."),
               RRClass::IN(), MasterLoader::MANY_ERRORS);
 
+    EXPECT_FALSE(loader_->loadedSucessfully());
     EXPECT_FALSE(loader_->loadIncremental(2));
+    EXPECT_FALSE(loader_->loadedSucessfully());
 
     EXPECT_TRUE(errors_.empty());
     EXPECT_TRUE(warnings_.empty());
@@ -175,6 +181,7 @@ TEST_F(MasterLoaderTest, incrementalLoad) {
 
     // Load the rest.
     EXPECT_TRUE(loader_->loadIncremental(20));
+    EXPECT_TRUE(loader_->loadedSucessfully());
 
     EXPECT_TRUE(errors_.empty());
     EXPECT_TRUE(warnings_.empty());
@@ -232,7 +239,9 @@ TEST_F(MasterLoaderTest, brokenZone) {
             stringstream zone_stream(zone);
             setLoader(zone_stream, Name("example.org."), RRClass::IN(),
                       MasterLoader::DEFAULT);
+            EXPECT_FALSE(loader_->loadedSucessfully());
             EXPECT_THROW(loader_->load(), MasterLoaderError);
+            EXPECT_FALSE(loader_->loadedSucessfully());
             EXPECT_EQ(1, errors_.size()) << errors_[0];
             EXPECT_TRUE(warnings_.empty());
 
@@ -249,7 +258,9 @@ TEST_F(MasterLoaderTest, brokenZone) {
             stringstream zone_stream(zone);
             setLoader(zone_stream, Name("example.org."), RRClass::IN(),
                       MasterLoader::MANY_ERRORS);
+            EXPECT_FALSE(loader_->loadedSucessfully());
             EXPECT_NO_THROW(loader_->load());
+            EXPECT_FALSE(loader_->loadedSucessfully());
             EXPECT_EQ(1, errors_.size());
             EXPECT_TRUE(warnings_.empty());
             checkRR("example.org", RRType::SOA(), "ns1.example.org. "
@@ -267,7 +278,9 @@ TEST_F(MasterLoaderTest, brokenZone) {
             stringstream zone_stream(zoneEOF);
             setLoader(zone_stream, Name("example.org."), RRClass::IN(),
                       MasterLoader::MANY_ERRORS);
+            EXPECT_FALSE(loader_->loadedSucessfully());
             EXPECT_NO_THROW(loader_->load());
+            EXPECT_FALSE(loader_->loadedSucessfully());
             EXPECT_EQ(1, errors_.size());
             // FIXME: The invalid rdata generates a warning.
             // And we may want to generate warning ourself here too.
@@ -299,4 +312,12 @@ TEST_F(MasterLoaderTest, loadTwice) {
     loader_->load();
     EXPECT_THROW(loader_->load(), isc::InvalidOperation);
 }
+
+// Load 0 items should be rejected
+TEST_F(MasterLoaderTest, loadZero) {
+    setLoader(TEST_DATA_SRCDIR "/example.org", Name("example.org."),
+              RRClass::IN(), MasterLoader::MANY_ERRORS);
+    EXPECT_THROW(loader_->loadIncremental(0), isc::InvalidParameter);
+}
+
 }



More information about the bind10-changes mailing list