Melchior FRANZ:

- check for isTied() and refcount has to be made *before* we go into
  recursion, so as to pertain subtrees of refcounted nodes, even if there
  are no refcounted/tied nodes *in* this tree
- return value inverted, because it's more logical to say
  removeChildren() == true --> everything removed;  false --> failed
- further cleanup

simgear/props/props.cxx

@@ -942,44 +942,39 @@ SGPropertyNode::detachChild (const char * name, int index, bool keep)
 
 /**
  * Remove a child node, and all children that aren't referenced.
- * Returns "true" if not all nodes could be deleted.
+ * Returns "true" if the node and all subnodes could be removed.
  */
 bool
 SGPropertyNode::removeChild (const char * name, int index)
 {
-  bool dirty = false;
   int pos = find_child(name, index, _children);
   if (pos >= 0) {
     vector<SGPropertyNode_ptr>::iterator it = _children.begin();
     it += pos;
     SGPropertyNode *node = _children[pos];
-    if (node->nChildren() && node->removeChildren())
+    if (node->_count != 1 || node->isTied() || !node->removeChildren())
-      dirty = true;
+      return false;
 
-    if (node->isTied() || node->_count != 1 || node->nChildren())
+    if (_path_cache)
-      dirty = true;
+      _path_cache->erase(name); // EMH - TODO: Take "index" into account!
-    else {
-      if (_path_cache)
-        _path_cache->erase(name); // EMH - TODO: Take "index" into account!
 
-      node->setAttribute(REMOVED, true);
+    node->setAttribute(REMOVED, true);
-      node->clearValue();
+    node->clearValue();
-      fireChildRemoved(node);
+    fireChildRemoved(node);
-      _children.erase(it);
+    _children.erase(it);
-    }
   }
-  return dirty;
+  return true;
 }
 
 
 /**
- * Remove all children nodes, or all with a given name. Returns
+ * Remove all children nodes, or all with a given name.
- * "true" if not all nodes could be deleted.
+ * Returns "true" if the node and all subnodes could be removed.
  */
 bool
 SGPropertyNode::removeChildren(const char *name)
 {
-  bool dirty = false;
+  bool success = true;
   vector<SGPropertyNode_ptr>::iterator it = _children.end();
   vector<SGPropertyNode_ptr>::iterator begin = _children.begin();
   while (it-- != begin) {
@@ -987,11 +982,8 @@ SGPropertyNode::removeChildren(const char *name)
     if (name && !compare_strings(node->getName(), name))
       continue;
 
-    if (node->nChildren() && node->removeChildren())
+    if (node->_count != 1 || node->isTied() || !node->removeChildren())
-      dirty = true;
+      success = false;
-
-    if (node->isTied() || node->_count != 1 || node->nChildren())
-      dirty = true;
     else {
       if (_path_cache)
         _path_cache->erase(node->getName());
@@ -1002,7 +994,7 @@ SGPropertyNode::removeChildren(const char *name)
       _children.erase(it);
     }
   }
-  return dirty;
+  return success;
 }
 
 

simgear/props/props.hxx

@@ -714,14 +714,16 @@ public:
 
 
   /**
-   * Remove a child node. Returns "true" if at least one node had to remain,
+   * Remove a child node. Returns "true" if the node and all subnodes could
-   * because it was tied, aliased, or refcounted through SGPropertyNode_ptr.
+   * be removed, and "false" if at least one node had to remain, because it
+   * was tied, aliased, or refcounted through SGPropertyNode_ptr.
    */
   bool removeChild (const char * name, int index = 0);
 
 
   /**
-   * Remove all children nodes, or all with a given name.
+   * Remove all children nodes, or all with a given name. True if all nodes
+   * were removed.
    */
   bool removeChildren(const char * name = 0);