[4498] Unified container types holding option definitions in LibDHCP.

diff --git a/src/lib/dhcp/libdhcp++.cc b/src/lib/dhcp/libdhcp++.cc
index 50bb45ab29d64caf098e2717482a50fd363ba043..4a146ec303440067235f7ebc8b78d1b5da63cf79 100644 (file)
--- a/src/lib/dhcp/libdhcp++.cc
+++ b/src/lib/dhcp/libdhcp++.cc
@@ -50,6 +50,8 @@ VendorOptionDefContainers LibDHCP::vendor6_defs_;
 // Static container with option definitions created in runtime.
 StagedValue<OptionDefSpaceContainer> LibDHCP::runtime_option_defs_;
 
+// Null container.
+const OptionDefContainerPtr null_option_def_container_(new OptionDefContainer());
 
 // Those two vendor classes are used for cable modems:
 
@@ -61,7 +63,7 @@ const char* isc::dhcp::DOCSIS3_CLASS_EROUTER = "eRouter1.0";
 
 // Let's keep it in .cc file. Moving it to .h would require including optionDefParams
 // definitions there
-void initOptionSpace(OptionDefContainer& defs,
+void initOptionSpace(OptionDefContainerPtr& defs,
                      const OptionDefParams* params,
                      size_t params_size);
 
@@ -85,7 +87,7 @@ LibDHCP::getOptionDefs(const Option::Universe u) {
     }
 }
 
-const OptionDefContainer*
+const OptionDefContainerPtr&
 LibDHCP::getVendorOption4Defs(const uint32_t vendor_id) {
 
     if (vendor_id == VENDOR_ID_CABLE_LABS &&
@@ -96,12 +98,12 @@ LibDHCP::getVendorOption4Defs(const uint32_t vendor_id) {
     VendorOptionDefContainers::const_iterator def = vendor4_defs_.find(vendor_id);
     if (def == vendor4_defs_.end()) {
         // No such vendor-id space
-        return (NULL);
+        return (null_option_def_container_);
     }
-    return (&(def->second));
+    return (def->second);
 }
 
-const OptionDefContainer*
+const OptionDefContainerPtr&
 LibDHCP::getVendorOption6Defs(const uint32_t vendor_id) {
 
     if (vendor_id == VENDOR_ID_CABLE_LABS &&
@@ -117,9 +119,9 @@ LibDHCP::getVendorOption6Defs(const uint32_t vendor_id) {
     VendorOptionDefContainers::const_iterator def = vendor6_defs_.find(vendor_id);
     if (def == vendor6_defs_.end()) {
         // No such vendor-id space
-        return (NULL);
+        return (null_option_def_container_);
     }
-    return (&(def->second));
+    return (def->second);
 }
 
 OptionDefinitionPtr
@@ -149,12 +151,8 @@ LibDHCP::getOptionDef(const Option::Universe u, const std::string& name) {
 OptionDefinitionPtr
 LibDHCP::getVendorOptionDef(const Option::Universe u, const uint32_t vendor_id,
                             const std::string& name) {
-    const OptionDefContainer* defs = NULL;
-    if (u == Option::V4) {
-        defs = getVendorOption4Defs(vendor_id);
-    } else if (u == Option::V6) {
-        defs = getVendorOption6Defs(vendor_id);
-    }
+    OptionDefContainerPtr defs = (u == Option::V4 ? getVendorOption4Defs(vendor_id) :
+                                  getVendorOption6Defs(vendor_id));
 
     if (!defs) {
         return (OptionDefinitionPtr());
@@ -171,12 +169,8 @@ LibDHCP::getVendorOptionDef(const Option::Universe u, const uint32_t vendor_id,
 OptionDefinitionPtr
 LibDHCP::getVendorOptionDef(const Option::Universe u, const uint32_t vendor_id,
                             const uint16_t code) {
-    const OptionDefContainer* defs = NULL;
-    if (u == Option::V4) {
-        defs = getVendorOption4Defs(vendor_id);
-    } else if (u == Option::V6) {
-        defs = getVendorOption6Defs(vendor_id);
-    }
+    OptionDefContainerPtr defs = (u == Option::V4 ? getVendorOption4Defs(vendor_id) :
+                                  getVendorOption6Defs(vendor_id));
 
     if (!defs) {
         // Weird universe or unknown vendor_id. We don't care. No definitions
@@ -564,8 +558,7 @@ size_t LibDHCP::unpackVendorOptions6(const uint32_t vendor_id,
     size_t length = buf.size();
 
     // Get the list of option definitions for this particular vendor-id
-    const OptionDefContainer* option_defs =
-        LibDHCP::getVendorOption6Defs(vendor_id);
+    const OptionDefContainerPtr& option_defs = LibDHCP::getVendorOption6Defs(vendor_id);
 
     // Get the search index #1. It allows to search for option definitions
     // using option code. If there's no such vendor-id space, we're out of luck
@@ -657,8 +650,7 @@ size_t LibDHCP::unpackVendorOptions4(const uint32_t vendor_id, const OptionBuffe
     size_t offset = 0;
 
     // Get the list of stdandard option definitions.
-    const OptionDefContainer* option_defs =
-        LibDHCP::getVendorOption4Defs(vendor_id);
+    const OptionDefContainerPtr& option_defs = LibDHCP::getVendorOption4Defs(vendor_id);
     // Get the search index #1. It allows to search for option definitions
     // using option code.
     const OptionDefContainerTypeIndex* idx = NULL;
@@ -838,12 +830,12 @@ void LibDHCP::OptionFactoryRegister(Option::Universe u,
 
 void
 LibDHCP::initStdOptionDefs4() {
-    initOptionSpace(*v4option_defs_, OPTION_DEF_PARAMS4, OPTION_DEF_PARAMS_SIZE4);
+    initOptionSpace(v4option_defs_, OPTION_DEF_PARAMS4, OPTION_DEF_PARAMS_SIZE4);
 }
 
 void
 LibDHCP::initStdOptionDefs6() {
-    initOptionSpace(*v6option_defs_, OPTION_DEF_PARAMS6, OPTION_DEF_PARAMS_SIZE6);
+    initOptionSpace(v6option_defs_, OPTION_DEF_PARAMS6, OPTION_DEF_PARAMS_SIZE6);
 }
 
 void
@@ -853,20 +845,26 @@ LibDHCP::initVendorOptsDocsis4() {
 
 void
 LibDHCP::initVendorOptsDocsis6() {
-    vendor6_defs_[VENDOR_ID_CABLE_LABS] = OptionDefContainer();
     initOptionSpace(vendor6_defs_[VENDOR_ID_CABLE_LABS], DOCSIS3_V6_DEFS, DOCSIS3_V6_DEFS_SIZE);
 }
 
 void
 LibDHCP::initVendorOptsIsc6() {
-    vendor6_defs_[ENTERPRISE_ID_ISC] = OptionDefContainer();
     initOptionSpace(vendor6_defs_[ENTERPRISE_ID_ISC], ISC_V6_DEFS, ISC_V6_DEFS_SIZE);
 }
 
-void initOptionSpace(OptionDefContainer& defs,
+void initOptionSpace(OptionDefContainerPtr& defs,
                      const OptionDefParams* params,
                      size_t params_size) {
-    defs.clear();
+    // Container holding vendor options is typically not initialized, as it
+    // is held in map of null pointers. We need to initialize here in this
+    // case.
+    if (!defs) {
+        defs.reset(new OptionDefContainer());
+
+    } else {
+        defs->clear();
+    }
 
     for (size_t i = 0; i < params_size; ++i) {
         std::string encapsulates(params[i].encapsulates);
@@ -909,9 +907,9 @@ void initOptionSpace(OptionDefContainer& defs,
             // be only caused by programming error. To guarantee the
             // data consistency we clear all option definitions that
             // have been added so far and pass the exception forward.
-            defs.clear();
+            defs->clear();
             throw;
         }
-        defs.push_back(definition);
+        defs->push_back(definition);
     }
 }

diff --git a/src/lib/dhcp/libdhcp++.h b/src/lib/dhcp/libdhcp++.h
index ea3838d182b2e28966f8f3a7f0f0a08a563f4929..5eb4813afc9b88117d6b6cd02315c0d1c701364d 100644 (file)
--- a/src/lib/dhcp/libdhcp++.h
+++ b/src/lib/dhcp/libdhcp++.h
@@ -1,4 +1,4 @@
-// Copyright (C) 2011-2015 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2011-2016 Internet Systems Consortium, Inc. ("ISC")
 //
 // This Source Code Form is subject to the terms of the Mozilla Public
 // License, v. 2.0. If a copy of the MPL was not distributed with this
@@ -250,17 +250,17 @@ public:
     /// @brief Returns v4 option definitions for a given vendor
     ///
     /// @param vendor_id enterprise-id of a given vendor
-    /// @return a container for a given vendor (or NULL if not option
+    /// @return a container for a given vendor (or NULL if no option
     ///         definitions are defined)
-    static const OptionDefContainer*
+    static const OptionDefContainerPtr&
     getVendorOption4Defs(const uint32_t vendor_id);
 
     /// @brief Returns v6 option definitions for a given vendor
     ///
     /// @param vendor_id enterprise-id of a given vendor
-    /// @return a container for a given vendor (or NULL if not option
+    /// @return a container for a given vendor (or NULL if no option
     ///         definitions are defined)
-    static const OptionDefContainer*
+    static const OptionDefContainerPtr&
     getVendorOption6Defs(const uint32_t vendor_id);
 
     /// @brief Parses provided buffer as DHCPv6 vendor options and creates

diff --git a/src/lib/dhcp/option_definition.h b/src/lib/dhcp/option_definition.h
index 2d87ed9d1a92dc98bb5d3553f40df820b4a9606e..91fb9181e2d6155ac386135a0435514b91ced34a 100644 (file)
--- a/src/lib/dhcp/option_definition.h
+++ b/src/lib/dhcp/option_definition.h
@@ -739,7 +739,7 @@ typedef boost::multi_index_container<
 typedef boost::shared_ptr<OptionDefContainer> OptionDefContainerPtr;
 
 /// Container that holds various vendor option containers
-typedef std::map<uint32_t, OptionDefContainer> VendorOptionDefContainers;
+typedef std::map<uint32_t, OptionDefContainerPtr> VendorOptionDefContainers;
 
 /// Type of the index #1 - option type.
 typedef OptionDefContainer::nth_index<1>::type OptionDefContainerTypeIndex;

diff --git a/src/lib/dhcp/tests/libdhcp++_unittest.cc b/src/lib/dhcp/tests/libdhcp++_unittest.cc
index d4ad53186f638435a8e74beffa1bea534ee1ddfd..f9fa03ea3db07b06fdab5b19af0c307f15bd45d1 100644 (file)
--- a/src/lib/dhcp/tests/libdhcp++_unittest.cc
+++ b/src/lib/dhcp/tests/libdhcp++_unittest.cc
@@ -1654,9 +1654,9 @@ TEST_F(LibDhcpTest, getOptionDefByName4) {
 // This test checks if the definition of the DHCPv6 vendor option can
 // be searched by option name.
 TEST_F(LibDhcpTest, getVendorOptionDefByName6) {
-    const OptionDefContainer* defs =
+    const OptionDefContainerPtr& defs =
         LibDHCP::getVendorOption6Defs(VENDOR_ID_CABLE_LABS);
-    ASSERT_TRUE(defs != NULL);
+    ASSERT_TRUE(defs);
     for (OptionDefContainer::const_iterator def = defs->begin();
          def != defs->end(); ++def) {
         OptionDefinitionPtr def_by_name =
@@ -1670,9 +1670,9 @@ TEST_F(LibDhcpTest, getVendorOptionDefByName6) {
 // This test checks if the definition of the DHCPv4 vendor option can
 // be searched by option name.
 TEST_F(LibDhcpTest, getVendorOptionDefByName4) {
-    const OptionDefContainer* defs =
+    const OptionDefContainerPtr& defs =
         LibDHCP::getVendorOption4Defs(VENDOR_ID_CABLE_LABS);
-    ASSERT_TRUE(defs != NULL);
+    ASSERT_TRUE(defs);
     for (OptionDefContainer::const_iterator def = defs->begin();
          def != defs->end(); ++def) {
         OptionDefinitionPtr def_by_name =