From: Marcin Siodelski <marcin@isc.org>
Date: Mon, 28 Apr 2014 13:57:41 +0000 (+0200)
Subject: [3409] Append line number when adding duplicated option definition.
X-Git-Tag: trac3434_base~43^2~3
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=db9aaee9e45fd7b30d52b9aa523e36966d35a03a;p=thirdparty%2Fkea.git

[3409] Append line number when adding duplicated option definition.
---

diff --git a/src/lib/dhcpsrv/dhcp_parsers.cc b/src/lib/dhcpsrv/dhcp_parsers.cc
index 117979bb97..abf7c4421a 100644
--- a/src/lib/dhcpsrv/dhcp_parsers.cc
+++ b/src/lib/dhcpsrv/dhcp_parsers.cc
@@ -847,17 +847,13 @@ OptionDefListParser::build(ConstElementPtr option_def_list) {
         parser->build(option_def);
         parser->commit();
     }
-}
 
-void
-OptionDefListParser::commit() {
     CfgMgr& cfg_mgr = CfgMgr::instance();
     cfg_mgr.deleteOptionDefs();
 
     // We need to move option definitions from the temporary
     // storage to the storage.
-    std::list<std::string> space_names =
-    storage_->getOptionSpaceNames();
+    std::list<std::string> space_names = storage_->getOptionSpaceNames();
 
     BOOST_FOREACH(std::string space_name, space_names) {
         BOOST_FOREACH(OptionDefinitionPtr def,
@@ -866,11 +862,24 @@ OptionDefListParser::commit() {
             // values. The validation is expected to be made by the
             // OptionDefParser when creating an option definition.
             assert(def);
-            cfg_mgr.addOptionDef(def, space_name);
+            // The Config Manager may thrown an exception if the duplicated
+            // definition is being added. Catch the exceptions here to and
+            // append the position in the config.
+            try {
+                cfg_mgr.addOptionDef(def, space_name);
+            } catch (const std::exception& ex) {
+                isc_throw(DhcpConfigError, ex.what() << " ("
+                          << option_def_list->getPosition() << ")");
+            }
         }
     }
 }
 
+void
+OptionDefListParser::commit() {
+    // Do nothing.
+}
+
 //****************************** RelayInfoParser ********************************
 RelayInfoParser::RelayInfoParser(const std::string&,
                                  const isc::dhcp::Subnet::RelayInfoPtr& relay_info,
diff --git a/src/lib/dhcpsrv/dhcp_parsers.h b/src/lib/dhcpsrv/dhcp_parsers.h
index 28d3d81727..f6da862cd4 100644
--- a/src/lib/dhcpsrv/dhcp_parsers.h
+++ b/src/lib/dhcpsrv/dhcp_parsers.h
@@ -760,18 +760,21 @@ public:
 
     /// @brief Parse configuration entries.
     ///
-    /// This function parses configuration entries and creates instances
-    /// of option definitions.
+    /// This function parses configuration entries, creates instances
+    /// of option definitions and tries to add them to the Configuration
+    /// Manager.
     ///
     /// @param option_def_list pointer to an element that holds entries
     /// that define option definitions.
     /// @throw DhcpConfigError if configuration parsing fails.
     void build(isc::data::ConstElementPtr option_def_list);
 
-    /// @brief Stores option definitions in the CfgMgr.
+    /// @brief Commits option definitions.
+    ///
+    /// Currently this function is no-op, because option definitions are
+    /// added to the Configuration Manager in the @c build method.
     void commit();
 
-private:
     /// @brief storage for option definitions.
     OptionDefStoragePtr storage_;