Browse Source

[2433] removed def ctor of callbacks; allow passing NULL (empty) instead.

JINMEI Tatuya 12 years ago
parent
commit
2d107bd1f9
2 changed files with 31 additions and 20 deletions
  1. 14 4
      src/lib/dns/tests/zone_checker_unittest.cc
  2. 17 16
      src/lib/dns/zone_checker.h

+ 14 - 4
src/lib/dns/tests/zone_checker_unittest.cc

@@ -113,10 +113,6 @@ TEST_F(ZoneCheckerTest, checkGood) {
     EXPECT_TRUE(checkZone(zname_, zclass_, *rrsets_, callbacks_));
     checkIssues();
 
-    // We can omit callbacks, in which case the default constructor for
-    // the callbacks is used, meaning callbacks are no-op.
-    EXPECT_TRUE(checkZone(zname_, zclass_, *rrsets_));
-
     // Multiple NS RRs are okay.
     rrsets_->removeRRset(zname_, zclass_, RRType::NS());
     ns_->addRdata(generic::NS(ns_txt1));
@@ -133,6 +129,13 @@ TEST_F(ZoneCheckerTest, checkSOA) {
     expected_errors_.push_back("zone example.com/IN: has 0 SOA records");
     checkIssues();
 
+    // If null callback is specified, checkZone() only returns the final
+    // result.
+    ZoneCheckerCallbacks noerror_callbacks(
+        NULL, boost::bind(&ZoneCheckerTest::callback, this, _1, false));
+    EXPECT_FALSE(checkZone(zname_, zclass_, *rrsets_, noerror_callbacks));
+    checkIssues();
+
     // If there are more than 1 SOA RR, it's also an error.
     errors_.clear();
     soa_->addRdata(generic::SOA(soa_txt));
@@ -190,6 +193,13 @@ TEST_F(ZoneCheckerTest, checkNSData) {
     expected_warns_.push_back("zone example.com/IN: NS has no address");
     checkIssues();
 
+    // Same check, but disabling warning callback.  Same result, but without
+    // the warning.
+    ZoneCheckerCallbacks nowarn_callbacks(
+        boost::bind(&ZoneCheckerTest::callback, this, _1, true), NULL);
+    EXPECT_TRUE(checkZone(zname_, zclass_, *rrsets_, nowarn_callbacks));
+    checkIssues();
+
     // A tricky case: if the name matches a wildcard, it should technically
     // be considered valid, but this checker doesn't check that far and still
     // warns.

+ 17 - 16
src/lib/dns/zone_checker.h

@@ -34,18 +34,14 @@ public:
     /// Its parameter indicates the reason for the corresponding issue.
     typedef boost::function<void(const std::string& reason)> IssueCallback;
 
-    /// \brief Default constructor.
+    /// \brief Constructor.
     ///
-    /// This is a convenient shortcut to specify callbacks that do nothing.
-    /// If, for example, the caller of \c checkZone() is only interested in
-    /// the final result, it can use this constructor.
-    ///
-    /// \throw none
-    ZoneCheckerCallbacks() :
-        error_callback_(nullCallback), warn_callback_(nullCallback)
-    {}
-
-    /// \brief Constructor with callbacks.
+    /// Either or both of the callbacks can be empty, in which case the
+    /// corresponding callback will be effectively no-operation.  This can be
+    /// used, for example, when the caller of \c checkZone() is only
+    /// interested in the final result.  Note that a \c NULL pointer will be
+    /// implicitly converted to an empty functor object, so passing \c NULL
+    /// suffices.
     ///
     /// \throw none
     ///
@@ -63,7 +59,11 @@ public:
     /// thrown from the callback.
     ///
     /// \param reason Textual representation of the reason for the error.
-    void error(const std::string& reason) { error_callback_(reason); }
+    void error(const std::string& reason) {
+        if (!error_callback_.empty()) {
+            error_callback_(reason);
+        }
+    }
 
     /// \brief Call the callback for a non critical issue.
     ///
@@ -71,11 +71,12 @@ public:
     /// thrown from the callback.
     ///
     /// \param reason Textual representation of the reason for the issue.
-    void warn(const std::string& reason) { warn_callback_(reason); }
+    void warn(const std::string& reason) {
+        if (!warn_callback_.empty())
+            warn_callback_(reason);
+    }
 
 private:
-    static void nullCallback(const std::string&) {}
-
     IssueCallback error_callback_;
     IssueCallback warn_callback_;
 };
@@ -150,7 +151,7 @@ private:
 bool
 checkZone(const Name& zone_name, const RRClass& zone_class,
           const RRsetCollectionBase& zone_rrsets,
-          const ZoneCheckerCallbacks& callbacks = ZoneCheckerCallbacks());
+          const ZoneCheckerCallbacks& callbacks);
 
 } // namespace dns
 } // namespace isc