Browse Source

[3589] Addressed review comments raised in the second review.

Marcin Siodelski 10 years ago
parent
commit
902bfdf4f0
3 changed files with 11 additions and 6 deletions
  1. 7 0
      src/lib/dhcpsrv/cfg_option.cc
  2. 2 2
      src/lib/dhcpsrv/cfg_option.h
  3. 2 4
      src/lib/dhcpsrv/libdhcpsrv.dox

+ 7 - 0
src/lib/dhcpsrv/cfg_option.cc

@@ -79,15 +79,22 @@ CfgOption::encapsulate() {
 
 void
 CfgOption::encapsulateInternal(const std::string& option_space) {
+    // Get all options for the particular option space.
     OptionContainerPtr options = getAll(option_space);
+    // For each option in the option space we will append sub-options
+    // from the option spaces they encapsulate.
     for (OptionContainer::const_iterator opt = options->begin();
          opt != options->end(); ++opt) {
+        // Get encapsulated option space for the option.
         const std::string& encap_space = opt->option_->getEncapsulatedSpace();
+        // Empty value means that no option space is encapsulated.
         if (!encap_space.empty()) {
+            // Retrieve all options from the encapsulated option space.
             OptionContainerPtr encap_options = getAll(encap_space);
             for (OptionContainer::const_iterator encap_opt =
                      encap_options->begin(); encap_opt != encap_options->end();
                  ++encap_opt) {
+                // Add sub-option if there isn't one added already.
                 if (!opt->option_->getOption(encap_opt->option_->getType())) {
                     opt->option_->addOption(encap_opt->option_);
                 }

+ 2 - 2
src/lib/dhcpsrv/cfg_option.h

@@ -331,8 +331,8 @@ public:
     /// If the option space name is specified in the following format:
     /// "vendor-X" where X is an uint32_t number, it is assumed to be
     /// a vendor space and the uint32_t number is returned by this function.
-    /// If the option space name is invalid this method will return 0 which
-    /// is not valid vendor-id.
+    /// If the option space name is invalid this method will return 0, which
+    /// is not a valid vendor-id, to signal an error.
     ///
     /// @todo remove this function once when the conversion is dealt by the
     /// appropriate functions returning options by option space names.

+ 2 - 4
src/lib/dhcpsrv/libdhcpsrv.dox

@@ -84,7 +84,7 @@ on migrating the other configuration parameters to it.
 @section optionsConfig Options Configuration Information
 
 The \ref isc::dhcp::CfgOption object holds a collection of options being
-sent to the client. Since, each subnet comes with a distnict set of
+sent to the client. Since each subnet comes with a distnict set of
 options, every \ref isc::dhcp::Subnet object holds its own copy of the
 \ref isc::dhcp::CfgOption object with specific options.
 
@@ -101,9 +101,7 @@ the \ref isc::dhcp::CfgOption instances for all subnets. This is
 causing some overhead during the reconfiguration of the server but on
 the other hand it avoids the lookup of options in two places (among
 subnet specific options and global options) during each packet
-processing. Note that the merge of global options to the subnet
-specific options doesn't affect the set of global options but only
-adds copies of global options to the subnet specific options.
+processing.
 
 One of the benefits of keeping a separate set of global options is
 that there may be cases when the server administrator doesn't specify