[#467] Addressed comments

diff --git a/src/bin/dhcp4/dhcp4_srv.cc b/src/bin/dhcp4/dhcp4_srv.cc
index 78cef4f97fca7381d23a63ecb7e87af8c93de2c9..1f45b7a6d2240e5108205c24544550be1911304f 100644 (file)
--- a/src/bin/dhcp4/dhcp4_srv.cc
+++ b/src/bin/dhcp4/dhcp4_srv.cc
@@ -1859,10 +1859,14 @@ Dhcpv4Srv::appendRequestedOptions(Dhcpv4Exchange& ex) {
         }
     }
 
-    // For each requested option code get the instance of the option
+    // For each requested option code get the first instance of the option
     // to be returned to the client.
     for (auto const& opt : requested_opts) {
         // Add nothing when it is already there.
+        // Skip special cases: DHO_VIVSO_SUBOPTIONS.
+        if (opt == DHO_VIVSO_SUBOPTIONS) {
+            continue;
+        }
         if (!resp->getOption(opt)) {
             // Iterate on the configured option list
             for (auto const& copts : co_list) {
@@ -1876,8 +1880,11 @@ Dhcpv4Srv::appendRequestedOptions(Dhcpv4Exchange& ex) {
         }
     }
 
+    // Special cases for vendor class and options which are identified
+    // by the code/type and the vendor/enterprise id vs. the code/type only.
     if (requested_opts.count(DHO_VIVCO_SUBOPTIONS) > 0) {
         set<uint32_t> vendor_ids;
+        // Get what already exists in the response.
         for (auto opt : resp->getOptions(DHO_VIVCO_SUBOPTIONS)) {
             OptionVendorClassPtr vendor_opts;
             vendor_opts = boost::dynamic_pointer_cast<OptionVendorClass>(opt.second);
@@ -1911,6 +1918,7 @@ Dhcpv4Srv::appendRequestedOptions(Dhcpv4Exchange& ex) {
 
     if (requested_opts.count(DHO_VIVSO_SUBOPTIONS) > 0) {
         set<uint32_t> vendor_ids;
+        // Get what already exists in the response.
         for (auto opt : resp->getOptions(DHO_VIVSO_SUBOPTIONS)) {
             OptionVendorPtr vendor_opts;
             vendor_opts = boost::dynamic_pointer_cast<OptionVendor>(opt.second);

diff --git a/src/bin/dhcp6/dhcp6_srv.cc b/src/bin/dhcp6/dhcp6_srv.cc
index 74f9004bfe581162dfb8c519e9801027623aadca..20189f09cf547e5044b0ea67b0aa1e279737df73 100644 (file)
--- a/src/bin/dhcp6/dhcp6_srv.cc
+++ b/src/bin/dhcp6/dhcp6_srv.cc
@@ -1518,6 +1518,8 @@ Dhcpv6Srv::appendRequestedOptions(const Pkt6Ptr& question, Pkt6Ptr& answer,
         }
     }
 
+    // For each requested option code get the first instance of the option
+    // to be returned to the client.
     for (uint16_t opt : requested_opts) {
         // Add nothing when it is already there.
         if (!answer->getOption(opt)) {
@@ -1534,9 +1536,11 @@ Dhcpv6Srv::appendRequestedOptions(const Pkt6Ptr& question, Pkt6Ptr& answer,
         }
     }
 
-    // Special cases for vendor class and options.
+    // Special cases for vendor class and options which are identified
+    // by the code/type and the vendor/enterprise id vs. the code/type only.
     if (requested_opts.count(D6O_VENDOR_CLASS) > 0) {
         set<uint32_t> vendor_ids;
+        // Get what already exists in the response.
         for (auto opt : answer->getOptions(D6O_VENDOR_CLASS)) {
             OptionVendorClassPtr vendor_class;
             vendor_class = boost::dynamic_pointer_cast<OptionVendorClass>(opt.second);
@@ -1572,6 +1576,7 @@ Dhcpv6Srv::appendRequestedOptions(const Pkt6Ptr& question, Pkt6Ptr& answer,
 
     if (requested_opts.count(D6O_VENDOR_OPTS) > 0) {
         set<uint32_t> vendor_ids;
+        // Get what already exists in the response.
         for (auto opt : answer->getOptions(D6O_VENDOR_OPTS)) {
             OptionVendorPtr vendor_opts;
             vendor_opts = boost::dynamic_pointer_cast<OptionVendor>(opt.second);

diff --git a/src/lib/dhcp/libdhcp++.cc b/src/lib/dhcp/libdhcp++.cc
index 9aa49cbe28cca265bb7c982924a3e75394217de1..f8ac769ed600ae8d666b5a37ad96797bce75dd2f 100644 (file)
--- a/src/lib/dhcp/libdhcp++.cc
+++ b/src/lib/dhcp/libdhcp++.cc
@@ -685,14 +685,12 @@ LibDHCP::fuseOptions4(OptionCollection& options) {
     return (result);
 }
 
-void
-LibDHCP::extendVendorOptions4(OptionCollection& options) {
-    LibDHCP::extendVivco(options);
-    LibDHCP::extendVivso(options);
-}
+namespace { // Anynomous namespace.
+
+// VIVCO part of extendVendorOptions4.
 
 void
-LibDHCP::extendVivco(OptionCollection& options) {
+extendVivco(OptionCollection& options) {
     typedef vector<OpaqueDataTuple> TuplesCollection;
     map<uint32_t, TuplesCollection> vendors_tuples;
     const auto& range = options.equal_range(DHO_VIVCO_SUBOPTIONS);
@@ -718,7 +716,7 @@ LibDHCP::extendVivco(OptionCollection& options) {
             } catch (const OpaqueDataTupleError&) {
                 // Ignore this kind of error and continue.
                 break;
-            } catch (const Exception&) {
+            } catch (const isc::Exception&) {
                 options.erase(DHO_VIVCO_SUBOPTIONS);
                 throw;
             }
@@ -749,8 +747,10 @@ LibDHCP::extendVivco(OptionCollection& options) {
     }
 }
 
+// VIVSO part of extendVendorOptions4.
+
 void
-LibDHCP::extendVivso(OptionCollection& options) {
+extendVivso(OptionCollection& options) {
     map<uint32_t, OptionCollection> vendors_data;
     const auto& range = options.equal_range(DHO_VIVSO_SUBOPTIONS);
     for (auto it = range.first; it != range.second; ++it) {
@@ -773,7 +773,7 @@ LibDHCP::extendVivso(OptionCollection& options) {
             } catch (const SkipThisOptionError&) {
                 // Ignore this kind of error and continue.
                 break;
-            } catch (const Exception&) {
+            } catch (const isc::Exception&) {
                 options.erase(DHO_VIVSO_SUBOPTIONS);
                 throw;
             }
@@ -796,6 +796,14 @@ LibDHCP::extendVivso(OptionCollection& options) {
     }
 }
 
+} // end of anonymous namespace.
+
+void
+LibDHCP::extendVendorOptions4(OptionCollection& options) {
+    extendVivco(options);
+    extendVivso(options);
+}
+
 size_t
 LibDHCP::unpackVendorOptions6(const uint32_t vendor_id, const OptionBuffer& buf,
                               OptionCollection& options) {

diff --git a/src/lib/dhcp/libdhcp++.h b/src/lib/dhcp/libdhcp++.h
index 241b50e2d5876e0007ae328bec51811124b7ce6d..452b2b9830ec9ec3adf0cd0d217979f69df47ecd 100644 (file)
--- a/src/lib/dhcp/libdhcp++.h
+++ b/src/lib/dhcp/libdhcp++.h
@@ -300,28 +300,12 @@ public:
     static bool fuseOptions4(isc::dhcp::OptionCollection& options);
 
     /// @brief Extend vendor options from fused options in multiple OptionVendor
-    /// options and add respective suboptions.
+    /// or OptionVendorClass options and add respective suboptions or values.
     ///
     /// @param options The option container which needs to be updated with
     /// extended vendor options.
     static void extendVendorOptions4(isc::dhcp::OptionCollection& options);
 
-    /// @brief Extend VIVCO options.
-    ///
-    /// VIVCO part of extendVendorOptions4.
-    ///
-    /// @param options The option container which needs to be updated with
-    /// extended vendor options.
-    static void extendVivco(isc::dhcp::OptionCollection& options);
-
-    /// @brief Extend VIVSO options.
-    ///
-    /// VIVSO part of extendVendorOptions4.
-    ///
-    /// @param options The option container which needs to be updated with
-    /// extended vendor options.
-    static void extendVivso(isc::dhcp::OptionCollection& options);
-
     /// @brief Parses provided buffer as DHCPv4 options and creates
     /// Option objects.
     ///