[1186] Part 6 of review changes.

- Added V6ADDRESS_LEN and V4ADDRESS_LEN
- Removed unnecessary parameters to Option constructors
- Many missing Doxygen comments added
- using sizeof(uintXX_t) instead of 2 and 4 in pack()/unpack()
- Many other smaller changes.

src/lib/asiolink/io_address.h

@@ -29,6 +29,12 @@
 namespace isc {
 namespace asiolink {
 
+    /// Defines length of IPv6 address.
+    const static size_t V6ADDRESS_LEN = 16;
+
+    /// Defines length of IPv4 address.
+    const static size_t V4ADDRESS_LEN = 4;
+
 /// \brief The \c IOAddress class represents an IP addresses (version
 /// agnostic)
 ///

src/lib/dhcp/libdhcp.cc

@@ -55,8 +55,7 @@ LibDHCP::unpackOptions6(const boost::shared_array<uint8_t> buf,
         case D6O_IA_NA:
         case D6O_IA_PD:
             // cout << "Creating Option6IA" << endl;
-            opt = boost::shared_ptr<Option>(new Option6IA(Option::V6,
-                                                          opt_type,
+            opt = boost::shared_ptr<Option>(new Option6IA(opt_type,
                                                           buf, buf_len,
                                                           offset,
                                                           opt_len));

src/lib/dhcp/option.h

@@ -106,7 +106,7 @@ public:
     /// @param offset offset, where start parsing option
     /// @param parse_len how many bytes should be parsed
     ///
-    /// @return offset after last parsed option
+    /// @return offset after last parsed octet
     virtual unsigned int
     unpack(boost::shared_array<uint8_t> buf,
            unsigned int buf_len,
@@ -174,7 +174,6 @@ public:
     /// @param type Type of option to be deleted.
     ///
     /// @return true if option was deleted, false if no such option existed
-    ///
     bool
     delOption(unsigned short type);
 

src/lib/dhcp/option6_addrlst.h

@@ -29,13 +29,10 @@ namespace dhcp {
 ///
 class Option6AddrLst: public Option {
 
-
 public:
     /// a container for (IPv6) addresses
     typedef std::vector<isc::asiolink::IOAddress> AddressContainer;
 
-    const static size_t V6ADDRESS_LEN = 16;
-
     /// @brief Constructor used during option generation.
     ///
     /// @param type option type

src/lib/dhcp/option6_ia.cc

@@ -25,18 +25,16 @@ using namespace std;
 using namespace isc;
 using namespace isc::dhcp;
 
-Option6IA::Option6IA(Universe u, unsigned short type, unsigned int iaid)
-    :Option(u, type), iaid_(iaid) {
-
+Option6IA::Option6IA(unsigned short type, unsigned int iaid)
+    :Option(Option::V6, type), iaid_(iaid) {
 }
 
-
-Option6IA::Option6IA(Universe u, unsigned short type,
+Option6IA::Option6IA(unsigned short type,
                    boost::shared_array<uint8_t> buf,
                    unsigned int buf_len,
                    unsigned int offset,
                    unsigned int option_len)
-    :Option(u, type) {
+    :Option(Option::V6, type) {
     unpack(buf, buf_len, offset, option_len);
 }
 
@@ -49,21 +47,26 @@ Option6IA::pack(boost::shared_array<uint8_t> buf,
                   << ", buffer=" << buf_len << ": too small buffer.");
     }
 
+    if (len() < 16 ) {
+        isc_throw(OutOfRange, "Attempt to build malformed IA option: len="
+                  << len() << " is too small (at least 16 is required).");
+    }
+
     uint8_t* ptr = &buf[offset];
     *(uint16_t*)ptr = htons(type_);
-    ptr += 2;
+    ptr += sizeof(uint16_t);
     *(uint16_t*)ptr = htons(len() - 4); // len() returns complete option length
     // len field contains length without 4-byte option header
-    ptr += 2;
+    ptr += sizeof(uint16_t);
 
     *(uint32_t*)ptr = htonl(iaid_);
-    ptr += 4;
+    ptr += sizeof(uint32_t);
 
     *(uint32_t*)ptr = htonl(t1_);
-    ptr += 4;
+    ptr += sizeof(uint32_t);
 
     *(uint32_t*)ptr = htonl(t2_);
-    ptr += 4;
+    ptr += sizeof(uint32_t);
 
     offset = LibDHCP::packOptions6(buf, buf_len, offset+16, options_);
     return offset;
@@ -74,17 +77,17 @@ Option6IA::unpack(boost::shared_array<uint8_t> buf,
                   unsigned int buf_len,
                   unsigned int offset,
                   unsigned int parse_len) {
-    if (parse_len<12 || offset+12>buf_len) {
+    if ( parse_len < OPTION6_IA_LEN || offset + OPTION6_IA_LEN > buf_len) {
         isc_throw(OutOfRange, "Option " << type_ << " truncated");
     }
     iaid_ = ntohl(*(uint32_t*)&buf[offset]);
-    offset +=4;
+    offset += sizeof(uint32_t);
     t1_ = ntohl(*(uint32_t*)&buf[offset]);
-    offset +=4;
+    offset += sizeof(uint32_t);
     t2_ = ntohl(*(uint32_t*)&buf[offset]);
-    offset +=4;
+    offset += sizeof(uint32_t);
     offset = LibDHCP::unpackOptions6(buf, buf_len, offset,
-                                     parse_len - 12, options_);
+                                     parse_len - OPTION6_IA_LEN, options_);
 
     return (offset);
 }
@@ -119,7 +122,8 @@ std::string Option6IA::toText(int indent /* = 0*/) {
 
 unsigned short Option6IA::len() {
 
-    unsigned short length = 4/*header*/ + 12 /* option content */; // header
+    unsigned short length = OPTION6_HDR_LEN /*header (4)*/ +
+        OPTION6_IA_LEN  /* option content (12) */;
 
     // length of all suboptions
     for (Option::Option6Collection::iterator it = options_.begin();

src/lib/dhcp/option6_ia.h

@@ -23,61 +23,110 @@ namespace dhcp {
 class Option6IA: public Option {
 
 public:
-    // ctor, used for options constructed, usually during transmission
-    Option6IA(Universe u, unsigned short type, unsigned int iaid);
-
-    // ctor, used for received options
-    // boost::shared_array allows sharing a buffer, but it requires that
-    // different instances share pointer to the whole array, not point
-    // to different elements in shared array. Therefore we need to share
-    // pointer to the whole array and remember offset where data for
-    // this option begins
-    Option6IA(Universe u, unsigned short type, boost::shared_array<uint8_t> buf,
-              unsigned int buf_len,
-              unsigned int offset,
-              unsigned int len);
-
-    // writes option in wire-format to buf, returns pointer to first unused
-    // byte after stored option
+    /// Length of IA_NA and IA_PD content
+    const static size_t OPTION6_IA_LEN = 12;
+
+    /// @brief ctor, used for options constructed, usually during transmission
+    ///
+    /// @param type option type (usually 4 for IA_NA, 25 for IA_PD)
+    /// @param iaid identity association identifier (id of IA)
+    Option6IA(unsigned short type, unsigned int iaid);
+
+    /// @brief ctor, used for received options
+    ///
+    /// boost::shared_array allows sharing a buffer, but it requires that
+    /// different instances share pointer to the whole array, not point
+    /// to different elements in shared array. Therefore we need to share
+    /// pointer to the whole array and remember offset where data for
+    /// this option begins
+    ///
+    /// @param type option type (usually 4 for IA_NA, 25 for IA_PD)
+    /// @param buf buffer to be parsed
+    /// @param buf_len buffer length
+    /// @param offset offset in buffer
+    /// @param len number of bytes to parse
+    Option6IA(unsigned short type, boost::shared_array<uint8_t> buf,
+              unsigned int buf_len, unsigned int offset, unsigned int len);
+
+    /// Writes option in wire-format to buf, returns pointer to first unused
+    /// byte after stored option.
+    ///
+    /// @param buf buffer (option will be stored here)
+    /// @param buf_len (buffer length)
+    /// @param offset offset place where option should be stored
+    ///
+    /// @return offset to the first unused byte after stored option
     unsigned int
     pack(boost::shared_array<uint8_t> buf, unsigned int buf_len,
          unsigned int offset);
 
-    // parses received buffer, returns offset to the first unused byte after
-    // parsed option
+    /// @brief Parses received buffer
+    ///
+    /// Parses received buffer and returns offset to the first unused byte after
+    /// parsed option.
+    ///
+    /// @param buf pointer to buffer
+    /// @param buf_len length of buf
+    /// @param offset offset, where start parsing option
+    /// @param parse_len how many bytes should be parsed
+    ///
+    /// @return offset after last parsed octet
     virtual unsigned int
-    unpack(boost::shared_array<uint8_t> buf,
-           unsigned int buf_len,
-           unsigned int offset,
-           unsigned int parse_len);
+    unpack(boost::shared_array<uint8_t> buf, unsigned int buf_len,
+           unsigned int offset, unsigned int parse_len);
 
     /// Provides human readable text representation
     ///
     /// @param indent number of leading space characters
     ///
     /// @return string with text represenation
-    ///
     virtual std::string
     toText(int indent = 0);
 
+    /// Sets T1 timer.
+    ///
+    /// @param t1 t1 value to be set
     void setT1(unsigned int t1) { t1_=t1; }
+
+
+    /// Sets T2 timer.
+    ///
+    /// @param t2 t2 value to be set
     void setT2(unsigned int t2) { t2_=t2; }
 
+    /// Returns IA identifier.
+    ///
+    /// @return IAID value.
+    ///
     unsigned int getIAID() { return iaid_; }
+
+    /// Returns T1 timer.
+    ///
+    /// @return T1 value.
     unsigned int getT1()   { return t1_; }
+
+    /// Returns T2 timer.
+    ///
+    /// @return T2 value.
     unsigned int getT2()   { return t2_; }
 
     /// @brief returns complete length of option
     ///
     /// Returns length of this option, including option header and suboptions
     ///
-    /// @return length
+    /// @return length of this option
     virtual unsigned short
     len();
 
 protected:
+
+    /// keeps IA identifier
     unsigned int iaid_;
+
+    /// keeps T1 timer value
     unsigned int t1_;
+
+    /// keeps T2 timer value
     unsigned int t2_;
 };
 

src/lib/dhcp/option6_iaaddr.cc

@@ -28,19 +28,17 @@ using namespace isc::dhcp;
 using namespace isc::asiolink;
 
 Option6IAAddr::Option6IAAddr(unsigned short type,
-                             isc::asiolink::IOAddress addr,
-                             unsigned int pref,
-                             unsigned int valid)
+                             const isc::asiolink::IOAddress& addr,
+                             unsigned int pref, unsigned int valid)
     :Option(V6, type), addr_(addr), preferred_(pref),
      valid_(valid) {
 }
 
 Option6IAAddr::Option6IAAddr(unsigned short type,
                              boost::shared_array<uint8_t> buf,
-                             unsigned int buf_len,
-                             unsigned int offset,
+                             unsigned int buf_len, unsigned int offset,
                              unsigned int option_len)
-    :Option(V6, type), addr_(IOAddress("::")) {
+    :Option(V6, type), addr_("::") {
     unpack(buf, buf_len, offset, option_len);
 }
 
@@ -54,18 +52,18 @@ Option6IAAddr::pack(boost::shared_array<uint8_t> buf,
     }
 
     *(uint16_t*)&buf[offset] = htons(type_);
-    offset += 2;
-    *(uint16_t*)&buf[offset] = htons(len()-4); // len() returns complete option
+    offset += sizeof(uint16_t);
+    *(uint16_t*)&buf[offset] = htons(len() - OPTION6_HDR_LEN); // len() returns complete option
     // length. len field contains length without 4-byte option header
-    offset += 2;
+    offset += sizeof(uint16_t);
 
     memcpy(&buf[offset], addr_.getAddress().to_v6().to_bytes().data(), 16);
-    offset += 16;
+    offset += V6ADDRESS_LEN;
 
     *(uint32_t*)&buf[offset] = htonl(preferred_);
-    offset += 4;
+    offset += sizeof(uint32_t);
     *(uint32_t*)&buf[offset] = htonl(valid_);
-    offset += 4;
+    offset += sizeof(uint32_t);
 
     // parse suboption (there shouldn't be any)
     offset = LibDHCP::packOptions6(buf, buf_len, offset, options_);
@@ -77,19 +75,19 @@ Option6IAAddr::unpack(boost::shared_array<uint8_t> buf,
                   unsigned int buf_len,
                   unsigned int offset,
                   unsigned int parse_len) {
-    if (parse_len<24 || offset+24>buf_len) {
+    if ( parse_len < OPTION6_IAADDR_LEN || offset + OPTION6_IAADDR_LEN > buf_len) {
         isc_throw(OutOfRange, "Option " << type_ << " truncated");
     }
 
     // 16 bytes: IPv6 address
     addr_ = IOAddress::from_bytes(AF_INET6, &buf[offset]);
-    offset += 16;
+    offset += V6ADDRESS_LEN;
 
     preferred_ = ntohl(*(uint32_t*)&buf[offset]);
-    offset +=4;
+    offset += sizeof(uint32_t);
 
     valid_ = ntohl(*(uint32_t*)&buf[offset]);
-    offset +=4;
+    offset += sizeof(uint32_t);
     offset = LibDHCP::unpackOptions6(buf, buf_len, offset,
                                      parse_len - 24, options_);
 
@@ -115,7 +113,7 @@ std::string Option6IAAddr::toText(int indent /* =0 */) {
 
 unsigned short Option6IAAddr::len() {
 
-    unsigned short length = 4/*header*/ + 24 /* content */; // header
+    unsigned short length = OPTION6_HDR_LEN + OPTION6_IAADDR_LEN;
 
     // length of all suboptions
     // TODO implement:

src/lib/dhcp/option6_iaaddr.h

@@ -24,53 +24,119 @@ namespace dhcp {
 class Option6IAAddr: public Option {
 
 public:
-    // ctor, used for options constructed, usually during transmission
-    Option6IAAddr(unsigned short type,
-                  isc::asiolink::IOAddress addr,
-                  unsigned int prefered,
-                  unsigned int valid);
-
-    // ctor, used for received options
-    // boost::shared_array allows sharing a buffer, but it requires that
-    // different instances share pointer to the whole array, not point
-    // to different elements in shared array. Therefore we need to share
-    // pointer to the whole array and remember offset where data for
-    // this option begins
+    /// length of the fixed part of the IAADDR option
+    static const size_t OPTION6_IAADDR_LEN = 24;
+
+    /// @brief ctor, used for options constructed (during transmission)
+    ///
+    /// @param type option type
+    /// @param addr reference to an address
+    /// @param preferred address preferred lifetime (in seconds)
+    /// @param valid address valid lifetime (in seconds)
+    Option6IAAddr(unsigned short type, const isc::asiolink::IOAddress& addr,
+                  unsigned int preferred, unsigned int valid);
+
+    /// ctor, used for received options
+    /// boost::shared_array allows sharing a buffer, but it requires that
+    /// different instances share pointer to the whole array, not point
+    /// to different elements in shared array. Therefore we need to share
+    /// pointer to the whole array and remember offset where data for
+    /// this option begins
+    ///
+    /// @param type option type
+    /// @param buf pointer to a buffer
+    /// @param offset offset to first data byte in that buffer
+    /// @param len data length of this option
     Option6IAAddr(unsigned short type, boost::shared_array<uint8_t> buf,
-                  unsigned int buf_len,
-                  unsigned int offset,
-                  unsigned int len);
+                  unsigned int buf_len, unsigned int offset, unsigned int len);
 
-    // writes option in wire-format to buf, returns pointer to first unused
-    // byte after stored option
+    /// @brief Writes option in wire-format.
+    ///
+    /// Writes option in wire-format to buf, returns pointer to first unused
+    /// byte after stored option.
+    ///
+    /// @param buf pointer to a buffer
+    /// @param buf_len length of the buffer
+    /// @param offset offset to place, where option shout be stored
+    ///
+    /// @return offset to first unused byte after stored option
     unsigned int
     pack(boost::shared_array<uint8_t> buf, unsigned int buf_len,
          unsigned int offset);
 
-    // parses received buffer, returns offset to the first unused byte after
-    // parsed option
+    /// @brief Parses buffer.
+    ///
+    /// Parses received buffer, returns offset to the first unused byte after
+    /// parsed option.
+    ///
+    /// @param buf pointer to buffer
+    /// @param buf_len length of buf
+    /// @param offset offset, where start parsing option
+    /// @param parse_len how many bytes should be parsed
+    ///
+    /// @return offset after last parsed octet
     virtual unsigned int
     unpack(boost::shared_array<uint8_t> buf,
            unsigned int buf_len,
            unsigned int offset,
            unsigned int parse_len);
 
-    virtual std::string toText(int indent = 0);
+    /// Returns string representation of the option.
+    ///
+    /// @param indent number of spaces before printing text
+    ///
+    /// @return string with text representation.
+    virtual std::string
+    toText(int indent = 0);
+
 
-    void setAddress(isc::asiolink::IOAddress addr) { addr_ = addr; }
+    /// sets address in this option.
+    ///
+    /// @param addr address to be sent in this option
+    void setAddress(const isc::asiolink::IOAddress& addr) { addr_ = addr; }
+
+    /// Sets preferred lifetime (in seconds)
+    ///
+    /// @param pref address preferred lifetime (in seconds)
+    ///
     void setPreferred(unsigned int pref) { preferred_=pref; }
+
+    /// Sets valid lifetime (in seconds).
+    ///
+    /// @param valid address valid lifetime (in seconds)
+    ///
     void setValid(unsigned int valid) { valid_=valid; }
 
-    isc::asiolink::IOAddress getAddress() { return addr_; }
-    unsigned int getPreferred()   { return preferred_; }
-    unsigned int getValid()   { return valid_; }
+    /// Returns  address contained within this option.
+    ///
+    /// @return address
+    isc::asiolink::IOAddress
+    getAddress() { return addr_; }
 
-    // returns data length (data length + DHCPv4/DHCPv6 option header)
-    virtual unsigned short len();
+    /// Returns preferred lifetime of an address.
+    ///
+    /// @return preferred lifetime (in seconds)
+    unsigned int
+    getPreferred()   { return preferred_; }
+
+    /// Returns valid lifetime of an address.
+    ///
+    /// @return valid lifetime (in seconds)
+    unsigned int
+    getValid()   { return valid_; }
+
+    /// returns data length (data length + DHCPv4/DHCPv6 option header)
+    virtual unsigned short
+    len();
 
 protected:
+    /// contains an IPv6 address
     isc::asiolink::IOAddress addr_;
+
+    /// contains preferred-lifetime timer (in seconds)
     unsigned int preferred_;
+
+    /// contains valid-lifetime timer (in seconds)
     unsigned int valid_;
 };
 

src/lib/dhcp/pkt6.cc

@@ -25,58 +25,43 @@ using namespace isc::dhcp;
 
 namespace isc {
 
-/**
- * Constructor.
- *
- * @param dataLen size of buffer to be allocated for this packet.
- * @param proto protocol (usually UDP, but TCP will be supported eventually)
- */
-Pkt6::Pkt6(unsigned int dataLen, DHCPv6Proto_ proto /* = UDP */)
-    :local_addr_("::"),
+Pkt6::Pkt6(unsigned int dataLen, DHCPv6Proto proto /* = UDP */)
+    :data_len_(dataLen),
+     local_addr_("::"),
      remote_addr_("::"),
-     proto_(proto)
+     iface_(""),
+     ifindex_(-1),
+     local_port_(-1),
+     remote_port_(-1),
+     proto_(proto),
+     msg_type_(-1),
+     transid_(rand()%0xffffff)
 {
-    try {
-        data_ = boost::shared_array<uint8_t>(new uint8_t[dataLen]);
-        data_len_ = dataLen;
-    } catch (const std::exception& ex) {
-        // TODO move to LOG_FATAL()
-        // let's continue with empty pkt for now
-        std::cout << "Failed to allocate " << dataLen << " bytes."
-                  << std::endl;
-        data_len_ = 0;
-    }
-}
 
+    data_ = boost::shared_array<uint8_t>(new uint8_t[dataLen]);
+    data_len_ = dataLen;
+}
 
 Pkt6::Pkt6(uint8_t msg_type,
            unsigned int transid,
-           DHCPv6Proto_ proto /*= UDP*/)
+           DHCPv6Proto proto /*= UDP*/)
     :local_addr_("::"),
      remote_addr_("::"),
+     iface_(""),
+     ifindex_(-1),
+     local_port_(-1),
+     remote_port_(-1),
      proto_(proto),
      msg_type_(msg_type),
      transid_(transid) {
 
-    try {
-        data_ = boost::shared_array<uint8_t>(new uint8_t[4]);
-        data_len_ = 4;
-    } catch (Exception e) {
-        cout << "Packet creation failed:" << e.what() << endl;
-    }
-    data_len_ = 0;
+    data_ = boost::shared_array<uint8_t>(new uint8_t[4]);
+    data_len_ = 4;
 }
 
-/**
- * Returns calculated length of the packet.
- *
- * This function returns size of required buffer to buld this packet.
- * To use that function, options_ field must be set.
- *
- * @return number of bytes required to build this packet
- */
-unsigned short Pkt6::len() {
-    unsigned int length = 4; // DHCPv6 header
+unsigned short
+Pkt6::len() {
+    unsigned int length = DHCPV6_PKT_HDR_LEN; // DHCPv6 header
 
     for (Option::Option6Collection::iterator it = options_.begin();
          it != options_.end();
@@ -88,13 +73,6 @@ unsigned short Pkt6::len() {
 }
 
 
-/**
- * Builds on wire packet.
- *
- * Prepares on wire packet format.
- *
- * @return true if preparation was successful
- */
 bool
 Pkt6::pack() {
     switch (proto_) {
@@ -108,12 +86,6 @@ Pkt6::pack() {
     return (false); // never happens
 }
 
-
-/**
- * Build on wire packet (in UDP format).
- *
- * @return true if packet build was successful, false otherwise
- */
 bool
 Pkt6::packUDP() {
     unsigned short length = len();
@@ -121,14 +93,10 @@ Pkt6::packUDP() {
         cout << "Previous len=" << data_len_ << ", allocating new buffer: len="
              << length << endl;
 
-        try {
-            data_ = boost::shared_array<uint8_t>(new uint8_t[length]);
-            data_len_ = length;
-        } catch (Exception e) {
-            cout << "Failed to allocate " << length << "-byte buffer:"
-                 << e.what() << endl;
-            return (false);
-        }
+        // May throw exception if out of memory. That is rather fatal,
+        // so we don't catch this
+        data_ = boost::shared_array<uint8_t>(new uint8_t[length]);
+        data_len_ = length;
     }
 
     try {
@@ -160,14 +128,6 @@ Pkt6::packUDP() {
     return (true);
 }
 
-
-/**
- * Builds on wire packet for TCP transmission.
- *
- * @note This function is not implemented yet.
- *
- * @return
- */
 bool
 Pkt6::packTCP() {
     /// TODO Implement this function.
@@ -175,13 +135,6 @@ Pkt6::packTCP() {
               "not implemented yet.");
 }
 
-/**
- * Dispatch method that handles binary packet parsing.
- *
- * This method calls appropriate dispatch function (unpackUDP or unpackTCP)
- *
- * @return true, if parsing was successful, false otherwise
- */
 bool
 Pkt6::unpack() {
     switch (proto_) {
@@ -195,11 +148,6 @@ Pkt6::unpack() {
     return (false); // never happens
 }
 
-/**
- * This method unpacks UDP packet.
- *
- * @return true, if parsing was successful, false otherwise
- */
 bool
 Pkt6::unpackUDP() {
     if (data_len_ < 4) {
@@ -226,11 +174,6 @@ Pkt6::unpackUDP() {
     return (true);
 }
 
-/**
- * This method unpacks TDP packet.
- *
- * @return true, if parsing was successful, false otherwise
- */
 bool
 Pkt6::unpackTCP() {
     isc_throw(Unexpected, "DHCPv6 over TCP (bulk leasequery and failover) "
@@ -238,13 +181,6 @@ Pkt6::unpackTCP() {
 }
 
 
-/**
- * Returns text representation of the packet.
- *
- * This function is useful mainly for debugging.
- *
- * @return string with text representation
- */
 std::string
 Pkt6::toText() {
     stringstream tmp;
@@ -261,17 +197,6 @@ Pkt6::toText() {
     return tmp.str();
 }
 
-/**
- * Returns the first option of specified type.
- *
- * Returns the first option of specified type. Note that in DHCPv6 several
- * instances of the same option are allowed (and frequently used).
- * See getOptions().
- *
- * @param opt_type option type we are looking for
- *
- * @return pointer to found option (or NULL)
- */
 boost::shared_ptr<isc::dhcp::Option>
 Pkt6::getOption(unsigned short opt_type) {
     isc::dhcp::Option::Option6Collection::const_iterator x = options_.find(opt_type);
@@ -281,16 +206,6 @@ Pkt6::getOption(unsigned short opt_type) {
     return boost::shared_ptr<isc::dhcp::Option>(); // NULL
 }
 
-/**
- * Returns message type.
- *
- * @return message type.
- */
-uint8_t
-Pkt6::getType() {
-    return (msg_type_);
-}
-
 void
 Pkt6::addOption(boost::shared_ptr<Option> opt) {
     options_.insert(pair<int, boost::shared_ptr<Option> >(opt->getType(), opt));
@@ -306,8 +221,4 @@ Pkt6::delOption(unsigned short type) {
     return (false); // can't find option to be deleted
 }
 
-Pkt6::~Pkt6() {
-    // no need to delete anything shared_ptr will take care of data_
-}
-
 };

src/lib/dhcp/pkt6.h

@@ -23,84 +23,212 @@
 
 namespace isc {
 
+namespace dhcp {
 
-    class Pkt6 {
-    public:
-        enum DHCPv6Proto_ {
-            UDP = 0, // most packets are UDP
-            TCP = 1  // there are TCP DHCPv6 packet (bulk leasequery, failover)
-        };
-
-        Pkt6(unsigned char msg_type,
-             unsigned int transid,
-             DHCPv6Proto_ proto = UDP);
-        Pkt6(unsigned int len, DHCPv6Proto_ proto = UDP);
-        ~Pkt6();
-
-        bool pack();
-        bool unpack();
-
-        DHCPv6Proto_ getProto();
-        void setProto(DHCPv6Proto_ proto = Pkt6::UDP);
-
-        // returns text representation, useful for debugging
-        std::string toText();
-
-        unsigned short len();
-
-        unsigned char getType();
-        void setType(unsigned char type) { msg_type_=type; };
-        unsigned int getTransid() { return transid_; };
-
-        /// TODO need getter/setter wrappers
-        ///      and hide following fields as protected
-        /// buffer that holds memory. It is shared_array as options may
-        /// share pointer to this buffer
-        boost::shared_array<uint8_t> data_;
-
-        // length of the data
-        unsigned int data_len_;
-
-        // local address (dst if receiving packet, src if sending packet)
-        isc::asiolink::IOAddress local_addr_;
-
-        // remote address (src if receiving packet, dst if sending packet)
-        isc::asiolink::IOAddress remote_addr_;
-
-        // name of the network interface the packet was received/to be sent over
-        std::string iface_;
-
-        // interface index (each network interface has assigned unique ifindex
-        // it is functional equvalent of name, but sometimes more useful, e.g.
-        // when using crazy systems that allow spaces in interface names
-        // e.g. windows
-        int ifindex_;
-
-        // local TDP or UDP port
-        int local_port_;
-
-        // remote TCP or UDP port
-        int remote_port_;
-
-        void addOption(boost::shared_ptr<isc::dhcp::Option> opt);
-        boost::shared_ptr<isc::dhcp::Option> getOption(unsigned short type);
-        bool delOption(unsigned short type);
-        /// TODO Need to implement getOptions() as well
-
-        // XXX: add *a lot* here
-        isc::dhcp::Option::Option6Collection options_;
-
-    protected:
-        bool packTCP();
-        bool packUDP();
-        bool unpackTCP();
-        bool unpackUDP();
-
-        DHCPv6Proto_ proto_; // UDP (most) or TCP (bulk leasequery or failover)
-        int msg_type_; // DHCPv6 message type
-        unsigned int transid_;  // DHCPv6 transaction-id
+class Pkt6 {
+public:
+    /// specifes DHCPv6 packet header length
+    const static size_t DHCPV6_PKT_HDR_LEN = 4;
 
+    /// DHCPv6 transport protocol
+    enum DHCPv6Proto {
+        UDP = 0, // most packets are UDP
+        TCP = 1  // there are TCP DHCPv6 packets (bulk leasequery, failover)
     };
-}
+
+    /// Constructor, used in replying to a message
+    ///
+    /// @param msg_type type of message (SOLICIT=1, ADVERTISE=2, ...)
+    /// @param transid transaction-id
+    /// @param proto protocol (TCP or UDP)
+    Pkt6(unsigned char msg_type,
+         unsigned int transid,
+         DHCPv6Proto proto = UDP);
+
+    /// Constructor, used in message transmission
+    ///
+    /// Creates new message. Transaction-id will randomized.
+    ///
+    /// @param len size of buffer to be allocated for this packet.
+    /// @param proto protocol (usually UDP, but TCP will be supported eventually)
+    Pkt6(unsigned int len, DHCPv6Proto proto = UDP);
+
+    /// @brief Prepares on-wire format.
+    ///
+    /// Prepares on-wire format of message and all its options.
+    /// Options must be stored in options_ field.
+    /// Output buffer will be stored in data_. Length
+    /// will be set in data_len_.
+    ///
+    /// @return true if packing procedure was successful
+    bool
+    pack();
+
+    /// @brief Dispatch method that handles binary packet parsing.
+    ///
+    /// This method calls appropriate dispatch function (unpackUDP or
+    /// unpackTCP).
+    ///
+    /// @return true if parsing was successful
+    bool
+    unpack();
+
+    /// Returns protocol of this packet (UDP or TCP)
+    ///
+    /// @return protocol type
+    DHCPv6Proto
+    getProto();
+
+    /// Sets protocol of this packet.
+    ///
+    /// @param proto protocol (UDP or TCP)
+    ///
+    void
+    setProto(DHCPv6Proto proto = UDP);
+
+    /// @brief Returns text representation of the packet.
+    ///
+    /// This function is useful mainly for debugging.
+    ///
+    /// @return string with text representation
+    std::string
+    toText();
+
+    /// @brief Returns calculated length of the packet.
+    ///
+    /// This function returns size of required buffer to buld this packet.
+    /// To use that function, options_ field must be set.
+    ///
+    /// @return number of bytes required to build this packet
+    unsigned short
+    len();
+
+    /// Returns message type (e.g. 1 = SOLICIT)
+    ///
+    /// @return message type
+    unsigned char
+    getType() { return (msg_type_); }
+
+    /// Sets message type (e.g. 1 = SOLICIT)
+    ///
+    /// @param type message type to be set
+    void setType(unsigned char type) { msg_type_=type; };
+
+    /// Returns value of transaction-id field
+    ///
+    /// @return transaction-id
+    unsigned int getTransid() { return (transid_); };
+
+    /// Adds an option to this packet.
+    ///
+    /// @param opt option to be added.
+    void addOption(boost::shared_ptr<isc::dhcp::Option> opt);
+
+    /// @brief Returns the first option of specified type.
+    ///
+    /// Returns the first option of specified type. Note that in DHCPv6 several
+    /// instances of the same option are allowed (and frequently used).
+    /// See getOptions().
+    ///
+    /// @param opt_type option type we are looking for
+    ///
+    /// @return pointer to found option (or NULL)
+    boost::shared_ptr<isc::dhcp::Option>
+    getOption(unsigned short type);
+
+    /// Attempts to delete first suboption of requested type
+    ///
+    /// @param type Type of option to be deleted.
+    ///
+    /// @return true if option was deleted, false if no such option existed
+    bool
+    delOption(unsigned short type);
+
+    /// TODO need getter/setter wrappers
+    ///      and hide following fields as protected
+
+    /// buffer that holds memory. It is shared_array as options may
+    /// share pointer to this buffer
+    boost::shared_array<uint8_t> data_;
+
+    /// length of the data
+    unsigned int data_len_;
+
+    /// local address (dst if receiving packet, src if sending packet)
+    isc::asiolink::IOAddress local_addr_;
+
+    /// remote address (src if receiving packet, dst if sending packet)
+    isc::asiolink::IOAddress remote_addr_;
+
+    /// name of the network interface the packet was received/to be sent over
+    std::string iface_;
+
+    /// @brief interface index
+    ///
+    /// interface index (each network interface has assigned unique ifindex
+    /// it is functional equvalent of name, but sometimes more useful, e.g.
+    /// when using crazy systems that allow spaces in interface names
+    /// e.g. windows
+    int ifindex_;
+
+    /// local TDP or UDP port
+    int local_port_;
+
+    /// remote TCP or UDP port
+    int remote_port_;
+
+    /// TODO Need to implement getOptions() as well
+
+    /// collection of options present in this message
+    isc::dhcp::Option::Option6Collection options_;
+
+protected:
+    /// Builds on wire packet for TCP transmission.
+    ///
+    /// TODO This function is not implemented yet.
+    ///
+    /// @return true, if build was successful
+    bool packTCP();
+
+    /// Builds on wire packet for UDP transmission.
+    ///
+    /// @return true, if build was successful
+    bool packUDP();
+
+    /// @brief Parses on-wire form of TCP DHCPv6 packet.
+    ///
+    /// Parses received packet, stored in on-wire format in data_.
+    /// data_len_ must be set to indicate data length.
+    /// Will create a collection of option objects that will
+    /// be stored in options_ container.
+    ///
+    /// TODO This function is not implemented yet.
+    ///
+    /// @return true, if build was successful
+    bool unpackTCP();
+
+    /// @brief Parses on-wire form of UDP DHCPv6 packet.
+    ///
+    /// Parses received packet, stored in on-wire format in data_.
+    /// data_len_ must be set to indicate data length.
+    /// Will create a collection of option objects that will
+    /// be stored in options_ container.
+    ///
+    /// @return true, if build was successful
+    bool unpackUDP();
+
+    /// UDP (usually) or TCP (bulk leasequery or failover)
+    DHCPv6Proto proto_;
+
+    /// DHCPv6 message type
+    int msg_type_;
+
+    /// DHCPv6 transaction-id
+    unsigned int transid_;
+}; // Pkt6 class
+
+} // isc::dhcp namespace
+
+} // isc namespace
 
 #endif

src/lib/dhcp/tests/option6_ia_unittest.cc

@@ -58,8 +58,7 @@ TEST_F(Option6IATest, basic) {
 
     // create an option
     // unpack() is called from constructor
-    Option6IA* opt = new Option6IA(Option::V6,
-                                   D6O_IA_NA,
+    Option6IA* opt = new Option6IA(D6O_IA_NA,
                                    simple_buf,
                                    128,
                                    0,
@@ -113,8 +112,7 @@ TEST_F(Option6IATest, simple) {
     for (int i=0; i<128; i++)
         simple_buf[i] = 0;
 
-    Option6IA * ia = new Option6IA(Option::V6, D6O_IA_NA,
-                                   1234);
+    Option6IA * ia = new Option6IA(D6O_IA_NA, 1234);
     ia->setT1(2345);
     ia->setT2(3456);
 
@@ -135,8 +133,7 @@ TEST_F(Option6IATest, suboptions_pack) {
     buf[1] = 0xfe;
     buf[2] = 0xfc;
 
-    Option6IA * ia = new Option6IA(Option::V6, D6O_IA_NA,
-                                   0x13579ace);
+    Option6IA * ia = new Option6IA(D6O_IA_NA, 0x13579ace);
     ia->setT1(0x2345);
     ia->setT2(0x3456);
 
@@ -213,8 +210,7 @@ TEST_F(Option6IATest, suboptions_unpack) {
 
     Option6IA* ia = 0;
     EXPECT_NO_THROW({
-        ia = new Option6IA(Option::V6, D6O_IA_NA,
-                           buf, 128, 4, 44);
+        ia = new Option6IA(D6O_IA_NA, buf, 128, 4, 44);
         cout << "Parsed option:" << endl << ia->toText() << endl;
     });
     ASSERT_TRUE(ia);