Pointers to objects in function scope

Question

When you create some pointers inside the scope of a function, what happens when the function goes out of scope? Do they get destroyed or should I call delete on them at some point?

void XMLDocument::AddNode(XMLNode& node)
{
    std::string val = node.GetNodeName();
    TiXmlElement* el = new TiXmlElement(val.c_str());  // What about this object ptr?
    TiXmlText * txt = new TiXmlText(node.GetNodeValue().c_str());  // And this one?
    el->LinkEndChild(txt);
    document.LinkEndChild(el);
}

Answer 1

Normally, you need to call delete on both the pointers in order to avoid memory leaks. But in you case, it looks like you are putting this pointer in a document object (I am assuming document stores the pointer itself and will not create copy of the pointed to object itself). So if you call delete here, the pointer you have given to the document object will be invalid one. So if document object takes ownership of deleting the memory you allocated in this function, then you should not call delete here, else if it is creating the copy of the object you passed, then you need to delete it here. Using a smart pointer will be quite useful in these type of scenarios. Take a look at boost::shared_ptr for more details.

Answer 2

When you use new, memory from the heap is allocated, while the objects locally declared in a function are allocated on the stack.
The objects allocated to the stack stop to exist after the method call.
In your case TiXmlElement* el is a pointer allocated on the stack that references memory on the heap. Once you leave the fuction, the el pointer will stop to exist (since it was on the stack) but the memory that it referenced, still exists (was on the heap). So if that memory is not "freed", it is considered as "leaked"

Answer 3

The only thing that can be stated for sure is that immediately after the exit of this function, neither of those pointers has been returned to the heap.

What happens after the function exits depends on the contract for memory usage that is implied by passing a pointer into LinkEndChild. Do you still own the memory after this, or does it get cleaned up when the new parent (in this case document, which accesses the memory directly for el, and via el for txt) is cleaned up? If the former, then you are correct not to delete it. If the latter, then at some point you need to clean it up, presumably after document (global? class member?) is done with.

A further nuance is what is the contract on construction of TiXmlElement and TiXmlText - do they copy the input C-string, or use them by reference?

I would guess that it's XmlDocument's doc to clean up all memory that is linked into the document. Kind of messy, though. Check the destructor XmlDocument::~XlmDocument, see if it walks a list of children cleaning them up (recursively).

EDIT: This looks like TinyXML, in which case it's supposed to clean up all the memory you give it on destruction. See here for a discussion : http://www.gamedev.net/community/forums/topic.asp?topic_id=518023

The semantics of TinyXML clearly imply that (a) it owns the document tree, and (b) you pass in nodes that you allocate. I can see no good argument for it not simply using the delete operator on everything linked into it.

EDIT: In fact, given that TinyXML (or at least the copy I have here) clearly has while ( node ) { temp = node; node = node->next; delete temp; } in the TiXMLNode destructor, I'm guessing this is just a bug in the TinyXML++ wrapper.

See also previous Stack Overflow question about TinyXml memory management here.

The documentation for LinkEndChild says this:

NOTE: the node to be added is passed by pointer, and will be henceforth owned (and deleted) by tinyXml. This method is efficient and avoids an extra copy, but should be used with care as it uses a different memory model than the other insert functions.