Browse Source

[2315] Return a pointer to option container rather than a reference.

Marcin Siodelski 12 years ago
parent
commit
b789222291

+ 4 - 4
src/bin/dhcp4/config_parser.cc

@@ -894,8 +894,8 @@ public:
             subnet->addPool4(*it);
             subnet->addPool4(*it);
         }
         }
 
 
-        const Subnet::OptionContainer& options = subnet->getOptions("dhcp4");
-        const Subnet::OptionContainerTypeIndex& idx = options.get<1>();
+        Subnet::OptionContainerPtr options = subnet->getOptionDescriptors("dhcp4");
+        const Subnet::OptionContainerTypeIndex& idx = options->get<1>();
 
 
         // Add subnet specific options.
         // Add subnet specific options.
         BOOST_FOREACH(Subnet::OptionDescriptor desc, options_) {
         BOOST_FOREACH(Subnet::OptionDescriptor desc, options_) {
@@ -914,8 +914,8 @@ public:
         BOOST_FOREACH(Subnet::OptionDescriptor desc, option_defaults) {
         BOOST_FOREACH(Subnet::OptionDescriptor desc, option_defaults) {
             // Get all options specified locally in the subnet and having
             // Get all options specified locally in the subnet and having
             // code equal to global option's code.
             // code equal to global option's code.
-            const Subnet::OptionContainer& options = subnet->getOptions("dhcp4");
-            const Subnet::OptionContainerTypeIndex& idx = options.get<1>();
+            Subnet::OptionContainerPtr options = subnet->getOptionDescriptors("dhcp4");
+            const Subnet::OptionContainerTypeIndex& idx = options->get<1>();
             Subnet::OptionContainerTypeRange range = idx.equal_range(desc.option->getType());
             Subnet::OptionContainerTypeRange range = idx.equal_range(desc.option->getType());
             // @todo: In the future we will be searching for options using either
             // @todo: In the future we will be searching for options using either
             // an option code or namespace. Currently we have only the option
             // an option code or namespace. Currently we have only the option

+ 15 - 15
src/bin/dhcp4/tests/config_parser_unittest.cc

@@ -461,11 +461,11 @@ TEST_F(Dhcp4ParserTest, optionDataDefaults) {
 
 
     Subnet4Ptr subnet = CfgMgr::instance().getSubnet4(IOAddress("192.0.2.200"));
     Subnet4Ptr subnet = CfgMgr::instance().getSubnet4(IOAddress("192.0.2.200"));
     ASSERT_TRUE(subnet);
     ASSERT_TRUE(subnet);
-    const Subnet::OptionContainer& options = subnet->getOptions("dhcp4");
-    ASSERT_EQ(2, options.size());
+    Subnet::OptionContainerPtr options = subnet->getOptionDescriptors("dhcp4");
+    ASSERT_EQ(2, options->size());
 
 
     // Get the search index. Index #1 is to search using option code.
     // Get the search index. Index #1 is to search using option code.
-    const Subnet::OptionContainerTypeIndex& idx = options.get<1>();
+    const Subnet::OptionContainerTypeIndex& idx = options->get<1>();
 
 
     // Get the options for specified index. Expecting one option to be
     // Get the options for specified index. Expecting one option to be
     // returned but in theory we may have multiple options with the same
     // returned but in theory we may have multiple options with the same
@@ -529,11 +529,11 @@ TEST_F(Dhcp4ParserTest, optionDataInSingleSubnet) {
 
 
     Subnet4Ptr subnet = CfgMgr::instance().getSubnet4(IOAddress("192.0.2.24"));
     Subnet4Ptr subnet = CfgMgr::instance().getSubnet4(IOAddress("192.0.2.24"));
     ASSERT_TRUE(subnet);
     ASSERT_TRUE(subnet);
-    const Subnet::OptionContainer& options = subnet->getOptions("dhcp4");
-    ASSERT_EQ(2, options.size());
+    Subnet::OptionContainerPtr options = subnet->getOptionDescriptors("dhcp4");
+    ASSERT_EQ(2, options->size());
 
 
     // Get the search index. Index #1 is to search using option code.
     // Get the search index. Index #1 is to search using option code.
-    const Subnet::OptionContainerTypeIndex& idx = options.get<1>();
+    const Subnet::OptionContainerTypeIndex& idx = options->get<1>();
 
 
     // Get the options for specified index. Expecting one option to be
     // Get the options for specified index. Expecting one option to be
     // returned but in theory we may have multiple options with the same
     // returned but in theory we may have multiple options with the same
@@ -594,11 +594,11 @@ TEST_F(Dhcp4ParserTest, optionDataInMultipleSubnets) {
 
 
     Subnet4Ptr subnet1 = CfgMgr::instance().getSubnet4(IOAddress("192.0.2.100"));
     Subnet4Ptr subnet1 = CfgMgr::instance().getSubnet4(IOAddress("192.0.2.100"));
     ASSERT_TRUE(subnet1);
     ASSERT_TRUE(subnet1);
-    const Subnet::OptionContainer& options1 = subnet1->getOptions("dhcp4");
-    ASSERT_EQ(1, options1.size());
+    Subnet::OptionContainerPtr options1 = subnet1->getOptionDescriptors("dhcp4");
+    ASSERT_EQ(1, options1->size());
 
 
     // Get the search index. Index #1 is to search using option code.
     // Get the search index. Index #1 is to search using option code.
-    const Subnet::OptionContainerTypeIndex& idx1 = options1.get<1>();
+    const Subnet::OptionContainerTypeIndex& idx1 = options1->get<1>();
 
 
     // Get the options for specified index. Expecting one option to be
     // Get the options for specified index. Expecting one option to be
     // returned but in theory we may have multiple options with the same
     // returned but in theory we may have multiple options with the same
@@ -618,10 +618,10 @@ TEST_F(Dhcp4ParserTest, optionDataInMultipleSubnets) {
     // Test another subnet in the same way.
     // Test another subnet in the same way.
     Subnet4Ptr subnet2 = CfgMgr::instance().getSubnet4(IOAddress("192.0.3.102"));
     Subnet4Ptr subnet2 = CfgMgr::instance().getSubnet4(IOAddress("192.0.3.102"));
     ASSERT_TRUE(subnet2);
     ASSERT_TRUE(subnet2);
-    const Subnet::OptionContainer& options2 = subnet2->getOptions("dhcp4");
-    ASSERT_EQ(1, options2.size());
+    Subnet::OptionContainerPtr options2 = subnet2->getOptionDescriptors("dhcp4");
+    ASSERT_EQ(1, options2->size());
 
 
-    const Subnet::OptionContainerTypeIndex& idx2 = options2.get<1>();
+    const Subnet::OptionContainerTypeIndex& idx2 = options2->get<1>();
     std::pair<Subnet::OptionContainerTypeIndex::const_iterator,
     std::pair<Subnet::OptionContainerTypeIndex::const_iterator,
               Subnet::OptionContainerTypeIndex::const_iterator> range2 =
               Subnet::OptionContainerTypeIndex::const_iterator> range2 =
         idx2.equal_range(23);
         idx2.equal_range(23);
@@ -703,11 +703,11 @@ TEST_F(Dhcp4ParserTest, optionDataLowerCase) {
 
 
     Subnet4Ptr subnet = CfgMgr::instance().getSubnet4(IOAddress("192.0.2.5"));
     Subnet4Ptr subnet = CfgMgr::instance().getSubnet4(IOAddress("192.0.2.5"));
     ASSERT_TRUE(subnet);
     ASSERT_TRUE(subnet);
-    const Subnet::OptionContainer& options = subnet->getOptions("dhcp4");
-    ASSERT_EQ(1, options.size());
+    Subnet::OptionContainerPtr options = subnet->getOptionDescriptors("dhcp4");
+    ASSERT_EQ(1, options->size());
 
 
     // Get the search index. Index #1 is to search using option code.
     // Get the search index. Index #1 is to search using option code.
-    const Subnet::OptionContainerTypeIndex& idx = options.get<1>();
+    const Subnet::OptionContainerTypeIndex& idx = options->get<1>();
 
 
     // Get the options for specified index. Expecting one option to be
     // Get the options for specified index. Expecting one option to be
     // returned but in theory we may have multiple options with the same
     // returned but in theory we may have multiple options with the same

+ 4 - 4
src/bin/dhcp6/config_parser.cc

@@ -925,8 +925,8 @@ public:
             subnet->addPool6(*it);
             subnet->addPool6(*it);
         }
         }
 
 
-        const Subnet::OptionContainer& options = subnet->getOptions("dhcp6");
-        const Subnet::OptionContainerTypeIndex& idx = options.get<1>();
+        Subnet::OptionContainerPtr options = subnet->getOptionDescriptors("dhcp6");
+        const Subnet::OptionContainerTypeIndex& idx = options->get<1>();
 
 
         // Add subnet specific options.
         // Add subnet specific options.
         BOOST_FOREACH(Subnet::OptionDescriptor desc, options_) {
         BOOST_FOREACH(Subnet::OptionDescriptor desc, options_) {
@@ -945,8 +945,8 @@ public:
         BOOST_FOREACH(Subnet::OptionDescriptor desc, option_defaults) {
         BOOST_FOREACH(Subnet::OptionDescriptor desc, option_defaults) {
             // Get all options specified locally in the subnet and having
             // Get all options specified locally in the subnet and having
             // code equal to global option's code.
             // code equal to global option's code.
-            const Subnet::OptionContainer& options = subnet->getOptions("dhcp6");
-            const Subnet::OptionContainerTypeIndex& idx = options.get<1>();
+            Subnet::OptionContainerPtr options = subnet->getOptionDescriptors("dhcp6");
+            const Subnet::OptionContainerTypeIndex& idx = options->get<1>();
             Subnet::OptionContainerTypeRange range = idx.equal_range(desc.option->getType());
             Subnet::OptionContainerTypeRange range = idx.equal_range(desc.option->getType());
             // @todo: In the future we will be searching for options using either
             // @todo: In the future we will be searching for options using either
             // an option code or namespace. Currently we have only the option
             // an option code or namespace. Currently we have only the option

+ 2 - 2
src/bin/dhcp6/dhcp6_srv.cc

@@ -340,8 +340,8 @@ void Dhcpv6Srv::appendRequestedOptions(const Pkt6Ptr& question, Pkt6Ptr& answer)
     // Get the list of options that client requested.
     // Get the list of options that client requested.
     const std::vector<uint16_t>& requested_opts = option_oro->getValues();
     const std::vector<uint16_t>& requested_opts = option_oro->getValues();
     // Get the list of options configured for a subnet.
     // Get the list of options configured for a subnet.
-    const Subnet::OptionContainer& options = subnet->getOptions("dhcp6");
-    const Subnet::OptionContainerTypeIndex& idx = options.get<1>();
+    Subnet::OptionContainerPtr options = subnet->getOptionDescriptors("dhcp6");
+    const Subnet::OptionContainerTypeIndex& idx = options->get<1>();
     // Try to match requested options with those configured for a subnet.
     // Try to match requested options with those configured for a subnet.
     // If match is found, append configured option to the answer message.
     // If match is found, append configured option to the answer message.
     BOOST_FOREACH(uint16_t opt, requested_opts) {
     BOOST_FOREACH(uint16_t opt, requested_opts) {

+ 18 - 18
src/bin/dhcp6/tests/config_parser_unittest.cc

@@ -449,11 +449,11 @@ TEST_F(Dhcp6ParserTest, optionDataDefaults) {
 
 
     Subnet6Ptr subnet = CfgMgr::instance().getSubnet6(IOAddress("2001:db8:1::5"));
     Subnet6Ptr subnet = CfgMgr::instance().getSubnet6(IOAddress("2001:db8:1::5"));
     ASSERT_TRUE(subnet);
     ASSERT_TRUE(subnet);
-    const Subnet::OptionContainer& options = subnet->getOptions("dhcp6");
-    ASSERT_EQ(2, options.size());
+    Subnet::OptionContainerPtr options = subnet->getOptionDescriptors("dhcp6");
+    ASSERT_EQ(2, options->size());
 
 
     // Get the search index. Index #1 is to search using option code.
     // Get the search index. Index #1 is to search using option code.
-    const Subnet::OptionContainerTypeIndex& idx = options.get<1>();
+    const Subnet::OptionContainerTypeIndex& idx = options->get<1>();
 
 
     // Get the options for specified index. Expecting one option to be
     // Get the options for specified index. Expecting one option to be
     // returned but in theory we may have multiple options with the same
     // returned but in theory we may have multiple options with the same
@@ -524,11 +524,11 @@ TEST_F(Dhcp6ParserTest, optionDataInSingleSubnet) {
 
 
     Subnet6Ptr subnet = CfgMgr::instance().getSubnet6(IOAddress("2001:db8:1::5"));
     Subnet6Ptr subnet = CfgMgr::instance().getSubnet6(IOAddress("2001:db8:1::5"));
     ASSERT_TRUE(subnet);
     ASSERT_TRUE(subnet);
-    const Subnet::OptionContainer& options = subnet->getOptions("dhcp6");
-    ASSERT_EQ(2, options.size());
+    Subnet::OptionContainerPtr options = subnet->getOptionDescriptors("dhcp6");
+    ASSERT_EQ(2, options->size());
 
 
     // Get the search index. Index #1 is to search using option code.
     // Get the search index. Index #1 is to search using option code.
-    const Subnet::OptionContainerTypeIndex& idx = options.get<1>();
+    const Subnet::OptionContainerTypeIndex& idx = options->get<1>();
 
 
     // Get the options for specified index. Expecting one option to be
     // Get the options for specified index. Expecting one option to be
     // returned but in theory we may have multiple options with the same
     // returned but in theory we may have multiple options with the same
@@ -590,11 +590,11 @@ TEST_F(Dhcp6ParserTest, optionDataInMultipleSubnets) {
 
 
     Subnet6Ptr subnet1 = CfgMgr::instance().getSubnet6(IOAddress("2001:db8:1::5"));
     Subnet6Ptr subnet1 = CfgMgr::instance().getSubnet6(IOAddress("2001:db8:1::5"));
     ASSERT_TRUE(subnet1);
     ASSERT_TRUE(subnet1);
-    const Subnet::OptionContainer& options1 = subnet1->getOptions("dhcp6");
-    ASSERT_EQ(1, options1.size());
+    Subnet::OptionContainerPtr options1 = subnet1->getOptionDescriptors("dhcp6");
+    ASSERT_EQ(1, options1->size());
 
 
     // Get the search index. Index #1 is to search using option code.
     // Get the search index. Index #1 is to search using option code.
-    const Subnet::OptionContainerTypeIndex& idx1 = options1.get<1>();
+    const Subnet::OptionContainerTypeIndex& idx1 = options1->get<1>();
 
 
     // Get the options for specified index. Expecting one option to be
     // Get the options for specified index. Expecting one option to be
     // returned but in theory we may have multiple options with the same
     // returned but in theory we may have multiple options with the same
@@ -614,10 +614,10 @@ TEST_F(Dhcp6ParserTest, optionDataInMultipleSubnets) {
     // Test another subnet in the same way.
     // Test another subnet in the same way.
     Subnet6Ptr subnet2 = CfgMgr::instance().getSubnet6(IOAddress("2001:db8:2::4"));
     Subnet6Ptr subnet2 = CfgMgr::instance().getSubnet6(IOAddress("2001:db8:2::4"));
     ASSERT_TRUE(subnet2);
     ASSERT_TRUE(subnet2);
-    const Subnet::OptionContainer& options2 = subnet2->getOptions("dhcp6");
-    ASSERT_EQ(1, options2.size());
+    Subnet::OptionContainerPtr options2 = subnet2->getOptionDescriptors("dhcp6");
+    ASSERT_EQ(1, options2->size());
 
 
-    const Subnet::OptionContainerTypeIndex& idx2 = options2.get<1>();
+    const Subnet::OptionContainerTypeIndex& idx2 = options2->get<1>();
     std::pair<Subnet::OptionContainerTypeIndex::const_iterator,
     std::pair<Subnet::OptionContainerTypeIndex::const_iterator,
               Subnet::OptionContainerTypeIndex::const_iterator> range2 =
               Subnet::OptionContainerTypeIndex::const_iterator> range2 =
         idx2.equal_range(101);
         idx2.equal_range(101);
@@ -708,11 +708,11 @@ TEST_F(Dhcp6ParserTest, optionDataLowerCase) {
 
 
     Subnet6Ptr subnet = CfgMgr::instance().getSubnet6(IOAddress("2001:db8:1::5"));
     Subnet6Ptr subnet = CfgMgr::instance().getSubnet6(IOAddress("2001:db8:1::5"));
     ASSERT_TRUE(subnet);
     ASSERT_TRUE(subnet);
-    const Subnet::OptionContainer& options = subnet->getOptions("dhcp6");
-    ASSERT_EQ(1, options.size());
+    Subnet::OptionContainerPtr options = subnet->getOptionDescriptors("dhcp6");
+    ASSERT_EQ(1, options->size());
 
 
     // Get the search index. Index #1 is to search using option code.
     // Get the search index. Index #1 is to search using option code.
-    const Subnet::OptionContainerTypeIndex& idx = options.get<1>();
+    const Subnet::OptionContainerTypeIndex& idx = options->get<1>();
 
 
     // Get the options for specified index. Expecting one option to be
     // Get the options for specified index. Expecting one option to be
     // returned but in theory we may have multiple options with the same
     // returned but in theory we may have multiple options with the same
@@ -749,11 +749,11 @@ TEST_F(Dhcp6ParserTest, stdOptionData) {
 
 
     Subnet6Ptr subnet = CfgMgr::instance().getSubnet6(IOAddress("2001:db8:1::5"));
     Subnet6Ptr subnet = CfgMgr::instance().getSubnet6(IOAddress("2001:db8:1::5"));
     ASSERT_TRUE(subnet);
     ASSERT_TRUE(subnet);
-    const Subnet::OptionContainer& options = subnet->getOptions("dhcp6");
-    ASSERT_EQ(1, options.size());
+    Subnet::OptionContainerPtr options = subnet->getOptionDescriptors("dhcp6");
+    ASSERT_EQ(1, options->size());
 
 
     // Get the search index. Index #1 is to search using option code.
     // Get the search index. Index #1 is to search using option code.
-    const Subnet::OptionContainerTypeIndex& idx = options.get<1>();
+    const Subnet::OptionContainerTypeIndex& idx = options->get<1>();
 
 
     // Get the options for specified index. Expecting one option to be
     // Get the options for specified index. Expecting one option to be
     // returned but in theory we may have multiple options with the same
     // returned but in theory we may have multiple options with the same

+ 13 - 8
src/lib/dhcpsrv/cfgmgr.cc

@@ -55,11 +55,17 @@ CfgMgr::addOptionDef(const OptionDefinitionPtr& def,
                   << option_space << "'.");
                   << option_space << "'.");
 
 
     }
     }
-    // Actually add the definition to the option space.
-    option_def_spaces_[option_space].push_back(def);
+    // Get existing option definitions for the option space.
+    OptionDefContainerPtr defs = getOptionDefs(option_space);
+    // getOptionDefs always returns a valid pointer to
+    // the container. Let's make an assert to make sure.
+    assert(defs);
+    // Actually add the new definition.
+    defs->push_back(def);
+    option_def_spaces_[option_space] = defs;
 }
 }
 
 
-const OptionDefContainer&
+OptionDefContainerPtr
 CfgMgr::getOptionDefs(const std::string& option_space) const {
 CfgMgr::getOptionDefs(const std::string& option_space) const {
     // @todo Validate the option space once the #2313 is implemented.
     // @todo Validate the option space once the #2313 is implemented.
 
 
@@ -69,8 +75,7 @@ CfgMgr::getOptionDefs(const std::string& option_space) const {
     // If there are no option definitions for the particular option space
     // If there are no option definitions for the particular option space
     // then return empty container.
     // then return empty container.
     if (defs == option_def_spaces_.end()) {
     if (defs == option_def_spaces_.end()) {
-        static OptionDefContainer empty_container;
-        return (empty_container);
+        return (OptionDefContainerPtr(new OptionDefContainer()));
     }
     }
     // If option definitions found, return them.
     // If option definitions found, return them.
     return (defs->second);
     return (defs->second);
@@ -82,14 +87,14 @@ CfgMgr::getOptionDef(const std::string& option_space,
     // @todo Validate the option space once the #2313 is implemented.
     // @todo Validate the option space once the #2313 is implemented.
 
 
     // Get a reference to option definitions for a particular option space.
     // Get a reference to option definitions for a particular option space.
-    const OptionDefContainer& defs = getOptionDefs(option_space);
+    OptionDefContainerPtr defs = getOptionDefs(option_space);
     // If there are no matching option definitions then return the empty pointer.
     // If there are no matching option definitions then return the empty pointer.
-    if (defs.empty()) {
+    if (!defs || defs->empty()) {
         return (OptionDefinitionPtr());
         return (OptionDefinitionPtr());
     }
     }
     // If there are some option definitions for a particular option space
     // If there are some option definitions for a particular option space
     // use an option code to get the one we want.
     // use an option code to get the one we want.
-    const OptionDefContainerTypeIndex& idx = defs.get<1>();
+    const OptionDefContainerTypeIndex& idx = defs->get<1>();
     const OptionDefContainerTypeRange& range = idx.equal_range(option_code);
     const OptionDefContainerTypeRange& range = idx.equal_range(option_code);
     // If there is no definition that matches option code, return empty pointer.
     // If there is no definition that matches option code, return empty pointer.
     if (std::distance(range.first, range.second) == 0) {
     if (std::distance(range.first, range.second) == 0) {

+ 5 - 4
src/lib/dhcpsrv/cfgmgr.h

@@ -97,9 +97,10 @@ public:
     ///
     ///
     /// @param option_space option space.
     /// @param option_space option space.
     ///
     ///
-    /// @return collection of option definitions for a particular
-    /// option space.
-    const OptionDefContainer&
+    /// @return pointer to the collection of option definitions for
+    /// the particular option space. The option collection is empty
+    /// if no option exists for the option space specified.
+    OptionDefContainerPtr
     getOptionDefs(const std::string& option_space) const;
     getOptionDefs(const std::string& option_space) const;
 
 
     /// @brief Return option definition for a particular option space and code.
     /// @brief Return option definition for a particular option space and code.
@@ -223,7 +224,7 @@ private:
     /// They key of this map is the name of the option space. The
     /// They key of this map is the name of the option space. The
     /// value is the the option container holding option definitions
     /// value is the the option container holding option definitions
     /// for the particular option space.
     /// for the particular option space.
-    typedef std::map<std::string, OptionDefContainer> OptionDefsMap;
+    typedef std::map<std::string, OptionDefContainerPtr> OptionDefsMap;
 
 
     /// A map containing option definitions for different option spaces.
     /// A map containing option definitions for different option spaces.
     /// The map key holds an option space name.
     /// The map key holds an option space name.

+ 15 - 9
src/lib/dhcpsrv/subnet.cc

@@ -54,7 +54,14 @@ Subnet::addOption(OptionPtr& option, bool persistent,
         isc_throw(isc::BadValue, "option space name must not be empty");
         isc_throw(isc::BadValue, "option space name must not be empty");
     }
     }
     validateOption(option);
     validateOption(option);
-    option_spaces_[option_space].push_back(OptionDescriptor(option, persistent));
+
+    OptionContainerPtr container = getOptionDescriptors(option_space);
+    // getOptionDescriptors is expected to return the pointer to the
+    // valid container. Let's make sure it does by performing an assert.
+    assert(container);
+    // Actually add the new descriptor.
+    container->push_back(OptionDescriptor(option, persistent));
+    option_spaces_[option_space] = container;
 }
 }
 
 
 void
 void
@@ -62,19 +69,18 @@ Subnet::delOptions() {
     option_spaces_.clear();
     option_spaces_.clear();
 }
 }
 
 
-const Subnet::OptionContainer&
-Subnet::getOptions(const std::string& option_space) const {
+Subnet::OptionContainerPtr
+Subnet::getOptionDescriptors(const std::string& option_space) const {
     // Search the map to get the options container for the particular
     // Search the map to get the options container for the particular
     // option space.
     // option space.
-    const std::map<std::string, OptionContainer>::const_iterator& options =
+    const OptionSpacesPtr::const_iterator& options =
         option_spaces_.find(option_space);
         option_spaces_.find(option_space);
     // If the option space has not been found it means that no option
     // If the option space has not been found it means that no option
     // has been configured for this option space yet. Thus we have to
     // has been configured for this option space yet. Thus we have to
     // return an empty container to the caller.
     // return an empty container to the caller.
     if (options == option_spaces_.end()) {
     if (options == option_spaces_.end()) {
         // The default constructor creates an empty container.
         // The default constructor creates an empty container.
-        static OptionContainer container;
-        return (container);
+        return (OptionContainerPtr(new OptionContainer()));
     }
     }
     // We found some option container for the option space specified.
     // We found some option container for the option space specified.
     // Let's return a const reference to it.
     // Let's return a const reference to it.
@@ -84,11 +90,11 @@ Subnet::getOptions(const std::string& option_space) const {
 Subnet::OptionDescriptor
 Subnet::OptionDescriptor
 Subnet::getOptionDescriptor(const std::string& option_space,
 Subnet::getOptionDescriptor(const std::string& option_space,
                             const uint16_t option_code) {
                             const uint16_t option_code) {
-    const OptionContainer& options = getOptions(option_space);
-    if (options.empty()) {
+    OptionContainerPtr options = getOptionDescriptors(option_space);
+    if (!options || options->empty()) {
         return (OptionDescriptor(false));
         return (OptionDescriptor(false));
     }
     }
-    const OptionContainerTypeIndex& idx = options.get<1>();
+    const OptionContainerTypeIndex& idx = options->get<1>();
     const OptionContainerTypeRange& range = idx.equal_range(option_code);
     const OptionContainerTypeRange& range = idx.equal_range(option_code);
     if (std::distance(range.first, range.second) == 0) {
     if (std::distance(range.first, range.second) == 0) {
         return (OptionDescriptor(false));
         return (OptionDescriptor(false));

+ 20 - 10
src/lib/dhcpsrv/subnet.h

@@ -78,6 +78,9 @@ public:
             : option(OptionPtr()), persistent(persist) {};
             : option(OptionPtr()), persistent(persist) {};
     };
     };
 
 
+    /// A pointer to option descriptor.
+    typedef boost::shared_ptr<OptionDescriptor> OptionDescriptorPtr;
+
     /// @brief Extractor class to extract key with another key.
     /// @brief Extractor class to extract key with another key.
     ///
     ///
     /// This class solves the problem of accessing index key values
     /// This class solves the problem of accessing index key values
@@ -198,6 +201,8 @@ public:
         >
         >
     > OptionContainer;
     > OptionContainer;
 
 
+    // Pointer to the OptionContainer object.
+    typedef boost::shared_ptr<OptionContainer> OptionContainerPtr;
     /// Type of the index #1 - option type.
     /// Type of the index #1 - option type.
     typedef OptionContainer::nth_index<1>::type OptionContainerTypeIndex;
     typedef OptionContainer::nth_index<1>::type OptionContainerTypeIndex;
     /// Pair of iterators to represent the range of options having the
     /// Pair of iterators to represent the range of options having the
@@ -258,10 +263,9 @@ public:
     ///
     ///
     /// @param option_space name of the option space.
     /// @param option_space name of the option space.
     ///
     ///
-    /// @return reference to collection of options configured for a subnet.
-    /// The returned reference is valid as long as the Subnet object which
-    /// returned it still exists.
-    const OptionContainer& getOptions(const std::string& option_space) const;
+    /// @return pointer to collection of options configured for a subnet.
+    OptionContainerPtr
+    getOptionDescriptors(const std::string& option_space) const;
 
 
     /// @brief Return single option descriptor.
     /// @brief Return single option descriptor.
     ///
     ///
@@ -270,8 +274,9 @@ public:
     ///
     ///
     /// @return option descriptor found for the specified option space
     /// @return option descriptor found for the specified option space
     /// and option code.
     /// and option code.
-    OptionDescriptor getOptionDescriptor(const std::string& option_space,
-                                         const uint16_t option_code);
+    OptionDescriptor
+    getOptionDescriptor(const std::string& option_space,
+                        const uint16_t option_code);
 
 
     /// @brief returns the last address that was tried from this pool
     /// @brief returns the last address that was tried from this pool
     ///
     ///
@@ -365,10 +370,6 @@ protected:
     /// @brief a tripet (min/default/max) holding allowed valid lifetime values
     /// @brief a tripet (min/default/max) holding allowed valid lifetime values
     Triplet<uint32_t> valid_;
     Triplet<uint32_t> valid_;
 
 
-    /// @brief a collection of DHCP option spaces holding options
-    /// configured for a subnet.
-    std::map<std::string, OptionContainer> option_spaces_;
-
     /// @brief last allocated address
     /// @brief last allocated address
     ///
     ///
     /// This is the last allocated address that was previously allocated from
     /// This is the last allocated address that was previously allocated from
@@ -379,6 +380,15 @@ protected:
     /// that purpose it should be only considered a help that should not be
     /// that purpose it should be only considered a help that should not be
     /// fully trusted.
     /// fully trusted.
     isc::asiolink::IOAddress last_allocated_;
     isc::asiolink::IOAddress last_allocated_;
+
+private:
+
+    /// Container holding options grouped by option space names.
+    typedef std::map<std::string, OptionContainerPtr> OptionSpacesPtr;
+
+    /// @brief a collection of DHCP option spaces holding options
+    /// configured for a subnet.
+    OptionSpacesPtr option_spaces_;
 };
 };
 
 
 /// @brief A configuration holder for IPv4 subnet.
 /// @brief A configuration holder for IPv4 subnet.

+ 13 - 10
src/lib/dhcpsrv/tests/cfgmgr_unittest.cc

@@ -75,28 +75,30 @@ TEST_F(CfgMgrTest, getOptionDefs) {
     }
     }
 
 
     // Sanity check that all 10 option definitions are there.
     // Sanity check that all 10 option definitions are there.
-    const OptionDefContainer& option_defs1 = cfg_mgr.getOptionDefs("isc");
-    ASSERT_EQ(10, option_defs1.size());
+    OptionDefContainerPtr option_defs1 = cfg_mgr.getOptionDefs("isc");
+    ASSERT_TRUE(option_defs1);
+    ASSERT_EQ(10, option_defs1->size());
 
 
     // Iterate over all option definitions and check that they have
     // Iterate over all option definitions and check that they have
     // valid codes. Also, their order should be the same as they
     // valid codes. Also, their order should be the same as they
     // were added (codes 100-109).
     // were added (codes 100-109).
     uint16_t code = 100;
     uint16_t code = 100;
-    for (OptionDefContainer::const_iterator it = option_defs1.begin();
-         it != option_defs1.end(); ++it, ++code) {
+    for (OptionDefContainer::const_iterator it = option_defs1->begin();
+         it != option_defs1->end(); ++it, ++code) {
         OptionDefinitionPtr def(*it);
         OptionDefinitionPtr def(*it);
         ASSERT_TRUE(def);
         ASSERT_TRUE(def);
         EXPECT_EQ(code, def->getCode());
         EXPECT_EQ(code, def->getCode());
     }
     }
 
 
     // Sanity check that all 10 option definitions are there.
     // Sanity check that all 10 option definitions are there.
-    const OptionDefContainer& option_defs2 = cfg_mgr.getOptionDefs("abcde");
-    ASSERT_EQ(10, option_defs2.size());
+    OptionDefContainerPtr option_defs2 = cfg_mgr.getOptionDefs("abcde");
+    ASSERT_TRUE(option_defs2);
+    ASSERT_EQ(10, option_defs2->size());
 
 
     // Check that the option codes are valid.
     // Check that the option codes are valid.
     code = 105;
     code = 105;
-    for (OptionDefContainer::const_iterator it = option_defs2.begin();
-         it != option_defs2.end(); ++it, ++code) {
+    for (OptionDefContainer::const_iterator it = option_defs2->begin();
+         it != option_defs2->end(); ++it, ++code) {
         OptionDefinitionPtr def(*it);
         OptionDefinitionPtr def(*it);
         ASSERT_TRUE(def);
         ASSERT_TRUE(def);
         EXPECT_EQ(code, def->getCode());
         EXPECT_EQ(code, def->getCode());
@@ -104,8 +106,9 @@ TEST_F(CfgMgrTest, getOptionDefs) {
 
 
     // Let's make one more check that the empty set is returned when
     // Let's make one more check that the empty set is returned when
     // invalid option space is used.
     // invalid option space is used.
-    const OptionDefContainer& option_defs3 = cfg_mgr.getOptionDefs("non-existing");
-    ASSERT_TRUE(option_defs3.empty());
+    OptionDefContainerPtr option_defs3 = cfg_mgr.getOptionDefs("non-existing");
+    ASSERT_TRUE(option_defs3);
+    EXPECT_TRUE(option_defs3->empty());
 }
 }
 
 
 // This test verifies that single option definition is correctly
 // This test verifies that single option definition is correctly

+ 33 - 28
src/lib/dhcpsrv/tests/subnet_unittest.cc

@@ -274,43 +274,48 @@ TEST(Subnet6Test, addOptions) {
     }
     }
 
 
     // Get options from the Subnet and check if all 10 are there.
     // Get options from the Subnet and check if all 10 are there.
-    Subnet::OptionContainer options = subnet->getOptions("dhcp6");
-    ASSERT_EQ(10, options.size());
+    Subnet::OptionContainerPtr options = subnet->getOptionDescriptors("dhcp6");
+    ASSERT_TRUE(options);
+    ASSERT_EQ(10, options->size());
 
 
     // Validate codes of options added to dhcp6 option space.
     // Validate codes of options added to dhcp6 option space.
     uint16_t expected_code = 100;
     uint16_t expected_code = 100;
-    for (Subnet::OptionContainer::const_iterator option_desc = options.begin();
-         option_desc != options.end(); ++option_desc) {
+    for (Subnet::OptionContainer::const_iterator option_desc = options->begin();
+         option_desc != options->end(); ++option_desc) {
         ASSERT_TRUE(option_desc->option);
         ASSERT_TRUE(option_desc->option);
         EXPECT_EQ(expected_code, option_desc->option->getType());
         EXPECT_EQ(expected_code, option_desc->option->getType());
         ++expected_code;
         ++expected_code;
     }
     }
 
 
-    options = subnet->getOptions("isc");
-    ASSERT_EQ(7, options.size());
+    options = subnet->getOptionDescriptors("isc");
+    ASSERT_TRUE(options);
+    ASSERT_EQ(7, options->size());
 
 
     // Validate codes of options added to isc option space.
     // Validate codes of options added to isc option space.
     expected_code = 105;
     expected_code = 105;
-    for (Subnet::OptionContainer::const_iterator option_desc = options.begin();
-         option_desc != options.end(); ++option_desc) {
+    for (Subnet::OptionContainer::const_iterator option_desc = options->begin();
+         option_desc != options->end(); ++option_desc) {
         ASSERT_TRUE(option_desc->option);
         ASSERT_TRUE(option_desc->option);
         EXPECT_EQ(expected_code, option_desc->option->getType());
         EXPECT_EQ(expected_code, option_desc->option->getType());
         ++expected_code;
         ++expected_code;
     }
     }
 
 
     // Try to get options from a non-existing option space.
     // Try to get options from a non-existing option space.
-    options = subnet->getOptions("abcd");
-    EXPECT_TRUE(options.empty());
+    options = subnet->getOptionDescriptors("abcd");
+    ASSERT_TRUE(options);
+    EXPECT_TRUE(options->empty());
 
 
     // Delete options from all spaces.
     // Delete options from all spaces.
     subnet->delOptions();
     subnet->delOptions();
 
 
     // Make sure that all options have been removed.
     // Make sure that all options have been removed.
-    options = subnet->getOptions("dhcp6");
-    EXPECT_EQ(0, options.size());
+    options = subnet->getOptionDescriptors("dhcp6");
+    ASSERT_TRUE(options);
+    EXPECT_TRUE(options->empty());
 
 
-    options = subnet->getOptions("isc");
-    EXPECT_EQ(0, options.size());
+    options = subnet->getOptionDescriptors("isc");
+    ASSERT_TRUE(options);
+    EXPECT_TRUE(options->empty());
 }
 }
 
 
 TEST(Subnet6Test, addNonUniqueOptions) {
 TEST(Subnet6Test, addNonUniqueOptions) {
@@ -327,14 +332,14 @@ TEST(Subnet6Test, addNonUniqueOptions) {
     }
     }
 
 
     // Sanity check that all options are there.
     // Sanity check that all options are there.
-    Subnet::OptionContainer options = subnet->getOptions("dhcp6");
-    ASSERT_EQ(20, options.size());
+    Subnet::OptionContainerPtr options = subnet->getOptionDescriptors("dhcp6");
+    ASSERT_EQ(20, options->size());
 
 
     // Use container index #1 to get the options by their codes.
     // Use container index #1 to get the options by their codes.
-    Subnet::OptionContainerTypeIndex& idx = options.get<1>();
+    Subnet::OptionContainerTypeIndex& idx = options->get<1>();
     // Look for the codes 100-109.
     // Look for the codes 100-109.
     for (uint16_t code = 100; code < 110; ++ code) {
     for (uint16_t code = 100; code < 110; ++ code) {
-        // For each code we should get two instances of options.
+        // For each code we should get two instances of options->
         std::pair<Subnet::OptionContainerTypeIndex::const_iterator,
         std::pair<Subnet::OptionContainerTypeIndex::const_iterator,
                   Subnet::OptionContainerTypeIndex::const_iterator> range =
                   Subnet::OptionContainerTypeIndex::const_iterator> range =
             idx.equal_range(code);
             idx.equal_range(code);
@@ -359,8 +364,8 @@ TEST(Subnet6Test, addNonUniqueOptions) {
 
 
     subnet->delOptions();
     subnet->delOptions();
 
 
-    options = subnet->getOptions("dhcp6");
-    EXPECT_EQ(0, options.size());
+    options = subnet->getOptionDescriptors("dhcp6");
+    EXPECT_EQ(0, options->size());
 }
 }
 
 
 TEST(Subnet6Test, addInvalidOption) {
 TEST(Subnet6Test, addInvalidOption) {
@@ -401,20 +406,20 @@ TEST(Subnet6Test, addPersistentOption) {
     }
     }
 
 
     // Get added options from the subnet.
     // Get added options from the subnet.
-    Subnet::OptionContainer options = subnet->getOptions("dhcp6");
+    Subnet::OptionContainerPtr options = subnet->getOptionDescriptors("dhcp6");
 
 
-    // options.get<2> returns reference to container index #2. This
+    // options->get<2> returns reference to container index #2. This
     // index is used to access options by the 'persistent' flag.
     // index is used to access options by the 'persistent' flag.
-    Subnet::OptionContainerPersistIndex& idx = options.get<2>();
+    Subnet::OptionContainerPersistIndex& idx = options->get<2>();
 
 
-    // Get all persistent options.
+    // Get all persistent options->
     std::pair<Subnet::OptionContainerPersistIndex::const_iterator,
     std::pair<Subnet::OptionContainerPersistIndex::const_iterator,
               Subnet::OptionContainerPersistIndex::const_iterator> range_persistent =
               Subnet::OptionContainerPersistIndex::const_iterator> range_persistent =
         idx.equal_range(true);
         idx.equal_range(true);
     // 3 out of 10 options have been flagged persistent.
     // 3 out of 10 options have been flagged persistent.
     ASSERT_EQ(7, distance(range_persistent.first, range_persistent.second));
     ASSERT_EQ(7, distance(range_persistent.first, range_persistent.second));
 
 
-    // Get all non-persistent options.
+    // Get all non-persistent options->
     std::pair<Subnet::OptionContainerPersistIndex::const_iterator,
     std::pair<Subnet::OptionContainerPersistIndex::const_iterator,
               Subnet::OptionContainerPersistIndex::const_iterator> range_non_persistent =
               Subnet::OptionContainerPersistIndex::const_iterator> range_non_persistent =
         idx.equal_range(false);
         idx.equal_range(false);
@@ -423,11 +428,11 @@ TEST(Subnet6Test, addPersistentOption) {
 
 
     subnet->delOptions();
     subnet->delOptions();
 
 
-    options = subnet->getOptions("dhcp6");
-    EXPECT_EQ(0, options.size());
+    options = subnet->getOptionDescriptors("dhcp6");
+    EXPECT_EQ(0, options->size());
 }
 }
 
 
-TEST(Subnet6Test, getOptionSingle) {
+TEST(Subnet6Test, getOptionDescriptor) {
     Subnet6Ptr subnet(new Subnet6(IOAddress("2001:db8::"), 56, 1, 2, 3, 4));
     Subnet6Ptr subnet(new Subnet6(IOAddress("2001:db8::"), 56, 1, 2, 3, 4));
 
 
     // Add 10 options to a "dhcp6" option space in the subnet.
     // Add 10 options to a "dhcp6" option space in the subnet.