[1226] Changes after review.

src/lib/asiolink/io_address.cc

@@ -50,6 +50,11 @@ IOAddress::IOAddress(const ip::address& asio_address) :
     asio_address_(asio_address)
 {}
 
+IOAddress::IOAddress(uint32_t v4address):
+    asio_address_(asio::ip::address_v4(v4address)) {
+
+}
+
 string
 IOAddress::toText() const {
     return (asio_address_.to_string());
@@ -71,11 +76,6 @@ IOAddress::from_bytes(short family, const uint8_t* data) {
     return IOAddress(string(addr_str));
 }
 
-IOAddress
-IOAddress::from_uint32(uint32_t v4address) {
-    return IOAddress(asio::ip::address_v4(v4address));
-}
-
 short
 IOAddress::getFamily() const {
     if (asio_address_.is_v4()) {

src/lib/asiolink/io_address.h

@@ -72,6 +72,15 @@ public:
     IOAddress(const asio::ip::address& asio_address);
     //@}
 
+    /// @brief Constructor for ip::address_v4 object.
+    ///
+    /// This constructor is intented to be used when constructing
+    /// IPv4 address out of uint32_t type. Passed value must be in
+    /// network byte order
+    ///
+    /// @param v4address IPv4 address represnted by uint32_t
+    IOAddress(uint32_t v4address);
+
     /// \brief Convert the address to a string.
     ///
     /// This method is basically expected to be exception free, but
@@ -104,15 +113,6 @@ public:
     static IOAddress
     from_bytes(short family, const uint8_t* data);
 
-    /// \brief Creates an IPv4 address from uint32 value
-    ///
-    /// \param v4address specified IPv4 address in network
-    ///        byte order
-    ///
-    /// \return Created IOAddress that holds IPv4 address
-    static IOAddress
-    from_uint32(uint32_t v4address);
-
     /// \brief Compare addresses for equality
     ///
     /// \param other Address to compare against.

src/lib/asiolink/tests/io_address_unittest.cc

@@ -95,7 +95,7 @@ TEST(IOAddressTest, uint32) {
     EXPECT_EQ(expected, tmp);
 
     // now let's try opposite conversion
-    IOAddress addr3 = IOAddress::from_uint32(expected);
+    IOAddress addr3 = IOAddress(expected);
 
     EXPECT_EQ(addr3.toText(), "192.0.2.5");
 }

src/lib/dhcp/pkt4.cc

@@ -128,10 +128,10 @@ Pkt4::unpack() {
     transid_ = bufferIn_.readUint32();
     secs_ = bufferIn_.readUint16();
     flags_ = bufferIn_.readUint16();
-    ciaddr_ = IOAddress::from_uint32(bufferIn_.readUint32());
-    yiaddr_ = IOAddress::from_uint32(bufferIn_.readUint32());
-    siaddr_ = IOAddress::from_uint32(bufferIn_.readUint32());
-    giaddr_ = IOAddress::from_uint32(bufferIn_.readUint32());
+    ciaddr_ = IOAddress(bufferIn_.readUint32());
+    yiaddr_ = IOAddress(bufferIn_.readUint32());
+    siaddr_ = IOAddress(bufferIn_.readUint32());
+    giaddr_ = IOAddress(bufferIn_.readUint32());
     bufferIn_.readData(chaddr_, MAX_CHADDR_LEN);
     bufferIn_.readData(sname_, MAX_SNAME_LEN);
     bufferIn_.readData(file_, MAX_FILE_LEN);

src/lib/dhcp/pkt4.h

@@ -105,7 +105,7 @@ public:
     ///
     /// @return hops field
     uint8_t
-    getHops() { return (hops_); };
+    getHops() const { return (hops_); };
 
     // Note: There's no need to manipulate OP field directly,
     // thus no setOp() method. See op_ comment.
@@ -114,7 +114,7 @@ public:
     ///
     /// @return op field
     uint8_t
-    getOp() { return (op_); };
+    getOp() const { return (op_); };
 
     /// Sets secs field
     ///
@@ -126,7 +126,7 @@ public:
     ///
     /// @return secs field
     uint16_t
-    getSecs() { return (secs_); };
+    getSecs() const { return (secs_); };
 
     /// Sets flags field
     ///
@@ -138,14 +138,14 @@ public:
     ///
     /// @return flags field
     uint16_t
-    getFlags() { return (flags_); };
+    getFlags() const { return (flags_); };
 
 
     /// Returns ciaddr field
     ///
     /// @return ciaddr field
-    isc::asiolink::IOAddress&
-    getCiaddr() { return (ciaddr_); };
+    const isc::asiolink::IOAddress&
+    getCiaddr() const { return (ciaddr_); };
 
     /// Sets ciaddr field
     ///
@@ -157,8 +157,8 @@ public:
     /// Returns siaddr field
     ///
     /// @return siaddr field
-    isc::asiolink::IOAddress&
-    getSiaddr() { return (siaddr_); };
+    const isc::asiolink::IOAddress&
+    getSiaddr() const { return (siaddr_); };
 
     /// Sets siaddr field
     ///
@@ -170,8 +170,8 @@ public:
     /// Returns yiaddr field
     ///
     /// @return yiaddr field
-    isc::asiolink::IOAddress&
-    getYiaddr() { return (yiaddr_); };
+    const isc::asiolink::IOAddress&
+    getYiaddr() const { return (yiaddr_); };
 
     /// Sets yiaddr field
     ///
@@ -183,8 +183,8 @@ public:
     /// Returns giaddr field
     ///
     /// @return giaddr field
-    isc::asiolink::IOAddress&
-    getGiaddr() { return (giaddr_); };
+    const isc::asiolink::IOAddress&
+    getGiaddr() const { return (giaddr_); };
 
     /// Sets giaddr field
     ///
@@ -195,13 +195,13 @@ public:
     /// Returns value of transaction-id field
     ///
     /// @return transaction-id
-    uint32_t getTransid() { return (transid_); };
+    uint32_t getTransid() const { return (transid_); };
 
     /// Returns message type (e.g. 1 = DHCPDISCOVER)
     ///
     /// @return message type
     uint8_t
-    getType() { return (msg_type_); }
+    getType() const { return (msg_type_); }
 
     /// Sets message type (e.g. 1 = DHCPDISCOVER)
     ///
@@ -215,7 +215,7 @@ public:
     ///
     /// @return sname field
     const std::vector<uint8_t>
-    getSname() { return (std::vector<uint8_t>(sname_, &sname_[MAX_SNAME_LEN])); };
+    getSname() const { return (std::vector<uint8_t>(sname_, &sname_[MAX_SNAME_LEN])); };
 
     /// Sets sname field
     ///
@@ -230,7 +230,7 @@ public:
     ///
     /// @return pointer to file field
     const std::vector<uint8_t>
-    getFile() { return (std::vector<uint8_t>(file_, &file_[MAX_FILE_LEN])); };
+    getFile() const { return (std::vector<uint8_t>(file_, &file_[MAX_FILE_LEN])); };
 
     /// Sets file field
     ///
@@ -256,13 +256,13 @@ public:
     ///
     /// @return hardware type
     uint8_t
-    getHtype() { return (htype_); };
+    getHtype() const { return (htype_); };
 
     /// Returns hlen field
     ///
     /// @return hardware address length
     uint8_t
-    getHlen() { return (hlen_); };
+    getHlen() const { return (hlen_); };
 
     /// @brief Returns chaddr field
     ///
@@ -271,20 +271,21 @@ public:
     ///
     /// @return pointer to hardware address
     const uint8_t*
-    getChaddr() { return (chaddr_); };
+    getChaddr() const { return (chaddr_); };
 
 
     /// Returns reference to output buffer
     ///
     /// Returned buffer will contain reasonable data only for
-    /// output (TX) packet and after pack() was called.
+    /// output (TX) packet and after pack() was called. This buffer
+    /// is only valid till Pkt4 object is valid.
     ///
     /// RX packet or TX packet before pack() will return buffer with
     /// zero length
     ///
     /// @return reference to output buffer
-    isc::util::OutputBuffer&
-    getBuffer() { return (bufferOut_); };
+    const isc::util::OutputBuffer&
+    getBuffer() const { return (bufferOut_); };
 
 protected:
 

src/lib/dhcp/tests/pkt4_unittest.cc

@@ -85,18 +85,18 @@ TEST(Pkt4Test, constructor) {
 }
 
 // a sample data
-const static uint8_t dummyOp = BOOTREQUEST;
-const static uint8_t dummyHtype = 6;
-const static uint8_t dummyHlen = 6;
-const static uint8_t dummyHops = 13;
-const static uint32_t dummyTransid = 0x12345678;
-const static uint16_t dummySecs = 42;
-const static uint16_t dummyFlags = BOOTP_BROADCAST;
-
-const static IOAddress dummyCiaddr("192.0.2.1");
-const static IOAddress dummyYiaddr("1.2.3.4");
-const static IOAddress dummySiaddr("192.0.2.255");
-const static IOAddress dummyGiaddr("255.255.255.255");
+const uint8_t dummyOp = BOOTREQUEST;
+const uint8_t dummyHtype = 6;
+const uint8_t dummyHlen = 6;
+const uint8_t dummyHops = 13;
+const uint32_t dummyTransid = 0x12345678;
+const uint16_t dummySecs = 42;
+const uint16_t dummyFlags = BOOTP_BROADCAST;
+
+const IOAddress dummyCiaddr("192.0.2.1");
+const IOAddress dummyYiaddr("1.2.3.4");
+const IOAddress dummySiaddr("192.0.2.255");
+const IOAddress dummyGiaddr("255.255.255.255");
 
 // a dummy MAC address
 const uint8_t dummyMacAddr[] = {0, 1, 2, 3, 4, 5};
@@ -233,8 +233,8 @@ TEST(Pkt4Test, fixedFieldsPack) {
     ASSERT_EQ(static_cast<size_t>(Pkt4::DHCPV4_PKT_HDR_LEN), pkt->len());
 
     // redundant but MUCH easier for debug in gdb
-    const uint8_t * exp = &expectedFormat[0];
-    const uint8_t * got = static_cast<const uint8_t*>(pkt->getBuffer().getData());
+    const uint8_t* exp = &expectedFormat[0];
+    const uint8_t* got = static_cast<const uint8_t*>(pkt->getBuffer().getData());
 
     EXPECT_EQ(0, memcmp(exp, got, Pkt4::DHCPV4_PKT_HDR_LEN));
 }
@@ -286,14 +286,14 @@ TEST(Pkt4Test, hwAddr) {
 
     Pkt4* pkt = 0;
     // let's test each hlen, from 0 till 16
-    for (int macLen=0; macLen < Pkt4::MAX_CHADDR_LEN; macLen++) {
-        for (int i=0; i < Pkt4::MAX_CHADDR_LEN; i++) {
+    for (int macLen = 0; macLen < Pkt4::MAX_CHADDR_LEN; macLen++) {
+        for (int i = 0; i < Pkt4::MAX_CHADDR_LEN; i++) {
             mac[i] = 0;
             expectedChaddr[i] = 0;
         }
-        for (int i=0; i < macLen; i++) {
-            mac[i] = 128+i;
-            expectedChaddr[i] = 128+i;
+        for (int i = 0; i < macLen; i++) {
+            mac[i] = 128 + i;
+            expectedChaddr[i] = 128 + i;
         }
 
         // type and transaction doesn't matter in this test
@@ -344,7 +344,7 @@ TEST(Pkt4Test, msgTypes) {
     };
 
     Pkt4* pkt = 0;
-    for (int i=0; i < sizeof(types)/sizeof(msgType); i++) {
+    for (int i = 0; i < sizeof(types) / sizeof(msgType); i++) {
 
         pkt = new Pkt4(types[i].dhcp, 0);
         EXPECT_EQ(types[i].dhcp, pkt->getType());
@@ -372,10 +372,10 @@ TEST(Pkt4Test, sname) {
     Pkt4* pkt = 0;
     // let's test each sname length, from 0 till 64
     for (int snameLen=0; snameLen < Pkt4::MAX_SNAME_LEN; snameLen++) {
-        for (int i=0; i < Pkt4::MAX_SNAME_LEN; i++) {
+        for (int i = 0; i < Pkt4::MAX_SNAME_LEN; i++) {
             sname[i] = 0;
         }
-        for (int i=0; i < snameLen; i++) {
+        for (int i = 0; i < snameLen; i++) {
             sname[i] = i;
         }
 
@@ -404,11 +404,11 @@ TEST(Pkt4Test, file) {
 
     Pkt4* pkt = 0;
     // Let's test each file length, from 0 till 128.
-    for (int fileLen=0; fileLen < Pkt4::MAX_FILE_LEN; fileLen++) {
-        for (int i=0; i < Pkt4::MAX_FILE_LEN; i++) {
+    for (int fileLen = 0; fileLen < Pkt4::MAX_FILE_LEN; fileLen++) {
+        for (int i = 0; i < Pkt4::MAX_FILE_LEN; i++) {
             file[i] = 0;
         }
-        for (int i=0; i < fileLen; i++) {
+        for (int i = 0; i < fileLen; i++) {
             file[i] = i;
         }