My favorites | Sign in
Project Home Downloads Wiki Issues Source
New issue   Search
for
  Advanced search   Search tips   Subscriptions
Issue 58: GDataXMLNode doesn't support the NSXMLNode 'parent' method (Patch and Test included)
2 people starred this issue and may be notified of changes. Back to list
Status:  New
Owner:  ----


Sign in to add a comment
 
Reported by cyber...@gmail.com, Jun 25, 2010
What steps will reproduce the problem?
1. Any code that relies on the NSXMLNode parent attribute won't work, as GDataXMLNode doesn't supply it.  Since GDataXMLNode and GDataXMLElement attempt to mirror the NSXML equivalents, this signature missing causes problems.

What is the expected output? What do you see instead?
Expected behavior is that NSXMLNode#parent should return the parent object, or nil if it's not available.  Since NSXMLNode is replaced with GDataXMLNode, it should return the parent object for the parent method call.

What version of the product are you using? On what operating system?
I checked out trunk today, June 25, 2010; revision 538.  I'm using it in an iPhone project, but it's a problem in the code for any platform.

Please provide any additional information below.
Attached is a patch to fix it, along with a simple assertion to test it.

--  Morgan Schweers

add_parent_to_gdataxmlnode_signature.diff
2.1 KB   View   Download
test_gdataxmlnode_parent.diff
480 bytes   View   Download
Jun 25, 2010
Project Member #1 gregrobbins
We can't add pointers from children to parents at the GDataXMLNode level because that would create retain loops. 

Also complicating things is that "addChild:" doesn't add a pointer from parent to child, but rather makes a copy of the child. libxml isn't reference-counted so each GDataXMLNode must point to a unique copy of a libxml node, or else "borrow" it weakly.

A parent method like this would work, but might not give you the behavior you expect:

- (GDataXMLNode *)parent {
  if (xmlNode_ != NULL && xmlNode_->parent != NULL) {
    GDataXMLNode *parent = [GDataXMLNode nodeBorrowingXMLNode:xmlNode_->parent];
    return parent;
  }
  return nil;
}

It creates a new GDataXMLNode referencing the parent libxml node. There's no way to go from parent A to child B, then back to the parent A. It has to be a new GDataXMLNode that also points to the same parent data as A. But that new parent node can't guarantee the life of the underlying libxml tree data without copying it yet again.
Jun 26, 2010
#3 cyber...@gmail.com
Greetings,
Ah; yeah, I'm not sure that would do the right thing.  I'm relatively new to Objective C; is the problem with retain loops a garbage collection issue?

With your comment in mind I looked at -addChild anew.  Ignoring the fundamental problem of retain cycles, it's definitely the right thing to do to remove the child.parent=self's there, as they're strictly wrong, modifying the passed-in object, not the actual child object.

With that removed, because -addChild releases the cached values at the top of the -addChild method, the next reference to -children will recreate all the GDataXMLNode's, and since that code sets parent, it would do the right thing for my use case.

I'm less sure about the -addAttribute call; I think that I'd have to modify the -attributes method also, to set parent appropriately, again with the cache clearing that occurs in mind.

Poking around to try and understand, it seems like retain cycles shouldn't be an issue for non-GC'ed environments if the child never tries to release its parent.  Under GC, I could imagine it holding memory that should otherwise be released.

I understand that the patch, or probably any similar patch, isn't going to be helpful to the framework.  At this point I'm mainly trying to figure out how to make it work for my own code, which is an iPhone app, thus not facing GC issues.  Any suggestions or corrections are very welcome, as it'll help me learn ObjC better.  :)

--  Morgan Schweers

Jun 28, 2010
Project Member #4 gregrobbins
The -parent method I showed above is probably the most correct implementation given the underlying use of libxml, but I don't know if it's sufficient for your needs. 

Fundamentally, Apple's NSXML class only works perfectly with Apple's implementation (though they don't consider it perfect enough to put on the iPhone, clearly.)

Retain loops are a problem whenever a child points to a parent and the parent also points to the child, even indirectly. In this case, it's compounded by their being the real underlying libxml tree and the incomplete tree of GDataXML objects to try to maintain the NSXML interface.

Trying to keep track of parents and children in the GDataXMLNode is fraught with risk. The current implementation caches a bit for efficiency, but the actual tree is always the underlying libxml tree, never the GDataXML object tree.
Sign in to add a comment

Powered by Google Project Hosting