Browse Source

[3036] Addressed comments from the second review.

Marcin Siodelski 11 years ago
parent
commit
51e47dbf6c
2 changed files with 30 additions and 2 deletions
  1. 8 2
      src/bin/dhcp6/tests/dhcp6_srv_unittest.cc
  2. 22 0
      src/lib/dhcp/option6_client_fqdn.cc

+ 8 - 2
src/bin/dhcp6/tests/dhcp6_srv_unittest.cc

@@ -1851,8 +1851,11 @@ TEST_F(FqdnDhcpv6SrvTest, createNameChangeRequests) {
     addIA(2345, IOAddress("2001:db8:1::2"), answer);
     addIA(2345, IOAddress("2001:db8:1::2"), answer);
     addIA(3456, IOAddress("2001:db8:1::3"), answer);
     addIA(3456, IOAddress("2001:db8:1::3"), answer);
 
 
+    // Use domain name in upper case. It should be converted to lower-case
+    // before DHCID is calculated. So, we should get the same result as if
+    // we typed domain name in lower-case.
     Option6ClientFqdnPtr fqdn = createClientFqdn(Option6ClientFqdn::FLAG_S,
     Option6ClientFqdnPtr fqdn = createClientFqdn(Option6ClientFqdn::FLAG_S,
-                                                 "myhost.example.com",
+                                                 "MYHOST.EXAMPLE.COM",
                                                  Option6ClientFqdn::FULL);
                                                  Option6ClientFqdn::FULL);
 
 
     // Create NameChangeRequests. Since we have added 3 IAs, it should
     // Create NameChangeRequests. Since we have added 3 IAs, it should
@@ -1890,7 +1893,10 @@ TEST_F(FqdnDhcpv6SrvTest, createRemovalNameChangeRequestFwdRev) {
 
 
     lease_->fqdn_fwd_ = true;
     lease_->fqdn_fwd_ = true;
     lease_->fqdn_rev_ = true;
     lease_->fqdn_rev_ = true;
-    lease_->hostname_ = "myhost.example.com.";
+    // Part of the domain name is in upper case, to test that it gets converted
+    // to lower case before DHCID is computed. So, we should get the same DHCID
+    // as if we typed domain-name in lower case.
+    lease_->hostname_ = "MYHOST.example.com.";
 
 
     ASSERT_NO_THROW(srv.createRemovalNameChangeRequest(lease_));
     ASSERT_NO_THROW(srv.createRemovalNameChangeRequest(lease_));
 
 

+ 22 - 0
src/lib/dhcp/option6_client_fqdn.cc

@@ -107,6 +107,11 @@ public:
 Option6ClientFqdnImpl::
 Option6ClientFqdnImpl::
 Option6ClientFqdnImpl(const uint8_t flags,
 Option6ClientFqdnImpl(const uint8_t flags,
                       const std::string& domain_name,
                       const std::string& domain_name,
+                      // cppcheck 1.57 complains that const enum value is not
+                      // passed by reference. Note that it accepts the non-const
+                      // enum to be passed by value. In both cases it is
+                      // unnecessary to pass the enum by reference.
+                      // cppcheck-suppress passedByValue
                       const Option6ClientFqdn::DomainNameType name_type)
                       const Option6ClientFqdn::DomainNameType name_type)
     : flags_(flags),
     : flags_(flags),
       domain_name_(),
       domain_name_(),
@@ -138,6 +143,9 @@ Option6ClientFqdnImpl(const Option6ClientFqdnImpl& source)
 }
 }
 
 
 Option6ClientFqdnImpl&
 Option6ClientFqdnImpl&
+// This assignment operator handles assignment to self. It uses copy
+// constructor of Option6ClientFqdnImpl to copy all required values.
+// cppcheck-suppress operatorEqToSelf
 Option6ClientFqdnImpl::operator=(const Option6ClientFqdnImpl& source) {
 Option6ClientFqdnImpl::operator=(const Option6ClientFqdnImpl& source) {
     if (source.domain_name_) {
     if (source.domain_name_) {
         domain_name_.reset(new isc::dns::Name(*source.domain_name_));
         domain_name_.reset(new isc::dns::Name(*source.domain_name_));
@@ -157,6 +165,11 @@ Option6ClientFqdnImpl::operator=(const Option6ClientFqdnImpl& source) {
 void
 void
 Option6ClientFqdnImpl::
 Option6ClientFqdnImpl::
 setDomainName(const std::string& domain_name,
 setDomainName(const std::string& domain_name,
+              // cppcheck 1.57 complains that const enum value is not
+              // passed by reference. Note that it accepts the non-const
+              // enum to be passed by value. In both cases it is
+              // unnecessary to pass the enum by reference.
+              // cppcheck-suppress passedByValue
               const Option6ClientFqdn::DomainNameType name_type) {
               const Option6ClientFqdn::DomainNameType name_type) {
     // domain-name must be trimmed. Otherwise, string comprising spaces only
     // domain-name must be trimmed. Otherwise, string comprising spaces only
     // would be treated as a fully qualified name.
     // would be treated as a fully qualified name.
@@ -352,6 +365,11 @@ Option6ClientFqdn::getDomainName() const {
 
 
 void
 void
 Option6ClientFqdn::packDomainName(isc::util::OutputBuffer& buf) const {
 Option6ClientFqdn::packDomainName(isc::util::OutputBuffer& buf) const {
+    // There is nothing to do if domain-name is empty.
+    if (!impl_->domain_name_) {
+        return;
+    }
+
     // Domain name, encoded as a set of labels.
     // Domain name, encoded as a set of labels.
     isc::dns::LabelSequence labels(*impl_->domain_name_);
     isc::dns::LabelSequence labels(*impl_->domain_name_);
     if (labels.getDataLength() > 0) {
     if (labels.getDataLength() > 0) {
@@ -395,6 +413,10 @@ Option6ClientFqdn::unpack(OptionBufferConstIter first,
                           OptionBufferConstIter last) {
                           OptionBufferConstIter last) {
     setData(first, last);
     setData(first, last);
     impl_->parseWireData(first, last);
     impl_->parseWireData(first, last);
+    // Check that the flags in the received option are valid. Ignore MBZ bits
+    // because we don't want to discard the whole option because of MBZ bits
+    // being set.
+    impl_->checkFlags(impl_->flags_, false);
 }
 }
 
 
 std::string
 std::string