From a0164fae3100565ee1f22cfe60aeec6b1b8e213a Mon Sep 17 00:00:00 2001 From: Marcin Siodelski Date: Tue, 16 Sep 2014 20:57:07 +0200 Subject: [PATCH] [3588] Return pointer to the RW option definition configs, not a reference. --- src/bin/dhcp4/dhcp4_srv.cc | 2 +- src/bin/dhcp4/tests/config_parser_unittest.cc | 72 ++++++++++--------- .../dhcp4/tests/kea_controller_unittest.cc | 2 + src/bin/dhcp6/dhcp6_srv.cc | 4 +- src/bin/dhcp6/tests/config_parser_unittest.cc | 69 ++++++++++-------- .../dhcp6/tests/kea_controller_unittest.cc | 2 + src/lib/dhcpsrv/cfg_option_def.h | 10 +++ src/lib/dhcpsrv/dhcp_parsers.cc | 11 ++- src/lib/dhcpsrv/srv_config.cc | 8 +-- src/lib/dhcpsrv/srv_config.h | 26 +++++-- .../dhcpsrv/tests/dhcp_parsers_unittest.cc | 2 +- 11 files changed, 127 insertions(+), 81 deletions(-) diff --git a/src/bin/dhcp4/dhcp4_srv.cc b/src/bin/dhcp4/dhcp4_srv.cc index 29f7dda0a..6d2d3a7c6 100644 --- a/src/bin/dhcp4/dhcp4_srv.cc +++ b/src/bin/dhcp4/dhcp4_srv.cc @@ -1807,7 +1807,7 @@ Dhcpv4Srv::unpackOptions(const OptionBuffer& buf, option_defs = LibDHCP::getOptionDefs(Option::V4); } else if (!option_space.empty()) { OptionDefContainerPtr option_defs_ptr = CfgMgr::instance() - .getCurrentCfg()->getCfgOptionDef().getAll(option_space); + .getCurrentCfg()->getCfgOptionDef()->getAll(option_space); if (option_defs_ptr != NULL) { option_defs = *option_defs_ptr; } diff --git a/src/bin/dhcp4/tests/config_parser_unittest.cc b/src/bin/dhcp4/tests/config_parser_unittest.cc index 73a38a7e9..bc1b8d2ce 100644 --- a/src/bin/dhcp4/tests/config_parser_unittest.cc +++ b/src/bin/dhcp4/tests/config_parser_unittest.cc @@ -80,6 +80,7 @@ public: srv_.reset(new Dhcpv4Srv(0)); // Create fresh context. globalContext()->copyContext(ParserContext(Option::V4)); + resetConfiguration(); } // Check that no hooks libraries are loaded. This is a pre-condition for @@ -444,8 +445,6 @@ public: void resetConfiguration() { string config = "{ \"interfaces\": [ \"*\" ]," "\"hooks-libraries\": [ ], " - "\"rebind-timer\": 2000, " - "\"renew-timer\": 1000, " "\"valid-lifetime\": 4000, " "\"subnet4\": [ ], " "\"dhcp-ddns\": { \"enable-updates\" : false }, " @@ -453,6 +452,7 @@ public: "\"option-data\": [ ] }"; static_cast(executeConfiguration(config, "reset configuration database")); + CfgMgr::instance().clear(); } @@ -1231,8 +1231,8 @@ TEST_F(Dhcp4ParserTest, optionDefIpv4Address) { ElementPtr json = Element::fromJSON(config); // Make sure that the particular option definition does not exist. - OptionDefinitionPtr def = CfgMgr::instance().getCurrentCfg() - ->getCfgOptionDef().get("isc", 100); + OptionDefinitionPtr def = CfgMgr::instance().getStagingCfg()-> + getCfgOptionDef()->get("isc", 100); ASSERT_FALSE(def); // Use the configuration string to create new option definition. @@ -1242,7 +1242,7 @@ TEST_F(Dhcp4ParserTest, optionDefIpv4Address) { checkResult(status, 0); // The option definition should now be available in the CfgMgr. - def = CfgMgr::instance().getCurrentCfg()->getCfgOptionDef().get("isc", 100); + def = CfgMgr::instance().getStagingCfg()->getCfgOptionDef()->get("isc", 100); ASSERT_TRUE(def); // Verify that the option definition data is valid. @@ -1273,8 +1273,8 @@ TEST_F(Dhcp4ParserTest, optionDefRecord) { ElementPtr json = Element::fromJSON(config); // Make sure that the particular option definition does not exist. - OptionDefinitionPtr def = CfgMgr::instance().getCurrentCfg()-> - getCfgOptionDef().get("isc", 100); + OptionDefinitionPtr def = CfgMgr::instance().getStagingCfg()-> + getCfgOptionDef()->get("isc", 100); ASSERT_FALSE(def); // Use the configuration string to create new option definition. @@ -1284,7 +1284,7 @@ TEST_F(Dhcp4ParserTest, optionDefRecord) { checkResult(status, 0); // The option definition should now be available in the CfgMgr. - def = CfgMgr::instance().getCurrentCfg()->getCfgOptionDef().get("isc", 100); + def = CfgMgr::instance().getStagingCfg()->getCfgOptionDef()->get("isc", 100); ASSERT_TRUE(def); // Check the option data. @@ -1332,10 +1332,10 @@ TEST_F(Dhcp4ParserTest, optionDefMultiple) { ElementPtr json = Element::fromJSON(config); // Make sure that the option definitions do not exist yet. - ASSERT_FALSE(CfgMgr::instance().getCurrentCfg()-> - getCfgOptionDef().get("isc", 100)); - ASSERT_FALSE(CfgMgr::instance().getCurrentCfg()-> - getCfgOptionDef().get("isc", 101)); + ASSERT_FALSE(CfgMgr::instance().getStagingCfg()-> + getCfgOptionDef()->get("isc", 100)); + ASSERT_FALSE(CfgMgr::instance().getStagingCfg()-> + getCfgOptionDef()->get("isc", 101)); // Use the configuration string to create new option definitions. ConstElementPtr status; @@ -1344,8 +1344,8 @@ TEST_F(Dhcp4ParserTest, optionDefMultiple) { checkResult(status, 0); // Check the first definition we have created. - OptionDefinitionPtr def1 = CfgMgr::instance().getCurrentCfg()-> - getCfgOptionDef().get("isc", 100); + OptionDefinitionPtr def1 = CfgMgr::instance().getStagingCfg()-> + getCfgOptionDef()->get("isc", 100); ASSERT_TRUE(def1); // Check the option data. @@ -1356,8 +1356,8 @@ TEST_F(Dhcp4ParserTest, optionDefMultiple) { EXPECT_TRUE(def1->getEncapsulatedSpace().empty()); // Check the second option definition we have created. - OptionDefinitionPtr def2 = CfgMgr::instance().getCurrentCfg()-> - getCfgOptionDef().get("isc", 101); + OptionDefinitionPtr def2 = CfgMgr::instance().getStagingCfg()-> + getCfgOptionDef()->get("isc", 101); ASSERT_TRUE(def2); // Check the option data. @@ -1398,8 +1398,8 @@ TEST_F(Dhcp4ParserTest, optionDefDuplicate) { ElementPtr json = Element::fromJSON(config); // Make sure that the option definition does not exist yet. - ASSERT_FALSE(CfgMgr::instance().getCurrentCfg()-> - getCfgOptionDef().get("isc", 100)); + ASSERT_FALSE(CfgMgr::instance().getStagingCfg()-> + getCfgOptionDef()->get("isc", 100)); // Use the configuration string to create new option definitions. ConstElementPtr status; @@ -1429,8 +1429,8 @@ TEST_F(Dhcp4ParserTest, optionDefArray) { ElementPtr json = Element::fromJSON(config); // Make sure that the particular option definition does not exist. - OptionDefinitionPtr def = CfgMgr::instance().getCurrentCfg()-> - getCfgOptionDef().get("isc", 100); + OptionDefinitionPtr def = CfgMgr::instance().getStagingCfg()-> + getCfgOptionDef()->get("isc", 100); ASSERT_FALSE(def); // Use the configuration string to create new option definition. @@ -1440,8 +1440,8 @@ TEST_F(Dhcp4ParserTest, optionDefArray) { checkResult(status, 0); // The option definition should now be available in the CfgMgr. - def = CfgMgr::instance().getCurrentCfg()-> - getCfgOptionDef().get("isc", 100); + def = CfgMgr::instance().getStagingCfg()-> + getCfgOptionDef()->get("isc", 100); ASSERT_TRUE(def); // Check the option data. @@ -1472,8 +1472,8 @@ TEST_F(Dhcp4ParserTest, optionDefEncapsulate) { ElementPtr json = Element::fromJSON(config); // Make sure that the particular option definition does not exist. - OptionDefinitionPtr def = CfgMgr::instance().getCurrentCfg()-> - getCfgOptionDef().get("isc", 100); + OptionDefinitionPtr def = CfgMgr::instance().getStagingCfg()-> + getCfgOptionDef()->get("isc", 100); ASSERT_FALSE(def); // Use the configuration string to create new option definition. @@ -1483,8 +1483,8 @@ TEST_F(Dhcp4ParserTest, optionDefEncapsulate) { checkResult(status, 0); // The option definition should now be available in the CfgMgr. - def = CfgMgr::instance().getCurrentCfg()-> - getCfgOptionDef().get("isc", 100); + def = CfgMgr::instance().getStagingCfg()-> + getCfgOptionDef()->get("isc", 100); ASSERT_TRUE(def); // Check the option data. @@ -1681,8 +1681,8 @@ TEST_F(Dhcp4ParserTest, optionStandardDefOverride) { "}"; ElementPtr json = Element::fromJSON(config); - OptionDefinitionPtr def = CfgMgr::instance().getCurrentCfg()-> - getCfgOptionDef().get("dhcp4", 109); + OptionDefinitionPtr def = CfgMgr::instance().getStagingCfg()-> + getCfgOptionDef()->get("dhcp4", 109); ASSERT_FALSE(def); // Use the configuration string to create new option definition. @@ -1692,8 +1692,8 @@ TEST_F(Dhcp4ParserTest, optionStandardDefOverride) { checkResult(status, 0); // The option definition should now be available in the CfgMgr. - def = CfgMgr::instance().getCurrentCfg()-> - getCfgOptionDef().get("dhcp4", 109); + def = CfgMgr::instance().getStagingCfg()-> + getCfgOptionDef()->get("dhcp4", 109); ASSERT_TRUE(def); // Check the option data. @@ -1749,8 +1749,8 @@ TEST_F(Dhcp4ParserTest, optionStandardDefOverride) { // Expecting success. checkResult(status, 0); - def = CfgMgr::instance().getCurrentCfg()-> - getCfgOptionDef().get("dhcp4", 65); + def = CfgMgr::instance().getStagingCfg()-> + getCfgOptionDef()->get("dhcp4", 65); ASSERT_TRUE(def); // Check the option data. @@ -1964,6 +1964,8 @@ TEST_F(Dhcp4ParserTest, optionDataEncapsulate) { ASSERT_TRUE(status); checkResult(status, 0); + CfgMgr::instance().clear(); + // Stage 2. Configure base option and a subnet. Please note that // the configuration from the stage 2 is repeated because BIND // configuration manager sends whole configuration for the lists @@ -2538,6 +2540,8 @@ TEST_F(Dhcp4ParserTest, stdOptionDataEncapsulate) { ASSERT_TRUE(status); checkResult(status, 0); + CfgMgr::instance().clear(); + // Once the definitions have been added we can configure the // standard option #17. This option comprises an enterprise // number and sub options. By convention (introduced in @@ -2893,6 +2897,10 @@ TEST_F(Dhcp4ParserTest, LibrariesSpecified) { EXPECT_TRUE(checkMarkerFile(LOAD_MARKER_FILE, "12")); EXPECT_FALSE(checkMarkerFileExists(UNLOAD_MARKER_FILE)); + // Commit the changes so as we get the fresh configuration for the + // second part of this test. + CfgMgr::instance().commit(); + // Unload the libraries. The load file should not have changed, but // the unload one should indicate the unload() functions have been run. config = buildHooksLibrariesConfig(); diff --git a/src/bin/dhcp4/tests/kea_controller_unittest.cc b/src/bin/dhcp4/tests/kea_controller_unittest.cc index 5bfb3b099..38bb36b36 100644 --- a/src/bin/dhcp4/tests/kea_controller_unittest.cc +++ b/src/bin/dhcp4/tests/kea_controller_unittest.cc @@ -18,6 +18,7 @@ #include #include #include +#include #include #include @@ -57,6 +58,7 @@ public: } ~JSONFileBackendTest() { + isc::log::resetUnitTestRootLogger(); static_cast(unlink(TEST_FILE)); }; diff --git a/src/bin/dhcp6/dhcp6_srv.cc b/src/bin/dhcp6/dhcp6_srv.cc index 637e4015a..08e25ee9d 100644 --- a/src/bin/dhcp6/dhcp6_srv.cc +++ b/src/bin/dhcp6/dhcp6_srv.cc @@ -2444,8 +2444,8 @@ Dhcpv6Srv::unpackOptions(const OptionBuffer& buf, option_defs = LibDHCP::getOptionDefs(Option::V6); } else if (!option_space.empty()) { OptionDefContainerPtr option_defs_ptr = - CfgMgr::instance().getCurrentCfg()->getCfgOptionDef() - .getAll(option_space); + CfgMgr::instance().getCurrentCfg()->getCfgOptionDef()-> + getAll(option_space); if (option_defs_ptr != NULL) { option_defs = *option_defs_ptr; } diff --git a/src/bin/dhcp6/tests/config_parser_unittest.cc b/src/bin/dhcp6/tests/config_parser_unittest.cc index e9281a65f..082cc7209 100644 --- a/src/bin/dhcp6/tests/config_parser_unittest.cc +++ b/src/bin/dhcp6/tests/config_parser_unittest.cc @@ -302,6 +302,7 @@ public: try { ElementPtr json = Element::fromJSON(config); status = configureDhcp6Server(srv_, json); + } catch (const std::exception& ex) { ADD_FAILURE() << "Unable to " << operation << ". " << "The following configuration was used: " << std::endl @@ -382,6 +383,7 @@ public: EXPECT_NO_THROW(x = configureDhcp6Server(srv_, json)); checkResult(x, 1); EXPECT_TRUE(errorContainsPosition(x, "")); + CfgMgr::instance().clear(); } /// @brief Test invalid option paramater value. @@ -399,6 +401,7 @@ public: EXPECT_NO_THROW(x = configureDhcp6Server(srv_, json)); checkResult(x, 1); EXPECT_TRUE(errorContainsPosition(x, "")); + CfgMgr::instance().clear(); } /// @brief Test option against given code and data. @@ -468,11 +471,13 @@ public: const size_t expected_data_len) { std::string config = createConfigWithOption(params); ASSERT_TRUE(executeConfiguration(config, "parse option configuration")); + // The subnet should now hold one option with the specified code. Subnet::OptionDescriptor desc = getOptionFromSubnet(IOAddress("2001:db8:1::5"), option_code); ASSERT_TRUE(desc.option); testOption(desc, option_code, expected_data, expected_data_len); + CfgMgr::instance().clear(); } int rcode_; ///< Return code (see @ref isc::config::parseAnswer) @@ -1466,8 +1471,8 @@ TEST_F(Dhcp6ParserTest, optionDefIpv6Address) { ElementPtr json = Element::fromJSON(config); // Make sure that the particular option definition does not exist. - OptionDefinitionPtr def = CfgMgr::instance().getCurrentCfg()-> - getCfgOptionDef().get("isc", 100); + OptionDefinitionPtr def = CfgMgr::instance().getStagingCfg()-> + getCfgOptionDef()->get("isc", 100); ASSERT_FALSE(def); // Use the configuration string to create new option definition. @@ -1476,7 +1481,7 @@ TEST_F(Dhcp6ParserTest, optionDefIpv6Address) { ASSERT_TRUE(status); // The option definition should now be available in the CfgMgr. - def = CfgMgr::instance().getCurrentCfg()->getCfgOptionDef().get("isc", 100); + def = CfgMgr::instance().getStagingCfg()->getCfgOptionDef()->get("isc", 100); ASSERT_TRUE(def); // Verify that the option definition data is valid. @@ -1506,8 +1511,8 @@ TEST_F(Dhcp6ParserTest, optionDefRecord) { ElementPtr json = Element::fromJSON(config); // Make sure that the particular option definition does not exist. - OptionDefinitionPtr def = CfgMgr::instance().getCurrentCfg()-> - getCfgOptionDef().get("isc", 100); + OptionDefinitionPtr def = CfgMgr::instance().getStagingCfg()-> + getCfgOptionDef()->get("isc", 100); ASSERT_FALSE(def); // Use the configuration string to create new option definition. @@ -1517,7 +1522,7 @@ TEST_F(Dhcp6ParserTest, optionDefRecord) { checkResult(status, 0); // The option definition should now be available in the CfgMgr. - def = CfgMgr::instance().getCurrentCfg()->getCfgOptionDef().get("isc", 100); + def = CfgMgr::instance().getStagingCfg()->getCfgOptionDef()->get("isc", 100); ASSERT_TRUE(def); // Check the option data. @@ -1564,10 +1569,10 @@ TEST_F(Dhcp6ParserTest, optionDefMultiple) { ElementPtr json = Element::fromJSON(config); // Make sure that the option definitions do not exist yet. - ASSERT_FALSE(CfgMgr::instance().getCurrentCfg()-> - getCfgOptionDef().get("isc", 100)); - ASSERT_FALSE(CfgMgr::instance().getCurrentCfg()-> - getCfgOptionDef().get("isc", 101)); + ASSERT_FALSE(CfgMgr::instance().getStagingCfg()-> + getCfgOptionDef()->get("isc", 100)); + ASSERT_FALSE(CfgMgr::instance().getStagingCfg()-> + getCfgOptionDef()->get("isc", 101)); // Use the configuration string to create new option definitions. ConstElementPtr status; @@ -1576,8 +1581,8 @@ TEST_F(Dhcp6ParserTest, optionDefMultiple) { checkResult(status, 0); // Check the first definition we have created. - OptionDefinitionPtr def1 = CfgMgr::instance().getCurrentCfg()-> - getCfgOptionDef().get("isc", 100); + OptionDefinitionPtr def1 = CfgMgr::instance().getStagingCfg()-> + getCfgOptionDef()->get("isc", 100); ASSERT_TRUE(def1); // Check the option data. @@ -1587,8 +1592,8 @@ TEST_F(Dhcp6ParserTest, optionDefMultiple) { EXPECT_FALSE(def1->getArrayType()); // Check the second option definition we have created. - OptionDefinitionPtr def2 = CfgMgr::instance().getCurrentCfg()-> - getCfgOptionDef().get("isc", 101); + OptionDefinitionPtr def2 = CfgMgr::instance().getStagingCfg()-> + getCfgOptionDef()->get("isc", 101); ASSERT_TRUE(def2); // Check the option data. @@ -1628,8 +1633,8 @@ TEST_F(Dhcp6ParserTest, optionDefDuplicate) { ElementPtr json = Element::fromJSON(config); // Make sure that the option definition does not exist yet. - ASSERT_FALSE(CfgMgr::instance().getCurrentCfg()-> - getCfgOptionDef().get("isc", 100)); + ASSERT_FALSE(CfgMgr::instance().getStagingCfg()-> + getCfgOptionDef()->get("isc", 100)); // Use the configuration string to create new option definitions. ConstElementPtr status; @@ -1659,8 +1664,8 @@ TEST_F(Dhcp6ParserTest, optionDefArray) { ElementPtr json = Element::fromJSON(config); // Make sure that the particular option definition does not exist. - OptionDefinitionPtr def = CfgMgr::instance().getCurrentCfg()-> - getCfgOptionDef().get("isc", 100); + OptionDefinitionPtr def = CfgMgr::instance().getStagingCfg()-> + getCfgOptionDef()->get("isc", 100); ASSERT_FALSE(def); // Use the configuration string to create new option definition. @@ -1670,7 +1675,7 @@ TEST_F(Dhcp6ParserTest, optionDefArray) { checkResult(status, 0); // The option definition should now be available in the CfgMgr. - def = CfgMgr::instance().getCurrentCfg()->getCfgOptionDef().get("isc", 100); + def = CfgMgr::instance().getStagingCfg()->getCfgOptionDef()->get("isc", 100); ASSERT_TRUE(def); // Check the option data. @@ -1700,8 +1705,8 @@ TEST_F(Dhcp6ParserTest, optionDefEncapsulate) { ElementPtr json = Element::fromJSON(config); // Make sure that the particular option definition does not exist. - OptionDefinitionPtr def = CfgMgr::instance().getCurrentCfg()-> - getCfgOptionDef().get("isc", 100); + OptionDefinitionPtr def = CfgMgr::instance().getStagingCfg()-> + getCfgOptionDef()->get("isc", 100); ASSERT_FALSE(def); // Use the configuration string to create new option definition. @@ -1711,7 +1716,7 @@ TEST_F(Dhcp6ParserTest, optionDefEncapsulate) { checkResult(status, 0); // The option definition should now be available in the CfgMgr. - def = CfgMgr::instance().getCurrentCfg()->getCfgOptionDef().get("isc", 100); + def = CfgMgr::instance().getStagingCfg()->getCfgOptionDef()->get("isc", 100); ASSERT_TRUE(def); // Check the option data. @@ -1909,8 +1914,8 @@ TEST_F(Dhcp6ParserTest, optionStandardDefOverride) { "}"; ElementPtr json = Element::fromJSON(config); - OptionDefinitionPtr def = CfgMgr::instance().getCurrentCfg()-> - getCfgOptionDef().get("dhcp6", 100); + OptionDefinitionPtr def = CfgMgr::instance().getStagingCfg()-> + getCfgOptionDef()->get("dhcp6", 100); ASSERT_FALSE(def); // Use the configuration string to create new option definition. @@ -1920,8 +1925,8 @@ TEST_F(Dhcp6ParserTest, optionStandardDefOverride) { checkResult(status, 0); // The option definition should now be available in the CfgMgr. - def = CfgMgr::instance().getCurrentCfg()-> - getCfgOptionDef().get("dhcp6", 100); + def = CfgMgr::instance().getStagingCfg()-> + getCfgOptionDef()->get("dhcp6", 100); ASSERT_TRUE(def); // Check the option data. @@ -1977,8 +1982,8 @@ TEST_F(Dhcp6ParserTest, optionStandardDefOverride) { // Expecting success. checkResult(status, 0); - def = CfgMgr::instance().getCurrentCfg()-> - getCfgOptionDef().get("dhcp6", 59); + def = CfgMgr::instance().getStagingCfg()-> + getCfgOptionDef()->get("dhcp6", 59); ASSERT_TRUE(def); // Check the option data. @@ -2202,6 +2207,8 @@ TEST_F(Dhcp6ParserTest, optionDataEncapsulate) { ASSERT_TRUE(status); checkResult(status, 0); + CfgMgr::instance().clear(); + // Stage 2. Configure base option and a subnet. Please note that // the configuration from the stage 2 is repeated because BIND // configuration manager sends whole configuration for the lists @@ -2395,6 +2402,8 @@ TEST_F(Dhcp6ParserTest, optionDataBoolean) { ASSERT_TRUE(executeConfiguration(config, "parse configuration with a" " boolean value")); + CfgMgr::instance().commit(); + // The subnet should now hold one option with the code 1000. Subnet::OptionDescriptor desc = getOptionFromSubnet(IOAddress("2001:db8:1::5"), 1000); @@ -2785,6 +2794,8 @@ TEST_F(Dhcp6ParserTest, stdOptionDataEncapsulate) { ASSERT_TRUE(status); checkResult(status, 0); + CfgMgr::instance().clear(); + // Once the definitions have been added we can configure the // standard option #17. This option comprises an enterprise // number and sub options. By convention (introduced in @@ -3027,6 +3038,8 @@ TEST_F(Dhcp6ParserTest, LibrariesSpecified) { EXPECT_TRUE(checkMarkerFile(LOAD_MARKER_FILE, "12")); EXPECT_FALSE(checkMarkerFileExists(UNLOAD_MARKER_FILE)); + CfgMgr::instance().commit(); + // Unload the libraries. The load file should not have changed, but // the unload one should indicate the unload() functions have been run. config = buildHooksLibrariesConfig(); diff --git a/src/bin/dhcp6/tests/kea_controller_unittest.cc b/src/bin/dhcp6/tests/kea_controller_unittest.cc index 3bdffabb7..7a36f935f 100644 --- a/src/bin/dhcp6/tests/kea_controller_unittest.cc +++ b/src/bin/dhcp6/tests/kea_controller_unittest.cc @@ -18,6 +18,7 @@ #include #include #include +#include #include #include @@ -52,6 +53,7 @@ public: } ~JSONFileBackendTest() { + isc::log::resetUnitTestRootLogger(); static_cast(unlink(TEST_FILE)); }; diff --git a/src/lib/dhcpsrv/cfg_option_def.h b/src/lib/dhcpsrv/cfg_option_def.h index 5edc6a455..0035ed1f0 100644 --- a/src/lib/dhcpsrv/cfg_option_def.h +++ b/src/lib/dhcpsrv/cfg_option_def.h @@ -121,6 +121,16 @@ private: }; +/// @name Pointers to the @c CfgOptionDef objects. +//@{ +/// @brief Non-const pointer. +typedef boost::shared_ptr CfgOptionDefPtr; + +/// @brief Const pointer. +typedef boost::shared_ptr ConstCfgOptionDefPtr; + +//@} + } } diff --git a/src/lib/dhcpsrv/dhcp_parsers.cc b/src/lib/dhcpsrv/dhcp_parsers.cc index aa4899447..3173332fc 100644 --- a/src/lib/dhcpsrv/dhcp_parsers.cc +++ b/src/lib/dhcpsrv/dhcp_parsers.cc @@ -443,7 +443,7 @@ OptionDataParser::createOption(ConstElementPtr option_data) { // options. They are expected to be in the global storage // already. OptionDefContainerPtr defs = - CfgMgr::instance().getStagingCfg()->getCfgOptionDef().getAll(space); + CfgMgr::instance().getStagingCfg()->getCfgOptionDef()->getAll(space); // The getItems() should never return the NULL pointer. If there are // no option definitions for the particular option space a pointer @@ -646,16 +646,15 @@ OptionDefParser::build(ConstElementPtr option_def) { // Create an instance of option definition. createOptionDef(option_def); - CfgOptionDef cfg = CfgMgr::instance().getStagingCfg()->getCfgOptionDef(); try { - cfg.add(option_definition_, option_space_name_); + CfgMgr::instance().getStagingCfg()->getCfgOptionDef()-> + add(option_definition_, option_space_name_); } catch (const std::exception& ex) { - // Append position + // Append position if there is a failure. isc_throw(DhcpConfigError, ex.what() << " (" << option_def->getPosition() << ")"); } - CfgMgr::instance().getStagingCfg()->setCfgOptionDef(cfg); } void @@ -1044,7 +1043,7 @@ SubnetConfigParser::appendSubOptions(const std::string& option_space, } } else { OptionDefContainerPtr defs = CfgMgr::instance().getStagingCfg() - ->getCfgOptionDef().getAll(option_space); + ->getCfgOptionDef()->getAll(option_space); const OptionDefContainerTypeIndex& idx = defs->get<1>(); const OptionDefContainerTypeRange& range = diff --git a/src/lib/dhcpsrv/srv_config.cc b/src/lib/dhcpsrv/srv_config.cc index 42b6a48a6..a7215a911 100644 --- a/src/lib/dhcpsrv/srv_config.cc +++ b/src/lib/dhcpsrv/srv_config.cc @@ -25,11 +25,11 @@ namespace isc { namespace dhcp { SrvConfig::SrvConfig() - : sequence_(0) { + : sequence_(0), cfg_option_def_(new CfgOptionDef()) { } SrvConfig::SrvConfig(uint32_t sequence) - : sequence_(sequence) { + : sequence_(sequence), cfg_option_def_(new CfgOptionDef()) { } std::string @@ -89,7 +89,7 @@ SrvConfig::copy(SrvConfig& new_config) const { // Replace interface configuration. new_config.setCfgIface(cfg_iface_); // Replace option definitions. - new_config.setCfgOptionDef(cfg_option_def_); + new_config.setCfgOptionDef(*cfg_option_def_); } void @@ -134,7 +134,7 @@ SrvConfig::equals(const SrvConfig& other) const { } // Logging information is equal between objects, so check other values. return ((cfg_iface_ == other.cfg_iface_) && - (cfg_option_def_ == other.cfg_option_def_)); + (*cfg_option_def_ == *other.cfg_option_def_)); } } diff --git a/src/lib/dhcpsrv/srv_config.h b/src/lib/dhcpsrv/srv_config.h index e15664732..a5cb11f96 100644 --- a/src/lib/dhcpsrv/srv_config.h +++ b/src/lib/dhcpsrv/srv_config.h @@ -141,13 +141,25 @@ public: cfg_iface_ = cfg_iface; } - /// @brief Returns object which represents user-defined option definitions. + /// @brief Return pointer to non-const object representing user-defined + /// option definitions. /// - /// This function returns a reference to the object which represents the + /// This function returns a pointer to the object which represents the + /// user defined option definitions grouped by option space name. + /// + /// @return Pointer to an object holding option definitions. + CfgOptionDefPtr getCfgOptionDef() { + return (cfg_option_def_); + } + + /// @brief Returns pointer to the const object representing user-defined + /// option definitions. + /// + /// This function returns a pointer to the object which represents the /// user defined option definitions grouped by option space name. /// - /// @return Reference to an object holding option definitions. - const CfgOptionDef& getCfgOptionDef() const { + /// @return Pointer to an object holding option definitions. + ConstCfgOptionDefPtr getCfgOptionDef() const { return (cfg_option_def_); } @@ -155,7 +167,7 @@ public: /// /// @param cfg_option_def New object representing option definitions. void setCfgOptionDef(const CfgOptionDef& cfg_option_def) { - cfg_option_def_ = cfg_option_def; + *cfg_option_def_ = cfg_option_def; } //@} @@ -239,11 +251,11 @@ private: /// queries. CfgIface cfg_iface_; - /// @brief Option definitions configuration. + /// @brief Pointer to option definitions configuration. /// /// This object holds the user-defined option definitions grouped /// by option space name. - CfgOptionDef cfg_option_def_; + CfgOptionDefPtr cfg_option_def_; }; diff --git a/src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc b/src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc index d0687166a..ed794cf4c 100644 --- a/src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc +++ b/src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc @@ -536,7 +536,7 @@ TEST_F(ParseConfigTest, basicOptionDefTest) { // Verify that the option definition can be retrieved. OptionDefinitionPtr def = - CfgMgr::instance().getStagingCfg()->getCfgOptionDef().get("isc", 100); + CfgMgr::instance().getStagingCfg()->getCfgOptionDef()->get("isc", 100); ASSERT_TRUE(def); // Verify that the option definition is correct. -- GitLab