[1350] Changes after review:

- len() returns now uint16_t type
- using pre-increment intead of post-increment
- using distance() function
- moved some simple implementation to header

src/lib/dhcp/option.cc

@@ -203,7 +203,7 @@ Option::unpack6(const boost::shared_array<uint8_t>& buf,
 
 /// Returns length of the complete option (data length + DHCPv4/DHCPv6
 /// option header)
-unsigned short
+uint16_t
 Option::len() {
 
     // length of the whole option is header and data stored in this option...
@@ -278,17 +278,7 @@ std::string Option::toText(int indent /* =0 */ ) {
     return tmp.str();
 }
 
-unsigned short
-Option::getType() {
-    return type_;
-}
-
-const std::vector<uint8_t>&
-Option::getData() {
-    return (data_);
-}
-
-unsigned short
+uint16_t
 Option::getHeaderLen() {
     switch (universe_) {
     case V4:

src/lib/dhcp/option.h

@@ -180,20 +180,19 @@ public:
     /// Returns option type (0-255 for DHCPv4, 0-65535 for DHCPv6)
     ///
     /// @return option type
-    unsigned short
-    getType();
+    unsigned short getType() { return (type_); }
 
     /// Returns length of the complete option (data length + DHCPv4/DHCPv6
     /// option header)
     ///
     /// @return length of the option
-    virtual unsigned short
+    virtual uint16_t
     len();
 
     /// @brief Returns length of header (2 for v4, 4 for v6)
     ///
     /// @return length of option header
-    virtual unsigned short
+    virtual uint16_t
     getHeaderLen();
 
     /// returns if option is valid (e.g. option may be truncated)
@@ -204,9 +203,9 @@ public:
 
     /// Returns pointer to actual data.
     ///
-    /// @return pointer to actual data (or NULL if there is no data)
-    virtual const std::vector<uint8_t>&
-    getData();
+    /// @return pointer to actual data (or reference to an empty vector
+    ///         if there is no data)
+    virtual const std::vector<uint8_t>& getData() { return (data_); }
 
     /// Adds a sub-option.
     ///

src/lib/dhcp/option4_addrlst.cc

@@ -42,10 +42,9 @@ Option4AddrLst::Option4AddrLst(uint8_t type,
                                vector<uint8_t>::const_iterator first,
                                vector<uint8_t>::const_iterator last)
     :Option(V4, type) {
-    vector<uint8_t> buf = std::vector<uint8_t>(first, last);
-    if ( (buf.size() % V4ADDRESS_LEN) ) {
+    if ( (distance(first, last) % V4ADDRESS_LEN) ) {
         isc_throw(OutOfRange, "DHCPv4 Option4AddrLst " << type_
-                  << " has invalid length=" << buf.size()
+                  << " has invalid length=" << distance(first, last)
                   << ", must be divisible by 4.");
     }
 
@@ -79,7 +78,7 @@ Option4AddrLst::pack4(isc::util::OutputBuffer& buf) {
 
     while (addr != addrs_.end()) {
         buf.writeUint32(*addr);
-        addr++;
+        ++addr;
     }
 }
 
@@ -112,8 +111,7 @@ void Option4AddrLst::addAddress(const isc::asiolink::IOAddress& addr) {
     addrs_.push_back(addr);
 }
 
-unsigned short
-Option4AddrLst::len() {
+uint16_t Option4AddrLst::len() {
 
     // Returns length of the complete option (option header + data length)
     return (getHeaderLen() + addrs_.size() * V4ADDRESS_LEN);
@@ -122,14 +120,15 @@ Option4AddrLst::len() {
 std::string Option4AddrLst::toText(int indent /* =0 */ ) {
     std::stringstream tmp;
 
-    for (int i = 0; i < indent; i++)
+    for (int i = 0; i < indent; i++) {
         tmp << " ";
+    }
 
-    tmp << "type=" << type_ << ", len=" << len()-getHeaderLen() << ": ";
+    tmp << "type=" << type_ << ", len=" << len()-getHeaderLen() << ":";
 
     for (AddressContainer::const_iterator addr = addrs_.begin();
          addr != addrs_.end(); ++addr) {
-        tmp << (*addr) << " ";
+        tmp << " " << (*addr);
     }
 
     return tmp.str();

src/lib/dhcp/option4_addrlst.h

@@ -53,7 +53,7 @@ public:
     /// @param addrs container with a list of addresses
     Option4AddrLst(uint8_t type, const AddressContainer& addrs);
 
-    /// @brief Constrcutor, creates an option with a single address.
+    /// @brief Constructor, creates an option with a single address.
     ///
     /// Creates an option that contains a single address.
     ///
@@ -63,9 +63,6 @@ public:
 
     /// @brief Constructor, used for received options.
     ///
-    /// This contructor is similar to the previous one, but it does not take
-    /// the whole vector<uint8_t>, but rather subset of it.
-    ///
     /// TODO: This can be templated to use different containers, not just
     /// vector. Prototype should look like this:
     /// template<typename InputIterator> Option(Universe u, uint16_t type,
@@ -106,8 +103,7 @@ public:
     /// option header)
     ///
     /// @return length of the option
-    virtual unsigned short
-    len();
+    virtual uint16_t len();
 
     /// @brief Returns vector with addresses.
     ///

src/lib/dhcp/option6_addrlst.cc

@@ -132,7 +132,7 @@ std::string Option6AddrLst::toText(int indent /* =0 */) {
     return tmp.str();
 }
 
-unsigned short Option6AddrLst::len() {
+uint16_t Option6AddrLst::len() {
 
     return (OPTION6_HDR_LEN + addrs_.size()*16);
 }

src/lib/dhcp/option6_addrlst.h

@@ -114,7 +114,7 @@ public:
     getAddresses() { return addrs_; };
 
     // returns data length (data length + DHCPv4/DHCPv6 option header)
-    virtual unsigned short len();
+    virtual uint16_t len();
 
 protected:
     AddressContainer addrs_;

src/lib/dhcp/option6_ia.cc

@@ -77,7 +77,7 @@ Option6IA::unpack(const boost::shared_array<uint8_t>& buf,
     if ( parse_len < OPTION6_IA_LEN || offset + OPTION6_IA_LEN > buf_len) {
         isc_throw(OutOfRange, "Option " << type_ << " truncated");
     }
-    
+
     iaid_ = readUint32(&buf[offset]);
     offset += sizeof(uint32_t);
 
@@ -121,9 +121,9 @@ std::string Option6IA::toText(int indent /* = 0*/) {
     return tmp.str();
 }
 
-unsigned short Option6IA::len() {
+uint16_t Option6IA::len() {
 
-    unsigned short length = OPTION6_HDR_LEN /*header (4)*/ +
+    uint16_t length = OPTION6_HDR_LEN /*header (4)*/ +
         OPTION6_IA_LEN  /* option content (12) */;
 
     // length of all suboptions

src/lib/dhcp/option6_ia.h

@@ -116,7 +116,7 @@ public:
     /// Returns length of this option, including option header and suboptions
     ///
     /// @return length of this option
-    virtual unsigned short
+    virtual uint16_t
     len();
 
 protected:

src/lib/dhcp/option6_iaaddr.cc

@@ -116,9 +116,9 @@ std::string Option6IAAddr::toText(int indent /* =0 */) {
     return tmp.str();
 }
 
-unsigned short Option6IAAddr::len() {
+uint16_t Option6IAAddr::len() {
 
-    unsigned short length = OPTION6_HDR_LEN + OPTION6_IAADDR_LEN;
+    uint16_t length = OPTION6_HDR_LEN + OPTION6_IAADDR_LEN;
 
     // length of all suboptions
     // TODO implement:

src/lib/dhcp/option6_iaaddr.h

@@ -126,8 +126,7 @@ public:
     getValid() const { return valid_; }
 
     /// returns data length (data length + DHCPv4/DHCPv6 option header)
-    virtual unsigned short
-    len();
+    virtual uint16_t len();
 
 protected:
     /// contains an IPv6 address

src/lib/dhcp/pkt4.cc

@@ -51,7 +51,6 @@ Pkt4::Pkt4(uint8_t msg_type, uint32_t transid)
       bufferOut_(DHCPV4_PKT_HDR_LEN),
       msg_type_(msg_type)
 {
-    /// TODO: fixed fields, uncomment in ticket #1224
     memset(chaddr_, 0, MAX_CHADDR_LEN);
     memset(sname_, 0, MAX_SNAME_LEN);
     memset(file_, 0, MAX_FILE_LEN);
@@ -64,7 +63,6 @@ Pkt4::Pkt4(const uint8_t* data, size_t len)
       ifindex_(-1),
       local_port_(DHCP4_SERVER_PORT),
       remote_port_(DHCP4_CLIENT_PORT),
-      /// TODO Fixed fields, uncomment in ticket #1224
       op_(BOOTREQUEST),
       transid_(0),
       secs_(0),