[2940] Addressed review comments.

8 files changed, 26 insertions, 15 deletions

diff --git a/src/lib/dhcp/duid.cc b/src/lib/dhcp/duid.cc
index 8570d26d7c..a1645d9d2b 100644
--- a/src/lib/dhcp/duid.cc
+++ b/src/lib/dhcp/duid.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
 // purpose with or without fee is hereby granted, provided that the above
@@ -104,7 +104,7 @@ ClientId::ClientId(const uint8_t *clientid, size_t len)
 }
 
 // Returns a copy of client-id data
-std::vector<uint8_t> ClientId::getClientId() const {
+const std::vector<uint8_t>& ClientId::getClientId() const {
     return (duid_);
 }
 
diff --git a/src/lib/dhcp/duid.h b/src/lib/dhcp/duid.h
index d20a58df9c..2110c447dc 100644
--- a/src/lib/dhcp/duid.h
+++ b/src/lib/dhcp/duid.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
 // purpose with or without fee is hereby granted, provided that the above
@@ -116,8 +116,13 @@ public:
     /// @brief Constructor based on array and array size
     ClientId(const uint8_t* clientid, size_t len);
 
-    /// @brief Returns reference to the client-id data
-    std::vector<uint8_t> getClientId() const;
+    /// @brief Returns reference to the client-id data.
+    ///
+    /// @warning Since this function returns a reference to the vector (not a
+    /// copy) the returned object must be used with caution because it remains
+    /// valid only for the time period when the object which returned it is
+    /// valid.
+    const std::vector<uint8_t>& getClientId() const;
 
     /// @brief Returns textual representation of a DUID (e.g. 00:01:02:03:ff)
     std::string toText() const;
diff --git a/src/lib/dhcpsrv/lease.cc b/src/lib/dhcpsrv/lease.cc
index fdc05c460e..98121d1791 100644
--- a/src/lib/dhcpsrv/lease.cc
+++ b/src/lib/dhcpsrv/lease.cc
@@ -74,10 +74,11 @@ Lease4::Lease4(const Lease4& other)
     }
 }
 
-std::vector<uint8_t>
+const std::vector<uint8_t>&
 Lease4::getClientIdVector() const {
     if(!client_id_) {
-        return std::vector<uint8_t>();
+        static std::vector<uint8_t> empty_vec;
+        return (empty_vec);
     }
 
     return (client_id_->getClientId());
diff --git a/src/lib/dhcpsrv/lease.h b/src/lib/dhcpsrv/lease.h
index 3f98491f8b..3734e9f2a9 100644
--- a/src/lib/dhcpsrv/lease.h
+++ b/src/lib/dhcpsrv/lease.h
@@ -211,9 +211,14 @@ struct Lease4 : public Lease {
 
     /// @brief Returns a client identifier.
     ///
-    /// @return A client identifier as vector, or an empty vector if client
-    /// identifier is NULL.
-    std::vector<uint8_t> getClientIdVector() const;
+    /// @warning Since the function returns the reference to a vector (not a
+    /// copy), the returned object should be used with caution because it will
+    /// remain valid only for the period of time when an object which returned
+    /// it exists.
+    ///
+    /// @return A reference to a vector holding client identifier,
+    /// or an empty vector if client identifier is NULL.
+    const std::vector<uint8_t>& getClientIdVector() const;
 
     /// @brief Assignment operator.
     ///
diff --git a/src/lib/dhcpsrv/memfile_lease_mgr.cc b/src/lib/dhcpsrv/memfile_lease_mgr.cc
index 075a08289a..dbc3bdd5ce 100644
--- a/src/lib/dhcpsrv/memfile_lease_mgr.cc
+++ b/src/lib/dhcpsrv/memfile_lease_mgr.cc
@@ -80,7 +80,7 @@ Memfile_LeaseMgr::getLease4(const HWAddr& hwaddr) const {
         lease != idx.end(); ++lease) {
 
         // Every Lease4 has a hardware address, so we can compare it
-        if((*lease)->hwaddr_ == hwaddr.hwaddr_) {
+        if ((*lease)->hwaddr_ == hwaddr.hwaddr_) {
             collection.push_back((*lease));
         }
     }
diff --git a/src/lib/dhcpsrv/memfile_lease_mgr.h b/src/lib/dhcpsrv/memfile_lease_mgr.h
index 44d07d3bd6..9acc444601 100644
--- a/src/lib/dhcpsrv/memfile_lease_mgr.h
+++ b/src/lib/dhcpsrv/memfile_lease_mgr.h
@@ -330,7 +330,7 @@ protected:
                 // lease: client id and subnet id.
                 boost::multi_index::composite_key<
                     Lease4,
-                    boost::multi_index::const_mem_fun<Lease4, std::vector<uint8_t>,
+                    boost::multi_index::const_mem_fun<Lease4, const std::vector<uint8_t>&,
                                                       &Lease4::getClientIdVector>,
                     // The subnet id is accessed through the subnet_id_ member.
                     boost::multi_index::member<Lease, uint32_t, &Lease::subnet_id_>
@@ -343,7 +343,7 @@ protected:
                 // lease: client id and subnet id.
                 boost::multi_index::composite_key<
                     Lease4,
-                    boost::multi_index::const_mem_fun<Lease4, std::vector<uint8_t>,
+                    boost::multi_index::const_mem_fun<Lease4, const std::vector<uint8_t>&,
                                                       &Lease4::getClientIdVector>,
                     // The hardware address is held in the hwaddr_ member of the
                     // Lease4 object.
diff --git a/src/lib/dhcpsrv/tests/test_utils.cc b/src/lib/dhcpsrv/tests/test_utils.cc
index 5018487379..7563cfdbbf 100644
--- a/src/lib/dhcpsrv/tests/test_utils.cc
+++ b/src/lib/dhcpsrv/tests/test_utils.cc
@@ -545,7 +545,7 @@ GenericLeaseMgrTest::testGetLease4HWAddr() {
 }
 
 void
-GenericLeaseMgrTest::testLease4ClientIdHWAddrSubnetId() {
+GenericLeaseMgrTest::testGetLease4ClientIdHWAddrSubnetId() {
     Lease4Ptr leaseA = initializeLease4(straddress4_[4]);
     Lease4Ptr leaseB = initializeLease4(straddress4_[5]);
     Lease4Ptr leaseC = initializeLease4(straddress4_[6]);
diff --git a/src/lib/dhcpsrv/tests/test_utils.h b/src/lib/dhcpsrv/tests/test_utils.h
index 45970db99c..6a8672436c 100644
--- a/src/lib/dhcpsrv/tests/test_utils.h
+++ b/src/lib/dhcpsrv/tests/test_utils.h
@@ -116,7 +116,7 @@ public:
     void testGetLease4HWAddr();
 
     /// @brief Test lease retrieval using client id, HW address and subnet id.
-    void testLease4ClientIdHWAddrSubnetId();
+    void testGetLease4ClientIdHWAddrSubnetId();
 
     // Member variables
     std::vector<std::string>  straddress4_;   ///< String forms of IPv4 addresses