Calling this method from other class creates violation reading

Question

I am trying to create a configuration class which reads the data from an xml with rapidxml. Therefore I have a private xml_document which I parse inside of the constructor:

class Config
{
public:
    Config();
    ~Config();
    /*returns the first found node*/
    xml_node<>* findNode(const char* name);
    /*get an configuration value. It's always a char*/
    char* getConfigValue(const char* name);

private:
    rapidxml::xml_document<>* m_doc;
};

//cpp
Config::Config()
{
    m_doc = new xml_document<>();
    rapidxml::file<> xmlFile("config.xml");
    m_doc->parse<0>(xmlFile.data());
    //LOG(getConfigValue("width")); // here it works
}
xml_node<>* Config::findNode(const char* name)
{
    return m_doc->first_node()->first_node(name);
}
char* Config::getConfigValue(const char* name){
    return m_doc->first_node()->first_node(name)->value();
}

Inside of the wWinMain I create an config opject and try to call the methods.

Config* config = new Config();
LOG(config->findNode("width")->value()); // this does create violation reading

BUT if I put the same line into the constructor of the Config class it works without any problem. What is going wrong here?

Answer 1

I'm not familiar with rapid xml, but a quick google search told me:

http://rapidxml.sourceforge.net/manual.html

RapidXml is an in-situ parser, which allows it to achieve very high parsing speed. In-situ means that parser does not make copies of strings. Instead, it places pointers to the source text in the DOM hierarchy.

3.1 Lifetime Of Source Text

In-situ parsing requires that source text lives at least as long as the document object. If source text is destroyed, names and values of nodes in DOM tree will become destroyed as well. Additionally, whitespace processing, character entity translation, and zero-termination of strings require that source text be modified during parsing (but see non-destructive mode). This makes the text useless for further processing once it was parsed by RapidXml.

So what I take from that is that you can't just have rapidxml::file<> xmlFile("config.xml"); be a stack variable as xml_document->parse() will create a tree that points directly at memory in xmlFile. So xmlFile has to be in memory for at least as long as the xml_document otherwise you will have invalid data. This means that xmlFile needs to be a member of your Config class. Also, I don't see you initializing m_doc, did you just omit some code? Otherwise it should be complaining sooner and never work. Also, you should always check to make sure functions that return pointers aren't returning null before you access them to avoid getting reading violations.

So what you really want is something like this:

class Config
{
public:
    Config();
    ~Config();
    /*returns the first found node*/
    xml_node<>* findNode(const char* name);
    /*get an configuration value. It's always a char*/
    char* getConfigValue(const char* name);

private:
    rapidxml::file<> m_xmlFile;
    rapidxml::xml_document<> m_doc;
};

//cpp
Config::Config()
    : m_xmlFile("config.xml")
{
    m_doc.parse<0>(m_xmlFile.data());
}

xml_node<>* Config::findNode(const char* name)
{
    if (m_doc.first_node())
    {
        return m_doc.first_node()->first_node(name);
    }

    return 0;
}

char* Config::getConfigValue(const char* name)
{
    if (m_doc.first_node())
    {
        xml_node<>* node = m_doc.first_node()->first_node(name);
        if (node)
        {
            return node->value();
        }
    }

    return 0;
}