Skip to content

Commit

Permalink
Fix memory leak when accessing a const Node with a key that doesn't e…
Browse files Browse the repository at this point in the history
…xist.
  • Loading branch information
jbeder committed Jan 24, 2015
1 parent a5e86cd commit 1025f76
Show file tree
Hide file tree
Showing 8 changed files with 41 additions and 26 deletions.
12 changes: 6 additions & 6 deletions include/yaml-cpp/node/detail/impl.h
Expand Up @@ -58,28 +58,28 @@ struct get_idx<Key, typename boost::enable_if<boost::is_signed<Key> >::type> {

// indexing
template <typename Key>
inline node& node_data::get(const Key& key,
inline node* node_data::get(const Key& key,
shared_memory_holder pMemory) const {
switch (m_type) {
case NodeType::Map:
break;
case NodeType::Undefined:
case NodeType::Null:
return pMemory->create_node();
return NULL;
case NodeType::Sequence:
if (node* pNode = get_idx<Key>::get(m_sequence, key, pMemory))
return *pNode;
return pMemory->create_node();
return pNode;
return NULL;
case NodeType::Scalar:
throw BadSubscript();
}

for (node_map::const_iterator it = m_map.begin(); it != m_map.end(); ++it) {
if (equals(*it->first, key, pMemory))
return *it->second;
return it->second;
}

return pMemory->create_node();
return NULL;
}

template <typename Key>
Expand Down
10 changes: 8 additions & 2 deletions include/yaml-cpp/node/detail/node.h
Expand Up @@ -110,7 +110,10 @@ class node : private boost::noncopyable {

// indexing
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);
}
template <typename Key>
Expand All @@ -124,7 +127,10 @@ class node : private boost::noncopyable {
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);
}
node& get(node& key, shared_memory_holder pMemory) {
Expand Down
4 changes: 2 additions & 2 deletions include/yaml-cpp/node/detail/node_data.h
Expand Up @@ -63,13 +63,13 @@ class YAML_CPP_API node_data : private boost::noncopyable {

// indexing
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>
node& get(const Key& key, shared_memory_holder pMemory);
template <typename Key>
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);
bool remove(node& key, shared_memory_holder pMemory);

Expand Down
4 changes: 2 additions & 2 deletions include/yaml-cpp/node/detail/node_ref.h
Expand Up @@ -57,7 +57,7 @@ class node_ref : private boost::noncopyable {

// indexing
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);
}
template <typename Key>
Expand All @@ -69,7 +69,7 @@ class node_ref : private boost::noncopyable {
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);
}
node& get(node& key, shared_memory_holder pMemory) {
Expand Down
14 changes: 10 additions & 4 deletions include/yaml-cpp/node/impl.h
Expand Up @@ -366,9 +366,12 @@ inline const Node Node::operator[](const Key& key) const {
if (!m_isValid)
throw InvalidNode();
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);
return Node(value, m_pMemory);
if (!value) {
return Node(ZombieNode);
}
return Node(*value, m_pMemory);
}

template <typename Key>
Expand All @@ -394,9 +397,12 @@ inline const Node Node::operator[](const Node& key) const {
EnsureNodeExists();
key.EnsureNodeExists();
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);
return Node(value, m_pMemory);
if (!value) {
return Node(ZombieNode);
}
return Node(*value, m_pMemory);
}

inline Node Node::operator[](const Node& key) {
Expand Down
1 change: 1 addition & 0 deletions include/yaml-cpp/node/node.h
Expand Up @@ -129,6 +129,7 @@ class YAML_CPP_API Node {
bool m_isValid;
mutable detail::shared_memory_holder m_pMemory;
mutable detail::node* m_pNode;
mutable const detail::node* m_pConstNode;
};

YAML_CPP_API bool operator==(const Node& lhs, const Node& rhs);
Expand Down
11 changes: 6 additions & 5 deletions src/node_data.cpp
Expand Up @@ -192,16 +192,17 @@ void node_data::insert(node& key, node& value, shared_memory_holder pMemory) {
}

// indexing
node& node_data::get(node& key, shared_memory_holder pMemory) const {
if (m_type != NodeType::Map)
return pMemory->create_node();
node* node_data::get(node& key, shared_memory_holder /* pMemory */) const {
if (m_type != NodeType::Map) {
return NULL;
}

for (node_map::const_iterator it = m_map.begin(); it != m_map.end(); ++it) {
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) {
Expand Down
11 changes: 6 additions & 5 deletions util/sandbox.cpp
Expand Up @@ -26,10 +26,11 @@ class NullEventHandler : public YAML::EventHandler {
};

int main() {
std::stringstream stream("---{header: {id: 1");
YAML::Parser parser(stream);
// parser.PrintTokens(std::cout);
NullEventHandler handler;
parser.HandleNextDocument(handler);
const YAML::Node node;

std::string key = "doesnotexist";
for (;;) {
node[key];
}
return 0;
}

0 comments on commit 1025f76

Please sign in to comment.