BIND 10 trac1186, updated. 1213572a61fdf5a81e05fa42b4d7e4ed20c99060 [1186] Part 6 of review changes.

BIND 10 source code commits bind10-changes at lists.isc.org
Wed Oct 12 12:43:51 UTC 2011


The branch, trac1186 has been updated
       via  1213572a61fdf5a81e05fa42b4d7e4ed20c99060 (commit)
      from  2894b07900ec81ed39284c7d7df5660eb8afbfc0 (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

- Log -----------------------------------------------------------------
commit 1213572a61fdf5a81e05fa42b4d7e4ed20c99060
Author: Tomek Mrugalski <tomasz at isc.org>
Date:   Wed Oct 12 14:43:37 2011 +0200

    [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.

-----------------------------------------------------------------------

Summary of changes:
 src/lib/asiolink/io_address.h             |    6 +
 src/lib/dhcp/libdhcp.cc                   |    3 +-
 src/lib/dhcp/option.h                     |    3 +-
 src/lib/dhcp/option6_addrlst.h            |    3 -
 src/lib/dhcp/option6_ia.cc                |   38 +++--
 src/lib/dhcp/option6_ia.h                 |   97 ++++++++---
 src/lib/dhcp/option6_iaaddr.cc            |   32 ++--
 src/lib/dhcp/option6_iaaddr.h             |  118 ++++++++++---
 src/lib/dhcp/pkt6.cc                      |  143 +++------------
 src/lib/dhcp/pkt6.h                       |  280 +++++++++++++++++++++--------
 src/lib/dhcp/tests/option6_ia_unittest.cc |   12 +-
 11 files changed, 444 insertions(+), 291 deletions(-)

-----------------------------------------------------------------------
diff --git a/src/lib/asiolink/io_address.h b/src/lib/asiolink/io_address.h
index 763b02d..e9da9f9 100644
--- a/src/lib/asiolink/io_address.h
+++ b/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)
 ///
diff --git a/src/lib/dhcp/libdhcp.cc b/src/lib/dhcp/libdhcp.cc
index be4c163..15cccdd 100644
--- a/src/lib/dhcp/libdhcp.cc
+++ b/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));
diff --git a/src/lib/dhcp/option.h b/src/lib/dhcp/option.h
index de5a401..db5de84 100644
--- a/src/lib/dhcp/option.h
+++ b/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);
 
diff --git a/src/lib/dhcp/option6_addrlst.h b/src/lib/dhcp/option6_addrlst.h
index 0ed3f46..c3abf1b 100644
--- a/src/lib/dhcp/option6_addrlst.h
+++ b/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
diff --git a/src/lib/dhcp/option6_ia.cc b/src/lib/dhcp/option6_ia.cc
index 393fe1e..345bbe3 100644
--- a/src/lib/dhcp/option6_ia.cc
+++ b/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();
diff --git a/src/lib/dhcp/option6_ia.h b/src/lib/dhcp/option6_ia.h
index 013fd78..9511076 100644
--- a/src/lib/dhcp/option6_ia.h
+++ b/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_;
 };
 
diff --git a/src/lib/dhcp/option6_iaaddr.cc b/src/lib/dhcp/option6_iaaddr.cc
index 78b9637..82f4400 100644
--- a/src/lib/dhcp/option6_iaaddr.cc
+++ b/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:
diff --git a/src/lib/dhcp/option6_iaaddr.h b/src/lib/dhcp/option6_iaaddr.h
index 1663cf5..38e3386 100644
--- a/src/lib/dhcp/option6_iaaddr.h
+++ b/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_;
 };
 
diff --git a/src/lib/dhcp/pkt6.cc b/src/lib/dhcp/pkt6.cc
index f9a2710..39988dd 100644
--- a/src/lib/dhcp/pkt6.cc
+++ b/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_
-}
-
 };
diff --git a/src/lib/dhcp/pkt6.h b/src/lib/dhcp/pkt6.h
index 93782d6..af0761e 100644
--- a/src/lib/dhcp/pkt6.h
+++ b/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
diff --git a/src/lib/dhcp/tests/option6_ia_unittest.cc b/src/lib/dhcp/tests/option6_ia_unittest.cc
index 5de8c23..3b6a4db 100644
--- a/src/lib/dhcp/tests/option6_ia_unittest.cc
+++ b/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);




More information about the bind10-changes mailing list