Creating a boost property tree recursively

Question

I am trying to create a boost property tree from a tree structure that I already have. I get a memeroy access violation error on the ptree node = *p line.

How should I change this code to make it work?

ptree* WebTreeView::insertProjectNode(std::shared_ptr<ITreeNode> projectNode)
{

if (projectNode->Children().size() == 0)
{
    return nullptr;
}

ptree children;

for (auto child : projectNode->Children())
{
    std::shared_ptr<ITreeNode> c(child); // cast raw pointer to shared pointer
    std::string nodetext = c->Name().c_str();

    ptree *p = insertProjectNode(c);

    if (p)
    {
        ptree node = *p; 
        children.put_child(nodetext, node);
    }
    else
    {
        children.put(nodetext, " ");
    }
}

return &children;
}

Answer 1

The worst offenders are

1. returning a dangling pointer to a local ptree

2. this line:

   std::shared_ptr<ITreeNode> c(child); // cast raw pointer to shared pointer
   

   That's not a cast. It's a conversion constructor. And by calling it you transfer ownership. This means, directly, that your loop delete all the children in the original ITreeNode tree (because when c is destructed, it implicitly does delete child since no one else holds the shared pointer).

I'm simplifying the code. Will post in a bit.

void WebTreeView::insertProjectNode(ITreeNode const& node, ptree& into) {
    ptree current;

    for (auto const* child : node.Children())
        if (child) insertProjectNode(*child, current);

    into.add_child(node.Name(), current);
}

Or, closer to your interface (but missing the step of naming the root:)

ptree WebTreeView::insertProjectNode(ITreeNode const& node) {
    ptree current;

    for (auto const* child : node.Children())
        if (child) 
            current.add_child(child->Name(), insertProjectNode(*child));

    return current;
}

Full Demo

Live On Coliru

#include <boost/property_tree/ptree.hpp>
#include <memory>
#include <iostream>
#include <list>

struct ITreeNode {
    std::string Name() const { return _name; }
    std::list<ITreeNode *> const &Children() const { return _children; }

    ITreeNode(ITreeNode const&)            = delete;
    ITreeNode& operator=(ITreeNode const&) = delete;

    ITreeNode(std::string name = "", std::list<ITreeNode*> const& children = {})
        : _name(std::move(name)),
          _children(children)
    {
    }

    ~ITreeNode() {
        for (auto* c : _children)
            delete c; // TODO make depthfirst deletion using iteration instead
                      // of breadth-first using recursion to avoid
                      // stack-overflow on large trees
    }
  private:
    std::string _name;
    std::list<ITreeNode *> _children;
};

using boost::property_tree::ptree;

namespace demo {
    ptree insertProjectNode(ITreeNode const& node);

    ptree insertProjectNode(ITreeNode const& node) {
        ptree current;

        for (auto const* child : node.Children())
            if (child) 
                current.add_child(child->Name(), insertProjectNode(*child));

        return current;
    }
}

#include <boost/property_tree/xml_parser.hpp>

int main() {

    ITreeNode const source = { "a", {
        new ITreeNode { "ab", {
            new ITreeNode { "ab0" },
            new ITreeNode { "ab1" },
            new ITreeNode { "ab2" },
        } },
        new ITreeNode { "ac", {
            new ITreeNode { "ac0" },
        } },
        new ITreeNode { "ad", {
            new ITreeNode { "ad0" },
            new ITreeNode { "ad1" },
            new ITreeNode { "ad2" },
            new ITreeNode { "ad3" },
        } },
    } };

    ptree root;
    root.add_child(source.Name(), demo::insertProjectNode(source));

    boost::property_tree::write_xml(std::cout, root,
            boost::property_tree::xml_writer_make_settings<std::string>(' ', 2));
}

Prints

<?xml version="1.0" encoding="utf-8"?>
<a>
  <ab>
    <ab0/>
    <ab1/>
    <ab2/>
  </ab>
  <ac>
    <ac0/>
  </ac>
  <ad>
    <ad0/>
    <ad1/>
    <ad2/>
    <ad3/>
  </ad>
</a>

Answer 2

ptree children; is a local variable, the memory will not be valid anymore when the method return. Change this to ptree &children = *(new ptree()); . But you need to be careful to delete it after use. It will be safer if your method return a normal ptree rather than 'pointer to ptree'. In this case you will not need to worry about new or delete.