Browse Source

[5021] Changes after review:
- using const_pointer_cast in dhcp{4,6}/json_config_parser.cc
- removed obsolete comment from dhcp_parsers_unittest.cc
- updated simple_parser.h comments
- added include <vector> in simple_parser.h

Tomek Mrugalski 8 years ago
parent
commit
8e2c154501

+ 1 - 1
src/bin/dhcp4/json_config_parser.cc

@@ -566,7 +566,7 @@ configureDhcp4Server(Dhcpv4Srv&, isc::data::ConstElementPtr config_set) {
         // This is a way to convert ConstElementPtr to ElementPtr.
         // We need a config that can be edited, because we will insert
         // default values and will insert derived values as well.
-        ElementPtr mutable_cfg = Element::getMutableMap(config_set);
+        ElementPtr mutable_cfg = const_pointer_cast<Element>(config_set);
 
         // Set all default values if not specified by the user.
         SimpleParser4::setAllDefaults(mutable_cfg);

+ 1 - 1
src/bin/dhcp6/json_config_parser.cc

@@ -840,7 +840,7 @@ configureDhcp6Server(Dhcpv6Srv&, isc::data::ConstElementPtr config_set) {
         // This is a way to convert ConstElementPtr to ElementPtr.
         // We need a config that can be edited, because we will insert
         // default values and will insert derived values as well.
-        ElementPtr mutable_cfg = Element::getMutableMap(config_set);
+        ElementPtr mutable_cfg = const_pointer_cast<Element>(config_set);
 
         SimpleParser6::setAllDefaults(mutable_cfg);
 

+ 1 - 1
src/lib/cc/simple_parser.cc

@@ -123,7 +123,7 @@ size_t SimpleParser::setDefaults(isc::data::ElementPtr scope,
 
         // ... and insert it into the provided Element tree.
         scope->set(def_value.name_, x);
-        cnt++;
+        ++cnt;
     }
 
     return (cnt);

+ 15 - 10
src/lib/cc/simple_parser.h

@@ -8,6 +8,7 @@
 #define SIMPLE_PARSER_H
 
 #include <cc/data.h>
+#include <vector>
 #include <string>
 #include <stdint.h>
 
@@ -33,17 +34,20 @@ typedef std::vector<std::string> ParamsList;
 
 /// @brief A simple parser
 ///
-/// This class is intended to be a simpler replacement for
-/// @ref isc::dhcp::DhcpConfigParser.
+/// This class is intended to be a simpler replacement for @ref
+/// isc::dhcp::DhcpConfigParser. This class has been initially created to
+/// facilitate DHCPv4 and DHCPv6 servers' configuration parsing. Thus examples
+/// provided herein are related to DHCP configuration. Nevertheless, this is a
+/// generic class to be used in other modules too.
+///
 /// The simplification comes from several factors:
 /// - no build/commit nonsense. There's a single step:
 ///   CfgStorage parse(ConstElementPtr json)
 ///   that converts JSON configuration into an object and returns it.
-/// - almost no state kept. The only state kept in most cases is whether the
-///   parsing is done in v4 or v6 context. This greatly simplifies the
-///   parsers (no contexts, no child parsers list, no separate storage for
-///   uint32, strings etc. In fact, there's so little state kept, that this
-///   parser is mostly a collection of static methods.
+/// - no state kept. This greatly simplifies the parsers (no contexts, no child
+///   parsers list, no separate storage for uint32, strings etc. In fact,
+///   this base class is purely static. However, some derived classes may store
+///   some state. Implementors are advised to store as little state as possible.
 /// - no optional parameters (all are mandatory). This simplifies the parser,
 ///   but introduces a new step before parsing where we insert the default
 ///   values into client configuration before parsing. This is actually a good
@@ -98,14 +102,15 @@ class SimpleParser {
 
     /// @brief Utility method that returns position of an element
     ///
-    /// It's mostly useful for logging.
+    /// It's mostly useful for logging. When any necessary parameter is
+    /// missing (either parent is null or it doesn't contain specified
+    /// name) ZERO_POSITION is returned.
     ///
     /// @param name position of that element will be returned
     /// @param parent parent element (optional)
     /// @return position of the element specified.
     static const data::Element::Position&
-    getPosition(const std::string& name, const data::ConstElementPtr parent =
-                data::ConstElementPtr());
+    getPosition(const std::string& name, const data::ConstElementPtr parent);
 
  protected:
 

+ 0 - 1
src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc

@@ -428,7 +428,6 @@ public:
                           const SimpleDefaults& option_def_defaults) {
         size_t cnt = 0;
         // Set global defaults first.
-        /// @todo: Uncomment as part of the ticket 5019 work.
         cnt = SimpleParser::setDefaults(global, global_defaults);
 
         // Now set option defintion defaults for each specified option definition