[#3074] addressed review comments

diff --git a/src/lib/dhcp/option_classless_static_route.cc b/src/lib/dhcp/option_classless_static_route.cc
index fa15b3670aa0cbb4c8bc17cc4b373dc174139495..edabeda0743697e8be32325a232be81a267704cc 100644 (file)
--- a/src/lib/dhcp/option_classless_static_route.cc
+++ b/src/lib/dhcp/option_classless_static_route.cc
@@ -235,12 +235,11 @@ OptionClasslessStaticRoute::parseConfigData(const std::string& config_txt) {
         }
 
         std::string txt_prefix_len = txt_subnet_prefix.substr(pos + 1);
-        int8_t prefix_len = 0;
+        int16_t prefix_len = 0;
         try {
-            // We should be able to lexically cast IPv4 prefix len to short int,
-            // and then downcast it to signed char. After that len<=32 check is
-            // also required.
-            prefix_len = boost::numeric_cast<int8_t>(boost::lexical_cast<int16_t>(txt_prefix_len));
+            // We should be able to lexically cast IPv4 prefix len to short int.
+            // After that len<=32 check is also required.
+            prefix_len = boost::lexical_cast<int16_t>(txt_prefix_len);
             if (prefix_len > 32) {
                 isc_throw(BadValue, "Provided IPv4 prefix len is out of 0-32 range.");
             }

diff --git a/src/lib/dhcp/option_definition.cc b/src/lib/dhcp/option_definition.cc
index 17e3be1a799ea0bb0cd57dd7ade91a2dcdfd09e3..2961faf222201968523a49d6ac9a87711d6a6a1a 100644 (file)
--- a/src/lib/dhcp/option_definition.cc
+++ b/src/lib/dhcp/option_definition.cc
@@ -187,7 +187,7 @@ OptionDefinition::optionFactory(Option::Universe u,
                                 uint16_t type,
                                 OptionBufferConstIter begin,
                                 OptionBufferConstIter end,
-                                bool custom_data) const {
+                                bool convenient_format) const {
 
     try {
         // Some of the options are represented by the specialized classes derived
@@ -196,7 +196,7 @@ OptionDefinition::optionFactory(Option::Universe u,
         // type to be returned. Therefore, we first check that if we are dealing
         // with such an option. If the instance is returned we just exit at this
         // point. If not, we will search for a generic option type to return.
-        OptionPtr option = factorySpecialFormatOption(u, begin, end, custom_data);
+        OptionPtr option = factorySpecialFormatOption(u, begin, end, convenient_format);
         if (option) {
             return (option);
         }
@@ -846,7 +846,7 @@ OptionPtr
 OptionDefinition::factorySpecialFormatOption(Option::Universe u,
                                              OptionBufferConstIter begin,
                                              OptionBufferConstIter end,
-                                             bool custom_data) const {
+                                             bool convenient_format) const {
     if ((u == Option::V6) && haveSpace(DHCP6_OPTION_SPACE)) {
         switch (getCode()) {
         case D6O_IA_NA:
@@ -908,7 +908,7 @@ OptionDefinition::factorySpecialFormatOption(Option::Universe u,
             return (OptionPtr(new Option4ClientFqdn(begin, end)));
 
         case DHO_CLASSLESS_STATIC_ROUTE:
-            return (OptionPtr(new OptionClasslessStaticRoute(begin, end, custom_data)));
+            return (OptionPtr(new OptionClasslessStaticRoute(begin, end, convenient_format)));
 
         case DHO_VIVCO_SUBOPTIONS:
             // Record of uint32 followed by binary.

diff --git a/src/lib/dhcp/option_definition.h b/src/lib/dhcp/option_definition.h
index dc991d3bbbd3c281ecdfcd4ea699645ed2f79130..12955308eba09665641ef3696d6e621fe9cbd835 100644 (file)
--- a/src/lib/dhcp/option_definition.h
+++ b/src/lib/dhcp/option_definition.h
@@ -433,9 +433,10 @@ public:
     /// @param type option type.
     /// @param begin beginning of the option buffer.
     /// @param end end of the option buffer.
-    /// @param custom_data flag letting know the factory that the buffer contains custom data.
-    ///                    Intended use case is @c OPT_CUSTOM_TYPE option def and csv-format=true.
-    ///                    Defaults to false.
+    /// @param convenient_format flag which indicates that the buffer contains option data
+    ///                          as a string formatted in user-friendly, convenient way.
+    ///                          The flag is propagated to the option constructor, so that
+    ///                          the data could be parsed properly. Defaults to false.
     ///
     /// @return instance of the DHCP option.
     /// @throw InvalidOptionValue if data for the option is invalid.
@@ -443,7 +444,7 @@ public:
                             uint16_t type,
                             OptionBufferConstIter begin,
                             OptionBufferConstIter end,
-                            bool custom_data = false) const;
+                            bool convenient_format = false) const;
 
     /// @brief Option factory.
     ///
@@ -675,10 +676,10 @@ private:
     /// @param u A universe (V4 or V6).
     /// @param begin beginning of the option buffer.
     /// @param end end of the option buffer.
-    /// @param custom_data flag letting know the factory that the buffer contains custom data.
-    ///                    Intended use case is @c OPT_CUSTOM_TYPE option def and csv-format=true.
-    ///                    Defaults to false.
-    ///
+    /// @param convenient_format flag which indicates that the buffer contains option data
+    ///                          as a string formatted in user-friendly, convenient way.
+    ///                          The flag is propagated to the option constructor, so that
+    ///                          the data could be parsed properly. Defaults to false.
     ///
     /// @return An instance of the option having special format or NULL if
     /// such an option can't be created because an option with the given
@@ -686,7 +687,7 @@ private:
     OptionPtr factorySpecialFormatOption(Option::Universe u,
                                          OptionBufferConstIter begin,
                                          OptionBufferConstIter end,
-                                         bool custom_data = false) const;
+                                         bool convenient_format = false) const;
 
     /// @brief Check if specified type matches option definition type.
     ///

diff --git a/src/lib/dhcp/tests/option_definition_unittest.cc b/src/lib/dhcp/tests/option_definition_unittest.cc
index c58071af6981af605a213775b5e5fc0df807e3a3..647533da6a5bf0a66cd0e067a065127c9ea71d05 100644 (file)
--- a/src/lib/dhcp/tests/option_definition_unittest.cc
+++ b/src/lib/dhcp/tests/option_definition_unittest.cc
@@ -2133,6 +2133,9 @@ TEST_F(OptionDefinitionTest, customOptionTypeString) {
     // Validate that option's fields were correctly parsed from strings.
     OptionClasslessStaticRoutePtr option_cast = boost::dynamic_pointer_cast<OptionClasslessStaticRoute>(option);
 
+    // Check that cast was successful.
+    ASSERT_TRUE(option_cast);
+
     // Expected len: 2 (option code + option len headers) + 5 (1 dest descriptor + 4 router addr)
     // + 9 (5 dest descriptor + 4 router addr) + 8 (4 dest descriptor + 4 router addr).
     EXPECT_EQ(24, option_cast->len());
@@ -2181,6 +2184,9 @@ TEST_F(OptionDefinitionTest, customOptionTypeBinary) {
     // Validate parsed values.
     OptionClasslessStaticRoutePtr option_cast = boost::dynamic_pointer_cast<OptionClasslessStaticRoute>(option);
 
+    // Check that cast was successful.
+    ASSERT_TRUE(option_cast);
+
     // Expected len: 2 (option code + option len headers) + 5 (1 dest descriptor + 4 router addr)
     // + 9 (5 dest descriptor + 4 router addr) + 8 (4 dest descriptor + 4 router addr).
     EXPECT_EQ(24, option_cast->len());