From 7866e77be457ec569f72f38906c61cc316b5c8c4 Mon Sep 17 00:00:00 2001 From: Francis Dupont Date: Mon, 10 Jun 2019 14:43:24 +0200 Subject: [PATCH] [527-check-return-value-of-multi-index-push_back] Added either explicit cast or check return on push_back to multi index --- .../dhcp6/tests/shared_network_unittest.cc | 6 ++--- src/bin/perfdhcp/stats_mgr.h | 13 ++++++---- src/hooks/dhcp/mysql_cb/mysql_cb_dhcp4.cc | 19 +++++++++++++-- src/hooks/dhcp/mysql_cb/mysql_cb_dhcp6.cc | 19 +++++++++++++-- src/hooks/dhcp/mysql_cb/mysql_cb_impl.cc | 9 +++++-- src/lib/dhcp/libdhcp++.cc | 8 ++++++- src/lib/dhcp/option_space_container.h | 5 +++- src/lib/dhcpsrv/cfg_shared_networks.h | 4 ++-- src/lib/dhcpsrv/cfg_subnets4.cc | 4 ++-- src/lib/dhcpsrv/cfg_subnets6.cc | 4 ++-- src/lib/dhcpsrv/parsers/dhcp_parsers.cc | 24 +++++++++++++------ src/lib/dhcpsrv/shared_network.cc | 2 +- 12 files changed, 87 insertions(+), 30 deletions(-) diff --git a/src/bin/dhcp6/tests/shared_network_unittest.cc b/src/bin/dhcp6/tests/shared_network_unittest.cc index 7ffe9fc24d..c31ec68b8a 100644 --- a/src/bin/dhcp6/tests/shared_network_unittest.cc +++ b/src/bin/dhcp6/tests/shared_network_unittest.cc @@ -810,11 +810,11 @@ const char* NETWORKS_CONFIG[] = { " }," " {" " \"subnet\": \"2001:db8:2::/64\"," - " \"id\": 10," + " \"id\": 11," " \"interface-id\": \"vlan10\"," " \"pools\": [" " {" - " \"pool\": \"2001:db8:2::20 - 2001:db8:2::20\"" + " \"pool\": \"2001:db8:2::10 - 2001:db8:2::10\"" " }" " ]" " }" @@ -823,7 +823,7 @@ const char* NETWORKS_CONFIG[] = { " ]," " \"subnet6\": [" " {" - " \"subnet\": \"2001:db8:2::/64\"," + " \"subnet\": \"2001:db8:2::1/64\"," " \"id\": 1000," " \"interface-id\": \"vlan1000\"," " \"pools\": [" diff --git a/src/bin/perfdhcp/stats_mgr.h b/src/bin/perfdhcp/stats_mgr.h index deff500a04..45464f9f00 100644 --- a/src/bin/perfdhcp/stats_mgr.h +++ b/src/bin/perfdhcp/stats_mgr.h @@ -181,12 +181,15 @@ public: /// boost::shared_ptr pkt2(new Pkt4(...)); /// // Add new packet to the container, it will be available through /// // both indexes - /// packets_collection.push_back(pkt1); + /// static_cast(packets_collection.push_back(pkt1)); /// // Here is another way to add packet to the container. The result /// // is exactly the same as previously. - /// packets_collection.template get<0>().push_back(pkt2); + /// static_cast(packets_collection.template get<0>().push_back(pkt2)); /// \endcode /// + /// @note The multi index has no unique index so insertion should never + /// fail and there is no need to check the return of push_back(). + /// /// Example 2: Access elements through sequential index /// \code /// PktList packets_collection(); @@ -277,8 +280,8 @@ public: if (!packet) { isc_throw(BadValue, "Packet is null"); } + static_cast(sent_packets_.template get<0>().push_back(packet)); ++sent_packets_num_; - sent_packets_.template get<0>().push_back(packet); } /// \brief Add new packet to list of received packets. @@ -291,7 +294,7 @@ public: if (!packet) { isc_throw(BadValue, "Packet is null"); } - rcvd_packets_.push_back(packet); + static_cast(rcvd_packets_.push_back(packet)); } /// \brief Update delay counters. @@ -545,7 +548,7 @@ private: // move it to list of archived packets. List of // archived packets may be used for diagnostics // when test is completed. - archived_packets_.push_back(*it); + static_cast(archived_packets_.push_back(*it)); } // get<0>() template returns sequential index to // container. diff --git a/src/hooks/dhcp/mysql_cb/mysql_cb_dhcp4.cc b/src/hooks/dhcp/mysql_cb/mysql_cb_dhcp4.cc index 166bdd92de..29b48c1715 100644 --- a/src/hooks/dhcp/mysql_cb/mysql_cb_dhcp4.cc +++ b/src/hooks/dhcp/mysql_cb/mysql_cb_dhcp4.cc @@ -457,7 +457,14 @@ public: last_subnet->setServerTag(out_bindings[53]->getString()); // Subnet ready. Add it to the list. - subnets.push_back(last_subnet); + auto ret = subnets.push_back(last_subnet); + + // subnets is a multi index container with unique indexes + // but these indexes are unique too in the database, + // so this is for sanity only. + if (!ret.second) { + isc_throw(Unexpected, "add subnet failed"); + } } // If the row contains information about the pool and it appears to be @@ -1165,7 +1172,15 @@ public: // server_tag last_network->setServerTag(out_bindings[32]->getString()); - shared_networks.push_back(last_network); + // Add the shared network. + auto ret = shared_networks.push_back(last_network); + + // shared_networks is a multi index container with an unique + // index but this index is unique too in the database, + // so this is for sanity only. + if (!ret.second) { + isc_throw(Unexpected, "add shared network failed"); + } } // Parse option. diff --git a/src/hooks/dhcp/mysql_cb/mysql_cb_dhcp6.cc b/src/hooks/dhcp/mysql_cb/mysql_cb_dhcp6.cc index b0af981c84..3dd2abffc0 100644 --- a/src/hooks/dhcp/mysql_cb/mysql_cb_dhcp6.cc +++ b/src/hooks/dhcp/mysql_cb/mysql_cb_dhcp6.cc @@ -473,7 +473,14 @@ public: last_subnet->setServerTag(out_bindings[69]->getString()); // Subnet ready. Add it to the list. - subnets.push_back(last_subnet); + auto ret = subnets.push_back(last_subnet); + + // subnets is a multi index container with unique indexes + // but these indexes are unique too in the database, + // so this is for sanity only. + if (!ret.second) { + isc_throw(Unexpected, "add subnet failed"); + } } // Pool is between 15 and 19 @@ -1351,7 +1358,15 @@ public: // server_tag last_network->setServerTag(out_bindings[31]->getString()); - shared_networks.push_back(last_network); + // Add the shared network. + auto ret = shared_networks.push_back(last_network); + + // shared_networks is a multi index container with an unique + // index but this index is unique too in the database, + // so this is for sanity only. + if (!ret.second) { + isc_throw(Unexpected, "add shared network failed"); + } } // Parse option from 14 to 26. diff --git a/src/hooks/dhcp/mysql_cb/mysql_cb_impl.cc b/src/hooks/dhcp/mysql_cb/mysql_cb_impl.cc index aa0b94a68d..251e2e1c4c 100644 --- a/src/hooks/dhcp/mysql_cb/mysql_cb_impl.cc +++ b/src/hooks/dhcp/mysql_cb/mysql_cb_impl.cc @@ -367,7 +367,12 @@ MySqlConfigBackendImpl::getOptionDefs(const int index, last_def->setServerTag(out_bindings[10]->getString()); // Store created option definition. - option_defs.push_back(last_def); + auto ret = option_defs.push_back(last_def); + if (!ret.second) { + // option_defs is a multi-index container so push_back() + // can in theory fails. Now it has no unique indexes... + isc_throw(Unexpected, "can't store the option definition"); + } } }); } @@ -684,7 +689,7 @@ MySqlConfigBackendImpl::getOptions(const int index, if (desc) { // server_tag for the global option desc->setServerTag(out_bindings[12]->getString()); - options.push_back(*desc); + static_cast(options.push_back(*desc)); } } }); diff --git a/src/lib/dhcp/libdhcp++.cc b/src/lib/dhcp/libdhcp++.cc index 7eabbaa76c..2cd286ae77 100644 --- a/src/lib/dhcp/libdhcp++.cc +++ b/src/lib/dhcp/libdhcp++.cc @@ -1023,6 +1023,12 @@ void initOptionSpace(OptionDefContainerPtr& defs, defs->clear(); throw; } - defs->push_back(definition); + + auto ret = defs->push_back(definition); + if (!ret.second) { + // defs points to a multi index container with only not unique + // indexes so this is for sanity only. + isc_throw(isc::Unexpected, "can't store option definition"); + } } } diff --git a/src/lib/dhcp/option_space_container.h b/src/lib/dhcp/option_space_container.h index 5700ccdc84..d1c2952cb6 100644 --- a/src/lib/dhcp/option_space_container.h +++ b/src/lib/dhcp/option_space_container.h @@ -48,7 +48,10 @@ public: /// @param option_space name or vendor-id of the option space void addItem(const ItemType& item, const Selector& option_space) { ItemsContainerPtr items = getItems(option_space); - items->push_back(item); + // Assume that the push_back() can't fail even when the + // ContainerType is a multi index container, i.e., assume + // there is no unique index which can raise a conflict. + static_cast(items->push_back(item)); option_space_map_[option_space] = items; } diff --git a/src/lib/dhcpsrv/cfg_shared_networks.h b/src/lib/dhcpsrv/cfg_shared_networks.h index 8072306a24..a8de2f2047 100644 --- a/src/lib/dhcpsrv/cfg_shared_networks.h +++ b/src/lib/dhcpsrv/cfg_shared_networks.h @@ -50,7 +50,7 @@ public: "' found in the configuration"); } - networks_.push_back(network); + static_cast(networks_.push_back(network)); } /// @brief Deletes shared network from the configuration. @@ -202,7 +202,7 @@ public: (*other_network)->getCfgOption()->createOptions(cfg_def); // Add the new/updated nework. - networks_.push_back(*other_network); + static_cast(networks_.push_back(*other_network)); } } diff --git a/src/lib/dhcpsrv/cfg_subnets4.cc b/src/lib/dhcpsrv/cfg_subnets4.cc index 957a2c33e9..1a936210e9 100644 --- a/src/lib/dhcpsrv/cfg_subnets4.cc +++ b/src/lib/dhcpsrv/cfg_subnets4.cc @@ -38,7 +38,7 @@ CfgSubnets4::add(const Subnet4Ptr& subnet) { LOG_DEBUG(dhcpsrv_logger, DHCPSRV_DBG_TRACE, DHCPSRV_CFGMGR_ADD_SUBNET4) .arg(subnet->toText()); - subnets_.push_back(subnet); + static_cast(subnets_.push_back(subnet)); } Subnet4Ptr @@ -159,7 +159,7 @@ CfgSubnets4::merge(CfgOptionDefPtr cfg_def, CfgSharedNetworks4Ptr networks, } // Add the "other" subnet to the our collection of subnets. - subnets_.push_back(*other_subnet); + static_cast(subnets_.push_back(*other_subnet)); // If it belongs to a shared network, find the network and // add the subnet to it diff --git a/src/lib/dhcpsrv/cfg_subnets6.cc b/src/lib/dhcpsrv/cfg_subnets6.cc index 17ce409f42..852262c183 100644 --- a/src/lib/dhcpsrv/cfg_subnets6.cc +++ b/src/lib/dhcpsrv/cfg_subnets6.cc @@ -38,7 +38,7 @@ CfgSubnets6::add(const Subnet6Ptr& subnet) { LOG_DEBUG(dhcpsrv_logger, DHCPSRV_DBG_TRACE, DHCPSRV_CFGMGR_ADD_SUBNET6) .arg(subnet->toText()); - subnets_.push_back(subnet); + static_cast(subnets_.push_back(subnet)); } Subnet6Ptr @@ -161,7 +161,7 @@ CfgSubnets6::merge(CfgOptionDefPtr cfg_def, CfgSharedNetworks6Ptr networks, } // Add the "other" subnet to the our collection of subnets. - subnets_.push_back(*other_subnet); + static_cast(subnets_.push_back(*other_subnet)); // If it belongs to a shared network, find the network and // add the subnet to it diff --git a/src/lib/dhcpsrv/parsers/dhcp_parsers.cc b/src/lib/dhcpsrv/parsers/dhcp_parsers.cc index a920754db0..aa36e2e954 100644 --- a/src/lib/dhcpsrv/parsers/dhcp_parsers.cc +++ b/src/lib/dhcpsrv/parsers/dhcp_parsers.cc @@ -944,7 +944,11 @@ Subnets4ListConfigParser::parse(Subnet4Collection& subnets, Subnet4Ptr subnet = parser.parse(subnet_json); if (subnet) { try { - subnets.push_back(subnet); + auto ret = subnets.push_back(subnet); + if (!ret.second) { + isc_throw(Unexpected, + "can't store subnet because of conflict"); + } ++cnt; } catch (const std::exception& ex) { isc_throw(DhcpConfigError, ex.what() << " (" @@ -1311,12 +1315,18 @@ Subnets6ListConfigParser::parse(Subnet6Collection& subnets, Subnet6ConfigParser parser; Subnet6Ptr subnet = parser.parse(subnet_json); - try { - subnets.push_back(subnet); - ++cnt; - } catch (const std::exception& ex) { - isc_throw(DhcpConfigError, ex.what() << " (" - << subnet_json->getPosition() << ")"); + if (subnet) { + try { + auto ret = subnets.push_back(subnet); + if (!ret.second) { + isc_throw(Unexpected, + "can't store subnet because of conflict"); + } + ++cnt; + } catch (const std::exception& ex) { + isc_throw(DhcpConfigError, ex.what() << " (" + << subnet_json->getPosition() << ")"); + } } } return (cnt); diff --git a/src/lib/dhcpsrv/shared_network.cc b/src/lib/dhcpsrv/shared_network.cc index 8424f2d3f0..8f760892c3 100644 --- a/src/lib/dhcpsrv/shared_network.cc +++ b/src/lib/dhcpsrv/shared_network.cc @@ -66,7 +66,7 @@ public: } // Add a subnet to the collection of subnets for this shared network. - subnets.push_back(subnet); + static_cast(subnets.push_back(subnet)); } /// @brief Replaces IPv4 subnet in a shared network. -- 2.47.2