[#2181] revert type change for pkt options_ member

12 files changed, 40 insertions, 127 deletions

diff --git a/src/bin/dhcp6/dhcp6_srv.cc b/src/bin/dhcp6/dhcp6_srv.cc
index 51becc86fd..d497005107 100644
--- a/src/bin/dhcp6/dhcp6_srv.cc
+++ b/src/bin/dhcp6/dhcp6_srv.cc
@@ -1764,7 +1764,7 @@ Dhcpv6Srv::assignLeases(const Pkt6Ptr& question, Pkt6Ptr& answer,
     // responses in answer message (ADVERTISE or REPLY).
     //
     // @todo: IA_TA once we implement support for temporary addresses.
-    for (const auto& opt : (*question->options_)) {
+    for (const auto& opt : question->options_) {
         switch (opt.second->getType()) {
         case D6O_IA_NA: {
             OptionPtr answer_opt = assignIA_NA(question, ctx,
@@ -2656,7 +2656,7 @@ Dhcpv6Srv::extendLeases(const Pkt6Ptr& query, Pkt6Ptr& reply,
     // Save the originally selected subnet.
     Subnet6Ptr orig_subnet = ctx.subnet_;
 
-    for (const auto& opt : (*query->options_)) {
+    for (const auto& opt : query->options_) {
         switch (opt.second->getType()) {
         case D6O_IA_NA: {
             OptionPtr answer_opt = extendIA_NA(query, ctx,
@@ -2708,7 +2708,7 @@ Dhcpv6Srv::releaseLeases(const Pkt6Ptr& release, Pkt6Ptr& reply,
     // handled properly. Therefore the releaseIA_NA and releaseIA_PD options
     // may turn the status code to some error, but can't turn it back to success.
     int general_status = STATUS_Success;
-    for (const auto& opt : (*release->options_)) {
+    for (const auto& opt : release->options_) {
         Lease6Ptr old_lease;
         switch (opt.second->getType()) {
         case D6O_IA_NA: {
@@ -3406,7 +3406,7 @@ Dhcpv6Srv::declineLeases(const Pkt6Ptr& decline, Pkt6Ptr& reply,
     // may turn the status code to some error, but can't turn it back to success.
     int general_status = STATUS_Success;
 
-    for (const auto& opt : (*decline->options_)) {
+    for (const auto& opt : decline->options_) {
         switch (opt.second->getType()) {
         case D6O_IA_NA: {
             OptionPtr answer_opt = declineIA(decline, ctx.duid_, general_status,
diff --git a/src/bin/dhcp6/tests/dhcp6_client.cc b/src/bin/dhcp6/tests/dhcp6_client.cc
index bbb6cd5ed9..ec1714efa7 100644
--- a/src/bin/dhcp6/tests/dhcp6_client.cc
+++ b/src/bin/dhcp6/tests/dhcp6_client.cc
@@ -144,7 +144,7 @@ Dhcp6Client::applyRcvdConfiguration(const Pkt6Ptr& reply, uint32_t state) {
 
     // Get all options in the reply message and pick IA_NA, IA_PD and
     // Status code.
-    for (const auto& opt : *reply->options_) {
+    for (const auto& opt : reply->options_) {
         Option6IAPtr ia = boost::dynamic_pointer_cast<Option6IA>(opt.second);
         if (!ia) {
             // This is not IA, so let's just store it.
@@ -873,7 +873,7 @@ Dhcp6Client::getTeeTimes(const uint32_t iaid, uint32_t& t1, uint32_t& t2) const
     }
 
     // Get all options in the response message and pick IA_NA, IA_PD.
-    for (const auto& opt : *context_.response_->options_) {
+    for (const auto& opt : context_.response_->options_) {
         Option6IAPtr ia = boost::dynamic_pointer_cast<Option6IA>(opt.second);
         if (!ia) {
             // This is not IA, so let's just skip it.
diff --git a/src/bin/perfdhcp/perf_pkt4.cc b/src/bin/perfdhcp/perf_pkt4.cc
index 5c5e46bc15..0bceb82ba9 100644
--- a/src/bin/perfdhcp/perf_pkt4.cc
+++ b/src/bin/perfdhcp/perf_pkt4.cc
@@ -31,7 +31,7 @@ bool
 PerfPkt4::rawPack() {
     return (PktTransform::pack(dhcp::Option::V4,
                                data_,
-                               *options_,
+                               options_,
                                getTransidOffset(),
                                getTransid(),
                                buffer_out_));
@@ -42,7 +42,7 @@ PerfPkt4::rawUnpack() {
     uint32_t transid = getTransid();
     bool res = PktTransform::unpack(dhcp::Option::V4,
                                     data_,
-                                    *options_,
+                                    options_,
                                     getTransidOffset(),
                                     transid);
     if (res) {
diff --git a/src/bin/perfdhcp/perf_pkt6.cc b/src/bin/perfdhcp/perf_pkt6.cc
index 948fbaf244..066d8ba516 100644
--- a/src/bin/perfdhcp/perf_pkt6.cc
+++ b/src/bin/perfdhcp/perf_pkt6.cc
@@ -35,7 +35,7 @@ bool
 PerfPkt6::rawPack() {
     return (PktTransform::pack(dhcp::Option::V6,
                                data_,
-                               *options_,
+                               options_,
                                getTransidOffset(),
                                getTransid(),
                                buffer_out_));
@@ -46,7 +46,7 @@ PerfPkt6::rawUnpack() {
     uint32_t transid = getTransid();
     bool res =  PktTransform::unpack(dhcp::Option::V6,
                                      data_,
-                                     *options_,
+                                     options_,
                                      getTransidOffset(),
                                      transid);
     if (res) {
diff --git a/src/bin/perfdhcp/test_control.cc b/src/bin/perfdhcp/test_control.cc
index 51eada4d05..72f1e51711 100644
--- a/src/bin/perfdhcp/test_control.cc
+++ b/src/bin/perfdhcp/test_control.cc
@@ -861,7 +861,7 @@ TestControl::address6Uniqueness(const Pkt6Ptr& pkt6, ExchangeType xchg_type) {
         std::set<std::string> current;
         // addresses were already checked in validateIA
         // we can safely assume that those are correct
-        for (const auto& opt : *pkt6->options_) {
+        for (const auto& opt : pkt6->options_) {
             switch (opt.second->getType()) {
             case D6O_IA_PD: {
                 // add address and check if it has not been already assigned
diff --git a/src/lib/dhcp/pkt.cc b/src/lib/dhcp/pkt.cc
index 0476adc057..24f01704a6 100644
--- a/src/lib/dhcp/pkt.cc
+++ b/src/lib/dhcp/pkt.cc
@@ -17,16 +17,16 @@ namespace dhcp {
 Pkt::Pkt(uint32_t transid, const isc::asiolink::IOAddress& local_addr,
          const isc::asiolink::IOAddress& remote_addr, uint16_t local_port,
          uint16_t remote_port)
-    : options_(new OptionCollection()), transid_(transid), iface_(""), ifindex_(-1),
-      local_addr_(local_addr), remote_addr_(remote_addr), local_port_(local_port),
+    : transid_(transid), iface_(""), ifindex_(-1), local_addr_(local_addr),
+      remote_addr_(remote_addr), local_port_(local_port),
       remote_port_(remote_port), buffer_out_(0), copy_retrieved_options_(false) {
 }
 
 Pkt::Pkt(const uint8_t* buf, uint32_t len, const isc::asiolink::IOAddress& local_addr,
          const isc::asiolink::IOAddress& remote_addr, uint16_t local_port,
          uint16_t remote_port)
-    : options_(new OptionCollection()), transid_(0), iface_(""), ifindex_(-1),
-      local_addr_(local_addr), remote_addr_(remote_addr), local_port_(local_port),
+    : transid_(0), iface_(""), ifindex_(-1), local_addr_(local_addr),
+      remote_addr_(remote_addr), local_port_(local_port),
       remote_port_(remote_port), buffer_out_(0), copy_retrieved_options_(false) {
     if (len != 0) {
         if (buf == NULL) {
@@ -39,13 +39,13 @@ Pkt::Pkt(const uint8_t* buf, uint32_t len, const isc::asiolink::IOAddress& local
 
 void
 Pkt::addOption(const OptionPtr& opt) {
-    options_->insert(std::pair<int, OptionPtr>(opt->getType(), opt));
+    options_.insert(std::pair<int, OptionPtr>(opt->getType(), opt));
 }
 
 OptionPtr
 Pkt::getNonCopiedOption(const uint16_t type) const {
-    const auto& x = options_->find(type);
-    if (x != options_->end()) {
+    const auto& x = options_.find(type);
+    if (x != options_.end()) {
         return (x->second);
     }
     return (OptionPtr());
@@ -53,8 +53,8 @@ Pkt::getNonCopiedOption(const uint16_t type) const {
 
 OptionPtr
 Pkt::getOption(const uint16_t type) {
-    const auto& x = options_->find(type);
-    if (x != options_->end()) {
+    const auto& x = options_.find(type);
+    if (x != options_.end()) {
         if (copy_retrieved_options_) {
             OptionPtr option_copy = x->second->clone();
             x->second = option_copy;
@@ -66,9 +66,9 @@ Pkt::getOption(const uint16_t type) {
 
 bool
 Pkt::delOption(uint16_t type) {
-    const auto& x = options_->find(type);
-    if (x != options_->end()) {
-        options_->erase(x);
+    const auto& x = options_.find(type);
+    if (x != options_.end()) {
+        options_.erase(x);
         return (true); // delete successful
     } else {
         return (false); // can't find option to be deleted
diff --git a/src/lib/dhcp/pkt.h b/src/lib/dhcp/pkt.h
index 8536d3c96e..2a511735d8 100644
--- a/src/lib/dhcp/pkt.h
+++ b/src/lib/dhcp/pkt.h
@@ -78,56 +78,6 @@ private:
     std::pair<PktTypePtr, PktTypePtr> pkts_;
 };
 
-/// @brief RAII object enabling duplication of the stored options and restoring
-/// the original options on destructor.
-///
-/// This object enables duplication of the stored options and restoring the
-/// original options on destructor. When the object goes out of scope, the
-/// initial options are restored. This is applicable in cases when the server is
-/// going to invoke a callout (hook library) where the list of options in the
-/// packet will be modified. The use of RAII object eliminates the need for
-/// explicitly copying and restoring the list of options and is safer in case of
-/// exceptions thrown by callouts and a presence of multiple exit points.
-///
-/// @tparam PktType Type of the packet, e.g. Pkt4, Pkt6, Pkt4o6.
-template<typename PktType>
-class ScopedOptionsCopy {
-public:
-
-    /// @brief Pointer to an encapsulated packet.
-    typedef boost::shared_ptr<PktType> PktTypePtr;
-
-    /// @brief Constructor.
-    ///
-    /// Creates a copy of the initial options on a packet(s).
-    ///
-    /// @param pkt Pointer to the packet.
-    ScopedOptionsCopy(const PktTypePtr& pkt)
-        : pkt_(pkt) {
-        if (pkt) {
-            options_ = pkt->options_;
-            pkt->options_ = OptionCollectionPtr(new OptionCollection(*options_));
-        }
-    }
-
-    /// @brief Destructor.
-    ///
-    /// Restores the initial options on a packet.
-    ~ScopedOptionsCopy() {
-        if (pkt_) {
-            pkt_->options_ = options_;
-        }
-    }
-
-private:
-
-    /// @brief Holds a pointer to the packet.
-    PktTypePtr pkt_;
-
-    /// @brief Holds a pointer to the initial packet options.
-    OptionCollectionPtr options_;
-};
-
 /// @brief Base class for classes representing DHCP messages.
 ///
 /// This is a base class that holds common information (e.g. source
@@ -661,7 +611,7 @@ public:
     /// behavior must be taken into consideration before making
     /// changes to this member such as access scope restriction or
     /// data format change etc.
-    isc::dhcp::OptionCollectionPtr options_;
+    isc::dhcp::OptionCollection options_;
 
 protected:
 
@@ -846,7 +796,7 @@ private:
 /// @brief A pointer to either Pkt4 or Pkt6 packet
 typedef boost::shared_ptr<isc::dhcp::Pkt> PktPtr;
 
-} // namespace isc::dhcp
-} // namespace isc
+} // end of namespace isc::dhcp
+} // end of namespace isc
 
 #endif
diff --git a/src/lib/dhcp/pkt4.cc b/src/lib/dhcp/pkt4.cc
index 2f3f255c58..2a26e19c20 100644
--- a/src/lib/dhcp/pkt4.cc
+++ b/src/lib/dhcp/pkt4.cc
@@ -60,7 +60,7 @@ Pkt4::len() {
     size_t length = DHCPV4_PKT_HDR_LEN; // DHCPv4 header
 
     // ... and sum of lengths of all options
-    for (const auto& it : *options_) {
+    for (const auto& it : options_) {
         length += it.second->len();
     }
 
@@ -119,7 +119,7 @@ Pkt4::pack() {
 
         // Call packOptions4() with parameter,"top", true. This invokes
         // logic to emit the message type option first.
-        LibDHCP::packOptions4(buffer_out_, *options_, true);
+        LibDHCP::packOptions4(buffer_out_, options_, true);
 
         // add END option that indicates end of options
         // (End option is very simple, just a 255 octet)
@@ -187,7 +187,7 @@ Pkt4::unpack() {
     // a vector as an input.
     buffer_in.readVector(opts_buffer, opts_len);
 
-    size_t offset = LibDHCP::unpackOptions4(opts_buffer, DHCP4_OPTION_SPACE, *options_, deferred_options_, false);
+    size_t offset = LibDHCP::unpackOptions4(opts_buffer, DHCP4_OPTION_SPACE, options_, deferred_options_, false);
 
     // If offset is not equal to the size and there is no DHO_END,
     // then something is wrong here. We either parsed past input
@@ -413,9 +413,9 @@ Pkt4::toText() const {
 
     output << ", transid=0x" << hex << transid_ << dec;
 
-    if (!options_->empty()) {
+    if (!options_.empty()) {
         output << "," << std::endl << "options:";
-        for (const auto& opt : *options_) {
+        for (const auto& opt : options_) {
             try {
                 output << std::endl << opt.second->toText(2);
             } catch (...) {
diff --git a/src/lib/dhcp/pkt6.cc b/src/lib/dhcp/pkt6.cc
index f46032e4be..55d3b5f6bd 100644
--- a/src/lib/dhcp/pkt6.cc
+++ b/src/lib/dhcp/pkt6.cc
@@ -253,7 +253,7 @@ uint16_t Pkt6::calculateRelaySizes() {
 uint16_t Pkt6::directLen() const {
     uint16_t length = DHCPV6_PKT_HDR_LEN; // DHCPv6 header
 
-    for (const auto& it : *options_) {
+    for (const auto& it : options_) {
         length += it.second->len();
     }
 
@@ -330,7 +330,7 @@ Pkt6::packUDP() {
         buffer_out_.writeUint8( (transid_) & 0xff );
 
         // the rest are options
-        LibDHCP::packOptions6(buffer_out_, *options_);
+        LibDHCP::packOptions6(buffer_out_, options_);
     }
     catch (const Exception& e) {
        // An exception is thrown and message will be written to Logger
@@ -412,7 +412,7 @@ Pkt6::unpackMsg(OptionBuffer::const_iterator begin,
 
     // If custom option parsing function has been set, use this function
     // to parse options. Otherwise, use standard function from libdhcp.
-    size_t offset = LibDHCP::unpackOptions6(opt_buffer, DHCP6_OPTION_SPACE, *options_);
+    size_t offset = LibDHCP::unpackOptions6(opt_buffer, DHCP6_OPTION_SPACE, options_);
 
     // If offset is not equal to the size, then something is wrong here. We
     // either parsed past input buffer (bug in our code) or we haven't parsed
@@ -627,7 +627,7 @@ Pkt6::toText() const {
         hex << transid_ << dec << endl;
 
     // Then print the options
-    for (const auto& opt : *options_) {
+    for (const auto& opt : options_) {
         tmp << opt.second->toText() << std::endl;
     }
 
@@ -664,7 +664,7 @@ Pkt6::getClientId() const {
 isc::dhcp::OptionCollection
 Pkt6::getNonCopiedOptions(const uint16_t opt_type) const {
     std::pair<OptionCollection::const_iterator,
-              OptionCollection::const_iterator> range = options_->equal_range(opt_type);
+              OptionCollection::const_iterator> range = options_.equal_range(opt_type);
     return (OptionCollection(range.first, range.second));
 }
 
@@ -673,7 +673,7 @@ Pkt6::getOptions(const uint16_t opt_type) {
     OptionCollection options_copy;
 
     std::pair<OptionCollection::iterator,
-              OptionCollection::iterator> range = options_->equal_range(opt_type);
+              OptionCollection::iterator> range = options_.equal_range(opt_type);
     // If options should be copied on retrieval, we should now iterate over
     // matching options, copy them and replace the original ones with new
     // instances.
@@ -932,5 +932,5 @@ Pkt6::getMACFromRemoteIdRelayOption() {
     return (mac);
 }
 
-} // end of isc::dhcp namespace
-} // end of isc namespace
+} // end of namespace isc::dhcp
+} // end of namespace isc
diff --git a/src/lib/dhcp/pkt6.h b/src/lib/dhcp/pkt6.h
index 8683ae4f22..263f8eb833 100644
--- a/src/lib/dhcp/pkt6.h
+++ b/src/lib/dhcp/pkt6.h
@@ -585,7 +585,6 @@ protected:
 }; // Pkt6 class
 
 } // isc::dhcp namespace
-
 } // isc namespace
 
 #endif
diff --git a/src/lib/dhcp/tests/protocol_util_unittest.cc b/src/lib/dhcp/tests/protocol_util_unittest.cc
index 94d3f7d418..355114242d 100644
--- a/src/lib/dhcp/tests/protocol_util_unittest.cc
+++ b/src/lib/dhcp/tests/protocol_util_unittest.cc
@@ -413,40 +413,4 @@ TEST(ScopedEnableOptionsCopy, enableOptionsCopy) {
     }
 }
 
-TEST(ScopedOptionsCopy, optionsCopy) {
-    Pkt4Ptr pkt(new Pkt4(DHCPDISCOVER, 2543));
-    OptionPtr option = Option::create(Option::V4, DHO_BOOT_FILE_NAME);
-    OptionCollectionPtr options = pkt->options_;
-    pkt->addOption(option);
-    size_t count = pkt->options_->size();
-    ASSERT_NE(0, count);
-    ASSERT_EQ(option, pkt->getOption(DHO_BOOT_FILE_NAME));
-    {
-        ScopedOptionsCopy<Pkt4> oc(pkt);
-        ASSERT_NE(pkt->options_, options);
-        ASSERT_EQ(*pkt->options_, *options);
-        pkt->delOption(DHO_BOOT_FILE_NAME);
-        ASSERT_EQ(pkt->options_->size(), count - 1);
-        ASSERT_FALSE(pkt->getOption(DHO_BOOT_FILE_NAME));
-    }
-    ASSERT_EQ(pkt->options_, options);
-    ASSERT_EQ(pkt->getOption(DHO_BOOT_FILE_NAME), option);
-    {
-        try {
-            ScopedOptionsCopy<Pkt4> oc(pkt);
-            ASSERT_NE(pkt->options_, options);
-            ASSERT_EQ(*pkt->options_, *options);
-            pkt->delOption(DHO_BOOT_FILE_NAME);
-            ASSERT_EQ(pkt->options_->size(), count - 1);
-            ASSERT_FALSE(pkt->getOption(DHO_BOOT_FILE_NAME));
-            throw 0;
-        } catch (...) {
-            ASSERT_EQ(pkt->options_, options);
-            ASSERT_EQ(pkt->getOption(DHO_BOOT_FILE_NAME), option);
-        }
-        ASSERT_EQ(pkt->options_, options);
-        ASSERT_EQ(pkt->getOption(DHO_BOOT_FILE_NAME), option);
-    }
-}
-
 } // anonymous namespace
diff --git a/src/lib/eval/token.h b/src/lib/eval/token.h
index 3e4e3be474..1a5d12f62a 100644
--- a/src/lib/eval/token.h
+++ b/src/lib/eval/token.h
@@ -1194,7 +1194,7 @@ protected:
 /// option[149].exists - check if option 149 exists
 /// option[149].option[1].exists - check if suboption 1 exists in the option 149
 /// option[149].option[1].hex - return content of suboption 1 for option 149
-class TokenSubOption : public TokenOption {
+class TokenSubOption : public virtual TokenOption {
 public:
 
     /// @note Does not define its own representation type: