RapidXML NULL Pointer

Question

I'm getting values of my config file with rapidXML in a kindly bad way.

xml_document<> doc;
doc.parse<parse_full>(buffer);
int a = atoi(doc.first_node("master")->first_node("profile")->first_node("width")->value());

If the node doesn't exist "first_node" return 0 so "->value()" crash. Returning a new xml_node will fix the crash but what about the memory leaks?

This is the rapidXML's fonction with the old and new return :

    xml_node<Ch> *first_node(const Ch *name = 0, std::size_t name_size = 0, bool case_sensitive = true) const
    {
        if (name)
        {
            if (name_size == 0)
                name_size = internal::measure(name);
            for (xml_node<Ch> *child = m_first_node; child; child = child->next_sibling())
                if (internal::compare(child->name(), child->name_size(), name, name_size, case_sensitive))
                    return child;
            return new xml_node<Ch>(node_document);
            //return 0;
        }
        else
            return m_first_node;
    }

You should check that each node in the chain exist instead of changing the library to return a new empty node. That _will_ create memory leaks. — Some programmer dude, Sep 04 '12 at 06:18
I don't wanna check for each node to exist before reaching the next one. I would like to be able to shorten the lines like : doc.first_node("master/profile/width").value() without checking if I got a NULL pointer. Do you find this more user friendly? Also, how do I avoid memory leaks. How the newly created element to be destroyed automatically? — Adam Paquette, Sep 05 '12 at 22:56
For avoiding memory leaks, you should probably look into [smart pointers](http://en.cppreference.com/w/cpp/memory). — Some programmer dude, Sep 06 '12 at 05:40

score 0 · Answer 1 · answered Nov 06 '12 at 15:13

You should use exceptions, like:

int a;
try
{
    xml_document<> doc;
    doc.parse<parse_full>(buffer);
    a = atoi(doc.first_node("master")->first_node("profile")->first_node("width")->value());
}
catch(NodeNotExists &ex)
{
    // process error condition
}

Also, if don't want to change rapidxml code, use wrapper function like:

template<typename T>
T* notnull(T* arg)
{
    if (arg == 0)
        throw NodeNotExists();
    return arg;
}

score 0 · Answer 2 · edited May 23 '17 at 10:25

Expanding on hate-engine's answer, I created a class of static helper functions to handle this situation by wrapping the node value access. Check out my answer over here. You could expand upon that or pursue something more along the lines of a xpath-style of node access (which I don't believe is built-in to RapidXML currently but others do).

Edit: You could create a wrapper function with that takes a simple xpath-like path and combine that with one of my wrappers to get something like...

int a = GetNodeInt(GetFirstNodePath(doc, "master/profile/width"), 0);

Where GetNodeInt and GetFirstNodePath are defined something like this...

    int GetNodeInt(rapidxml::xml_base<>* node, int DefaultValue)
    {
        int temp;
        try
        {
            if (node == 0) return DefaultValue;
            if (sscanf(node->value(), "%d", &temp) != 1) return DefaultValue;
            return temp;
        }
        catch (...) { return DefaultValue; }
    }

    // Find node with simple path from current node such as "first/second/third"
    rapidxml::xml_node<>* GetFirstNodePath(rapidxml::xml_node<>* root, const char *path)
    {
        xml_node<>* node = root;
        char* last = (char*)path; // only using this pointer to read
        char* cur = last;
        if (cur == 0 || root == 0) return 0;
        while (node)
        {
            if (*cur == '/' || *cur == 0)
            {
                if (cur != last) node = node->first_node(last,cur-last);
                if (*cur == 0) break;
                last = cur + 1;
            }
            ++cur;
        }
        return node;
    }

Now this code has not been thoroughly tested, but gives you the general idea.

RapidXML NULL Pointer

2 Answers2