BIND 10 trac2565, updated. 080d47deb3088adddd96434acfd1c82bd14779c9 [2565] Add rationale for why bool is returned instead of throwing an exception

BIND 10 source code commits bind10-changes at lists.isc.org
Thu Dec 27 11:41:58 UTC 2012


The branch, trac2565 has been updated
       via  080d47deb3088adddd96434acfd1c82bd14779c9 (commit)
       via  9f5102a30e4cb8da4697b3561206957b9a2aae05 (commit)
       via  0758ead7f4044e0867d4f953427b6c10f7afab45 (commit)
       via  9ab72138d43cb21a620c021a24e6a248b3dd77ab (commit)
       via  ffe973e3df51ab36e17087eda23b1042500d39b5 (commit)
       via  857d7fbf031d29b5b66ebe38fec725dde35008fd (commit)
       via  514b87c307fc834f39f0be155fc9bac9a6508d70 (commit)
       via  6d28690c67369927bb0806618f23533a1457eb6f (commit)
       via  029be672c7bf1b936859e811a17d45d04215ecd4 (commit)
      from  5ec876c98d10dfdb0675ffcde709b5ce9871465b (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 080d47deb3088adddd96434acfd1c82bd14779c9
Author: Mukund Sivaraman <muks at isc.org>
Date:   Thu Dec 27 17:02:19 2012 +0530

    [2565] Add rationale for why bool is returned instead of throwing an exception

commit 9f5102a30e4cb8da4697b3561206957b9a2aae05
Author: Mukund Sivaraman <muks at isc.org>
Date:   Thu Dec 27 16:58:26 2012 +0530

    [2565] Rename constructor arguments to look nicer

commit 0758ead7f4044e0867d4f953427b6c10f7afab45
Author: Mukund Sivaraman <muks at isc.org>
Date:   Thu Dec 27 16:56:11 2012 +0530

    [2565] Update textToTypeCode() to match textToClassCode()

commit 9ab72138d43cb21a620c021a24e6a248b3dd77ab
Author: Mukund Sivaraman <muks at isc.org>
Date:   Thu Dec 27 16:55:08 2012 +0530

    [2565] Rename function argument

commit ffe973e3df51ab36e17087eda23b1042500d39b5
Author: Mukund Sivaraman <muks at isc.org>
Date:   Thu Dec 27 16:54:40 2012 +0530

    [2565] Fix indenting and untabify

commit 857d7fbf031d29b5b66ebe38fec725dde35008fd
Author: Mukund Sivaraman <muks at isc.org>
Date:   Thu Dec 27 16:38:37 2012 +0530

    [2565] Fix exception message returned when classes don't match

commit 514b87c307fc834f39f0be155fc9bac9a6508d70
Author: Mukund Sivaraman <muks at isc.org>
Date:   Thu Dec 27 16:30:01 2012 +0530

    [2565] Throw InternalException instead of isc::BadValue

commit 6d28690c67369927bb0806618f23533a1457eb6f
Author: Mukund Sivaraman <muks at isc.org>
Date:   Thu Dec 27 16:27:53 2012 +0530

    [2565] Use the wrapped RRClass directly, in the comparison

commit 029be672c7bf1b936859e811a17d45d04215ecd4
Author: Mukund Sivaraman <muks at isc.org>
Date:   Thu Dec 27 16:25:10 2012 +0530

    [2565] Fix grammar in methods' documentation

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

Summary of changes:
 src/lib/dns/master_loader.cc                |    8 +++----
 src/lib/dns/rrclass-placeholder.h           |    6 ++---
 src/lib/dns/rrclass.cc                      |   11 ++++-----
 src/lib/dns/rrparamregistry-placeholder.cc  |   23 ++++++++-----------
 src/lib/dns/rrparamregistry.h               |   33 ++++++++++++++++++---------
 src/lib/dns/rrttl.h                         |    2 +-
 src/lib/dns/rrtype.cc                       |    7 ++++--
 src/lib/dns/tests/master_loader_unittest.cc |    3 ++-
 8 files changed, 50 insertions(+), 43 deletions(-)

-----------------------------------------------------------------------
diff --git a/src/lib/dns/master_loader.cc b/src/lib/dns/master_loader.cc
index 4135a3b..4abc15e 100644
--- a/src/lib/dns/master_loader.cc
+++ b/src/lib/dns/master_loader.cc
@@ -213,11 +213,9 @@ private:
         const MaybeRRClass rrclass =
             RRClass::createFromText(rrparam_token.getString());
         if (rrclass) {
-            if (rrclass != zone_class_) {
-                // It doesn't really matter much what type of exception
-                // we throw, we catch it just below.
-                isc_throw(isc::BadValue, "Class mismatch: " << rrclass <<
-                          "vs. " << zone_class_);
+            if (*rrclass != zone_class_) {
+                isc_throw(InternalException, "Class mismatch: " << *rrclass <<
+                          " vs. " << zone_class_);
             }
             rrparam_token = lexer_.getNextToken(MasterToken::STRING);
         }
diff --git a/src/lib/dns/rrclass-placeholder.h b/src/lib/dns/rrclass-placeholder.h
index 4573738..1ff4163 100644
--- a/src/lib/dns/rrclass-placeholder.h
+++ b/src/lib/dns/rrclass-placeholder.h
@@ -135,8 +135,8 @@ public:
     /// If the given string is not recognized as a valid representation of
     /// an RR class, an exception of class \c InvalidRRClass will be thrown.
     ///
-    /// \param classstr A string representation of the \c RRClass
-    explicit RRClass(const std::string& classstr);
+    /// \param class_str A string representation of the \c RRClass
+    explicit RRClass(const std::string& class_str);
     /// Constructor from wire-format data.
     ///
     /// The \c buffer parameter normally stores a complete DNS message
@@ -173,7 +173,7 @@ public:
     /// One main purpose of this function is to minimize the overhead
     /// when the given text does not represent a valid RR class.  For
     /// this reason this function intentionally omits the capability of
-    /// delivering details reason for the parse failure, such as in the
+    /// delivering a detailed 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
diff --git a/src/lib/dns/rrclass.cc b/src/lib/dns/rrclass.cc
index 70907bd..4874a9b 100644
--- a/src/lib/dns/rrclass.cc
+++ b/src/lib/dns/rrclass.cc
@@ -30,12 +30,11 @@ using namespace isc::util;
 namespace isc {
 namespace dns {
 
-RRClass::RRClass(const std::string& classstr) {
-     if (!RRParamRegistry::getRegistry().textToClassCode(classstr,
-                                                         classcode_)) {
-          isc_throw(InvalidRRClass,
-                    "Unrecognized RR parameter string: " + classstr);
-     }
+RRClass::RRClass(const std::string& class_str) {
+    if (!RRParamRegistry::getRegistry().textToClassCode(class_str, classcode_)) {
+        isc_throw(InvalidRRClass,
+                  "Unrecognized RR parameter string: " + class_str);
+    }
 }
 
 RRClass::RRClass(InputBuffer& buffer) {
diff --git a/src/lib/dns/rrparamregistry-placeholder.cc b/src/lib/dns/rrparamregistry-placeholder.cc
index 9729d86..5960759 100644
--- a/src/lib/dns/rrparamregistry-placeholder.cc
+++ b/src/lib/dns/rrparamregistry-placeholder.cc
@@ -423,12 +423,12 @@ removeParam(uint16_t code, MC& codemap, MS& stringmap) {
 
 template <typename PT, typename MS>
 inline bool
-textToCode(const string& code_str, MS& stringmap, uint16_t& class_code) {
+textToCode(const string& code_str, MS& stringmap, uint16_t& ret_code) {
     typename MS::const_iterator found;
 
     found = stringmap.find(code_str);
     if (found != stringmap.end()) {
-        class_code = found->second->code_;
+        ret_code = found->second->code_;
         return (true);
     }
 
@@ -442,7 +442,7 @@ textToCode(const string& code_str, MS& stringmap, uint16_t& class_code) {
                                           l - PT::UNKNOWN_PREFIXLEN()));
         iss >> dec >> code;
         if (iss.rdstate() == ios::eofbit && code <= PT::MAX_CODE) {
-            class_code = code;
+            ret_code = code;
             return (true);
         }
     }
@@ -478,17 +478,12 @@ RRParamRegistry::removeType(uint16_t code) {
                                                      impl_->str2typemap));
 }
 
-uint16_t
-RRParamRegistry::textToTypeCode(const string& type_string) const {
-    uint16_t code;
-
-    if (!textToCode<RRTypeParam, StrRRTypeMap>
-        (type_string, impl_->str2typemap, code)) {
-         isc_throw(InvalidRRType,
-                   "Unrecognized RR parameter string: " + type_string);
-    }
-
-    return (code);
+bool
+RRParamRegistry::textToTypeCode(const string& type_string,
+                                uint16_t& type_code) const
+{
+    return (textToCode<RRTypeParam, StrRRTypeMap>
+            (type_string, impl_->str2typemap, type_code));
 }
 
 string
diff --git a/src/lib/dns/rrparamregistry.h b/src/lib/dns/rrparamregistry.h
index 6924e05..dfa4463 100644
--- a/src/lib/dns/rrparamregistry.h
+++ b/src/lib/dns/rrparamregistry.h
@@ -384,16 +384,23 @@ public:
     /// \brief Convert a textual representation of an RR type to the
     /// corresponding 16-bit integer code.
     ///
-    /// This method searches the \c RRParamRegistry for the mapping from the
-    /// given textual representation of RR type to the corresponding integer
-    /// code.  If a mapping is found, it returns the associated type code;
-    /// otherwise, if the given string is in the form of "TYPEnnnn", it returns
-    /// the corresponding number as the type code; otherwise, it throws an
-    /// exception of class \c InvalidRRType.
+    /// This method searches the \c RRParamRegistry for the mapping from
+    /// the given textual representation of RR type to the corresponding
+    /// integer code. If a mapping is found, it returns true with the
+    /// associated type code in \c type_code; otherwise, if the given
+    /// string is in the form of "TYPEnnnn", it returns true with the
+    /// corresponding number as the type code in \c type_code;
+    /// otherwise, it returns false and \c type_code is untouched.
+    ///
+    /// We return \c false and avoid simply throwing an exception in the
+    /// case of an error so that the code is performant in some
+    /// situations.
     ///
     /// \param type_string The textual representation of the RR type.
-    /// \return The RR type code for \c type_string.
-    uint16_t textToTypeCode(const std::string& type_string) const;
+    /// \param type_code Returns the RR type code in this argument.
+    /// \return true if conversion is successful, false otherwise.
+    bool textToTypeCode(const std::string& type_string,
+                        uint16_t& type_code) const;
 
     /// \brief Convert type code into its textual representation.
     ///
@@ -420,15 +427,19 @@ public:
     /// corresponding integer code.  If a mapping is found, it returns
     /// true with the associated class code in \c class_code; otherwise,
     /// if the given string is in the form of "CLASSnnnn", it returns
-    /// true with the corresponding number as the class code in \c
-    /// class_code; otherwise, it returns false and \c class_code is
+    /// true with the corresponding number as the class code in
+    /// \c class_code; otherwise, it returns false and \c class_code is
     /// untouched.
     ///
+    /// We return \c false and avoid simply throwing an exception in the
+    /// case of an error so that the code is performant in some
+    /// situations.
+    ///
     /// \param class_string The textual representation of the RR class.
     /// \param class_code Returns the RR class code in this argument.
     /// \return true if conversion is successful, false otherwise.
     bool textToClassCode(const std::string& class_string,
-			 uint16_t& class_code) const;
+                         uint16_t& class_code) const;
 
     /// \brief Convert class code into its textual representation.
     ///
diff --git a/src/lib/dns/rrttl.h b/src/lib/dns/rrttl.h
index 7904d84..23d57f4 100644
--- a/src/lib/dns/rrttl.h
+++ b/src/lib/dns/rrttl.h
@@ -133,7 +133,7 @@ public:
     /// 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()
+    /// a detailed 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
diff --git a/src/lib/dns/rrtype.cc b/src/lib/dns/rrtype.cc
index 4ef4e67..fd981f2 100644
--- a/src/lib/dns/rrtype.cc
+++ b/src/lib/dns/rrtype.cc
@@ -31,8 +31,11 @@ using isc::dns::RRType;
 namespace isc {
 namespace dns {
 
-RRType::RRType(const std::string& typestr) {
-    typecode_ = RRParamRegistry::getRegistry().textToTypeCode(typestr);
+RRType::RRType(const std::string& type_str) {
+    if (!RRParamRegistry::getRegistry().textToTypeCode(type_str, typecode_)) {
+        isc_throw(InvalidRRType,
+                  "Unrecognized RR parameter string: " + type_str);
+    }
 }
 
 RRType::RRType(InputBuffer& buffer) {
diff --git a/src/lib/dns/tests/master_loader_unittest.cc b/src/lib/dns/tests/master_loader_unittest.cc
index 667706a..3f3314c 100644
--- a/src/lib/dns/tests/master_loader_unittest.cc
+++ b/src/lib/dns/tests/master_loader_unittest.cc
@@ -391,7 +391,8 @@ struct ErrorCase {
       "Missing Rdata" },
 
     { "www      3600    IN", NULL, "Unexpected EOLN" },
-    { "www      3600    CH  TXT nothing", NULL, "Class mismatch" },
+    { "www      3600    CH  TXT nothing", "Class mismatch: CH vs. IN",
+      "Class mismatch" },
     { "www      \"3600\"  IN  A   192.0.2.1", NULL, "Quoted TTL" },
     { "www      3600    \"IN\"  A   192.0.2.1", NULL, "Quoted class" },
     { "www      3600    IN  \"A\"   192.0.2.1", NULL, "Quoted type" },



More information about the bind10-changes mailing list