Recursive bug when inserting childs.

[Paco Arjonilla]

There is a bug where the library gets into an infinite loop when adding by pointer a new node. If the node added is already inserted as an ancestor of itself, the infinite loop occurs. I have found three possible solutions:
1) Leave it as it is, and write a note stating to be careful when using LinkEndChild(). This is not elegant, and not error-tolerant.
2) Make LinkEndChild copy the node lo be linked. This makes an independent subtree, the same as the original, without the bug. The pitfall is that the speed optimization is lost.
3) Make a check to look if the pointer to be added is already in the DOM when comparing to the parents pointers. This is not such a speed and memory eater as 2), but the code size will increase a bit.

I have already implemented 3), please tell me your opinion.
Regards
Paco

[Ellers]

Interesting point. This becomes a design question, and IMO (though I'm not the designer) it looks to me like TinyXml was designed to be lightweight, so LinkEndChild() is a linked-list style function. So if you add a node already linked, well, you shouldn't :)

I think (1) should be done anyway, though I thought it was kinda obvious.

I think the caller has the option of doing (2) [ie copying the node], but the code of LinkEndChild() should definitely not. That is, if in doubt, you can do pTree->LinkEndChild(pFoo->Clone());

And alternatively a function could be written like pTree->CheckContains(pFoo) so you can check if its contained before adding it. That way TinyXml remains lean but if your code does things like adding one node twice then you have an option. You could write this function without too much trouble.

Oh, an alternative: Each node has a ptr to its parent; so prior to calling LinkEndChild() you can check if GetParent()==null, e.g.:

  pTree->LinkEndChild( (pNode->GetParent() ? pNode->Clone() : pNode )

but messy tho.

HTH