Browse Source

[2314] Changes as a result of the review.

Marcin Siodelski 12 years ago
parent
commit
0be5967e2f

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

@@ -1366,10 +1366,9 @@ private:
     /// @param option_space a name of the encapsulated option space.
     /// @param option_space a name of the encapsulated option space.
     /// @param option option instance to append sub-options to.
     /// @param option option instance to append sub-options to.
     void appendSubOptions(const std::string& option_space, OptionPtr& option) {
     void appendSubOptions(const std::string& option_space, OptionPtr& option) {
-        // If invalid pointer there is nothing to do.
-        if (!option) {
-            return;
-        }
+        // Only non-NULL options are stored in option container.
+        // If this option pointer is NULL this is a serious error.
+        assert(option);
 
 
         OptionDefinitionPtr def;
         OptionDefinitionPtr def;
         if (option_space == "dhcp4" &&
         if (option_space == "dhcp4" &&
@@ -1398,7 +1397,7 @@ private:
             assert(def);
             assert(def);
         }
         }
 
 
-        // We need to get option definition fo the particular option space
+        // We need to get option definition for the particular option space
         // and code. This definition holds the information whether our
         // and code. This definition holds the information whether our
         // option encapsulates any option space.
         // option encapsulates any option space.
         // Get the encapsulated option space name.
         // Get the encapsulated option space name.

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

@@ -1391,10 +1391,9 @@ private:
     /// @param option_space a name of the encapsulated option space.
     /// @param option_space a name of the encapsulated option space.
     /// @param option option instance to append sub-options to.
     /// @param option option instance to append sub-options to.
     void appendSubOptions(const std::string& option_space, OptionPtr& option) {
     void appendSubOptions(const std::string& option_space, OptionPtr& option) {
-        // If invalid pointer there is nothing to do.
-        if (!option) {
-            return;
-        }
+        // Only non-NULL options are stored in option container.
+        // If this option pointer is NULL this is a serious error.
+        assert(option);
 
 
         OptionDefinitionPtr def;
         OptionDefinitionPtr def;
         if (option_space == "dhcp6" &&
         if (option_space == "dhcp6" &&
@@ -1423,7 +1422,7 @@ private:
             assert(def);
             assert(def);
         }
         }
 
 
-        // We need to get option definition fo the particular option space
+        // We need to get option definition for the particular option space
         // and code. This definition holds the information whether our
         // and code. This definition holds the information whether our
         // option encapsulates any option space.
         // option encapsulates any option space.
         // Get the encapsulated option space name.
         // Get the encapsulated option space name.

+ 1 - 1
src/lib/dhcp/option_custom.h

@@ -1,4 +1,4 @@
-// Copyright (C) 2012 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2012-2013 Internet Systems Consortium, Inc. ("ISC")
 //
 //
 // Permission to use, copy, modify, and/or distribute this software for any
 // Permission to use, copy, modify, and/or distribute this software for any
 // purpose with or without fee is hereby granted, provided that the above
 // purpose with or without fee is hereby granted, provided that the above

+ 5 - 6
src/lib/dhcp/option_definition.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2012 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2012-2013 Internet Systems Consortium, Inc. ("ISC")
 //
 //
 // Permission to use, copy, modify, and/or distribute this software for any
 // Permission to use, copy, modify, and/or distribute this software for any
 // purpose with or without fee is hereby granted, provided that the above
 // purpose with or without fee is hereby granted, provided that the above
@@ -66,13 +66,12 @@ OptionDefinition::OptionDefinition(const std::string& name,
                                    const char* encapsulated_space)
                                    const char* encapsulated_space)
     : name_(name),
     : name_(name),
       code_(code),
       code_(code),
-      type_(OPT_UNKNOWN_TYPE),
+      // Data type is held as enum value by this class.
+      // Use the provided option type string to get the
+      // corresponding enum value.
+      type_(OptionDataTypeUtil::getDataType(type)),
       array_type_(false),
       array_type_(false),
       encapsulated_space_(encapsulated_space) {
       encapsulated_space_(encapsulated_space) {
-    // Data type is held as enum value by this class.
-    // Use the provided option type string to get the
-    // corresponding enum value.
-    type_ = OptionDataTypeUtil::getDataType(type);
 }
 }
 
 
 OptionDefinition::OptionDefinition(const std::string& name,
 OptionDefinition::OptionDefinition(const std::string& name,

+ 2 - 2
src/lib/dhcp/option_definition.h

@@ -234,14 +234,14 @@ public:
     /// @brief Return name of the encapsulated option space.
     /// @brief Return name of the encapsulated option space.
     ///
     ///
     /// @return name of the encapsulated option space.
     /// @return name of the encapsulated option space.
-    const std::string& getEncapsulatedSpace() const {
+    std::string getEncapsulatedSpace() const {
         return (encapsulated_space_);
         return (encapsulated_space_);
     }
     }
 
 
     /// @brief Return option name.
     /// @brief Return option name.
     ///
     ///
     /// @return option name.
     /// @return option name.
-    const std::string& getName() const { return (name_); }
+    std::string getName() const { return (name_); }
 
 
     /// @brief Return list of record fields.
     /// @brief Return list of record fields.
     ///
     ///

+ 1 - 1
src/lib/dhcp/tests/libdhcp++_unittest.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2011-2012 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2011-2013 Internet Systems Consortium, Inc. ("ISC")
 //
 //
 // Permission to use, copy, modify, and/or distribute this software for any
 // Permission to use, copy, modify, and/or distribute this software for any
 // purpose with or without fee is hereby granted, provided that the above
 // purpose with or without fee is hereby granted, provided that the above

+ 1 - 1
src/lib/dhcp/tests/option_definition_unittest.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2012 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2012-2013 Internet Systems Consortium, Inc. ("ISC")
 //
 //
 // Permission to use, copy, modify, and/or distribute this software for any
 // Permission to use, copy, modify, and/or distribute this software for any
 // purpose with or without fee is hereby granted, provided that the above
 // purpose with or without fee is hereby granted, provided that the above