Fix memory leak when accessing a const Node with a key that doesn't exist.

This commit is contained in:
Jesse Beder
2015-01-24 17:22:45 -06:00
parent a5e86cde59
commit 1025f76df1
8 changed files with 41 additions and 26 deletions

View File

@@ -58,28 +58,28 @@ struct get_idx<Key, typename boost::enable_if<boost::is_signed<Key> >::type> {
// indexing // indexing
template <typename Key> template <typename Key>
inline node& node_data::get(const Key& key, inline node* node_data::get(const Key& key,
shared_memory_holder pMemory) const { shared_memory_holder pMemory) const {
switch (m_type) { switch (m_type) {
case NodeType::Map: case NodeType::Map:
break; break;
case NodeType::Undefined: case NodeType::Undefined:
case NodeType::Null: case NodeType::Null:
return pMemory->create_node(); return NULL;
case NodeType::Sequence: case NodeType::Sequence:
if (node* pNode = get_idx<Key>::get(m_sequence, key, pMemory)) if (node* pNode = get_idx<Key>::get(m_sequence, key, pMemory))
return *pNode; return pNode;
return pMemory->create_node(); return NULL;
case NodeType::Scalar: case NodeType::Scalar:
throw BadSubscript(); throw BadSubscript();
} }
for (node_map::const_iterator it = m_map.begin(); it != m_map.end(); ++it) { for (node_map::const_iterator it = m_map.begin(); it != m_map.end(); ++it) {
if (equals(*it->first, key, pMemory)) if (equals(*it->first, key, pMemory))
return *it->second; return it->second;
} }
return pMemory->create_node(); return NULL;
} }
template <typename Key> template <typename Key>

View File

@@ -110,7 +110,10 @@ class node : private boost::noncopyable {
// indexing // indexing
template <typename Key> template <typename Key>
node& get(const Key& key, shared_memory_holder pMemory) const { node* get(const Key& key, shared_memory_holder pMemory) const {
// NOTE: this returns a non-const node so that the top-level Node can wrap
// it, and returns a pointer so that it can be NULL (if there is no such
// key).
return static_cast<const node_ref&>(*m_pRef).get(key, pMemory); return static_cast<const node_ref&>(*m_pRef).get(key, pMemory);
} }
template <typename Key> template <typename Key>
@@ -124,7 +127,10 @@ class node : private boost::noncopyable {
return m_pRef->remove(key, pMemory); return m_pRef->remove(key, pMemory);
} }
node& get(node& key, shared_memory_holder pMemory) const { node* get(node& key, shared_memory_holder pMemory) const {
// NOTE: this returns a non-const node so that the top-level Node can wrap
// it, and returns a pointer so that it can be NULL (if there is no such
// key).
return static_cast<const node_ref&>(*m_pRef).get(key, pMemory); return static_cast<const node_ref&>(*m_pRef).get(key, pMemory);
} }
node& get(node& key, shared_memory_holder pMemory) { node& get(node& key, shared_memory_holder pMemory) {

View File

@@ -63,13 +63,13 @@ class YAML_CPP_API node_data : private boost::noncopyable {
// indexing // indexing
template <typename Key> template <typename Key>
node& get(const Key& key, shared_memory_holder pMemory) const; node* get(const Key& key, shared_memory_holder pMemory) const;
template <typename Key> template <typename Key>
node& get(const Key& key, shared_memory_holder pMemory); node& get(const Key& key, shared_memory_holder pMemory);
template <typename Key> template <typename Key>
bool remove(const Key& key, shared_memory_holder pMemory); bool remove(const Key& key, shared_memory_holder pMemory);
node& get(node& key, shared_memory_holder pMemory) const; node* get(node& key, shared_memory_holder pMemory) const;
node& get(node& key, shared_memory_holder pMemory); node& get(node& key, shared_memory_holder pMemory);
bool remove(node& key, shared_memory_holder pMemory); bool remove(node& key, shared_memory_holder pMemory);

View File

@@ -57,7 +57,7 @@ class node_ref : private boost::noncopyable {
// indexing // indexing
template <typename Key> template <typename Key>
node& get(const Key& key, shared_memory_holder pMemory) const { node* get(const Key& key, shared_memory_holder pMemory) const {
return static_cast<const node_data&>(*m_pData).get(key, pMemory); return static_cast<const node_data&>(*m_pData).get(key, pMemory);
} }
template <typename Key> template <typename Key>
@@ -69,7 +69,7 @@ class node_ref : private boost::noncopyable {
return m_pData->remove(key, pMemory); return m_pData->remove(key, pMemory);
} }
node& get(node& key, shared_memory_holder pMemory) const { node* get(node& key, shared_memory_holder pMemory) const {
return static_cast<const node_data&>(*m_pData).get(key, pMemory); return static_cast<const node_data&>(*m_pData).get(key, pMemory);
} }
node& get(node& key, shared_memory_holder pMemory) { node& get(node& key, shared_memory_holder pMemory) {

View File

@@ -366,9 +366,12 @@ inline const Node Node::operator[](const Key& key) const {
if (!m_isValid) if (!m_isValid)
throw InvalidNode(); throw InvalidNode();
EnsureNodeExists(); EnsureNodeExists();
detail::node& value = static_cast<const detail::node&>(*m_pNode) detail::node* value = static_cast<const detail::node&>(*m_pNode)
.get(detail::to_value(key), m_pMemory); .get(detail::to_value(key), m_pMemory);
return Node(value, m_pMemory); if (!value) {
return Node(ZombieNode);
}
return Node(*value, m_pMemory);
} }
template <typename Key> template <typename Key>
@@ -394,9 +397,12 @@ inline const Node Node::operator[](const Node& key) const {
EnsureNodeExists(); EnsureNodeExists();
key.EnsureNodeExists(); key.EnsureNodeExists();
m_pMemory->merge(*key.m_pMemory); m_pMemory->merge(*key.m_pMemory);
detail::node& value = detail::node* value =
static_cast<const detail::node&>(*m_pNode).get(*key.m_pNode, m_pMemory); static_cast<const detail::node&>(*m_pNode).get(*key.m_pNode, m_pMemory);
return Node(value, m_pMemory); if (!value) {
return Node(ZombieNode);
}
return Node(*value, m_pMemory);
} }
inline Node Node::operator[](const Node& key) { inline Node Node::operator[](const Node& key) {

View File

@@ -129,6 +129,7 @@ class YAML_CPP_API Node {
bool m_isValid; bool m_isValid;
mutable detail::shared_memory_holder m_pMemory; mutable detail::shared_memory_holder m_pMemory;
mutable detail::node* m_pNode; mutable detail::node* m_pNode;
mutable const detail::node* m_pConstNode;
}; };
YAML_CPP_API bool operator==(const Node& lhs, const Node& rhs); YAML_CPP_API bool operator==(const Node& lhs, const Node& rhs);

View File

@@ -192,16 +192,17 @@ void node_data::insert(node& key, node& value, shared_memory_holder pMemory) {
} }
// indexing // indexing
node& node_data::get(node& key, shared_memory_holder pMemory) const { node* node_data::get(node& key, shared_memory_holder /* pMemory */) const {
if (m_type != NodeType::Map) if (m_type != NodeType::Map) {
return pMemory->create_node(); return NULL;
}
for (node_map::const_iterator it = m_map.begin(); it != m_map.end(); ++it) { for (node_map::const_iterator it = m_map.begin(); it != m_map.end(); ++it) {
if (it->first->is(key)) if (it->first->is(key))
return *it->second; return it->second;
} }
return pMemory->create_node(); return NULL;
} }
node& node_data::get(node& key, shared_memory_holder pMemory) { node& node_data::get(node& key, shared_memory_holder pMemory) {

View File

@@ -26,10 +26,11 @@ class NullEventHandler : public YAML::EventHandler {
}; };
int main() { int main() {
std::stringstream stream("---{header: {id: 1"); const YAML::Node node;
YAML::Parser parser(stream);
// parser.PrintTokens(std::cout); std::string key = "doesnotexist";
NullEventHandler handler; for (;;) {
parser.HandleNextDocument(handler); node[key];
}
return 0; return 0;
} }