diff --git a/ChangeLog b/ChangeLog index 2c2cd2f8..a39b8463 100644 --- a/ChangeLog +++ b/ChangeLog @@ -13,6 +13,12 @@ multi-batch, and repeated-enumeration cases. 2026-06-28 Todd White + * Source/CFTree.c (CFTreeRemove): Unlink the node from its parent's + list of children (it walked the wrong list and never unlinked, + leaving a dangling pointer in the parent), update the parent's + _lastChild, and clear the node's _parent and _nextSibling. + * Tests/CFTree/basic.m: Regression tests for CFTreeRemove; release + the tree before its (unretained) children. * Source/CFBinaryHeap.c (CFBinaryHeapCreateCopy): Enlarge the copy's capacity to at least the source count before copying the values. (CFBinaryHeapRemoveMinimumValue): Do nothing on an empty heap diff --git a/Source/CFTree.c b/Source/CFTree.c index 96e93c79..e04fbba1 100644 --- a/Source/CFTree.c +++ b/Source/CFTree.c @@ -195,18 +195,39 @@ CFTreePrependChild (CFTreeRef tree, CFTreeRef newChild) void CFTreeRemove (CFTreeRef tree) { - CFTreeRef child; - CFTreeRef previousSibling; - - previousSibling = NULL; - child = tree->_firstChild; - while (child != previousSibling) - child = child->_nextSibling; - - if (previousSibling) - previousSibling->_nextSibling = tree->_nextSibling; - - CFTreeFinalize (tree); + CFTreeRef parent = tree->_parent; + + if (parent == NULL) + return; + + /* Unlink the tree from its parent's list of children. */ + if (parent->_firstChild == tree) + { + parent->_firstChild = tree->_nextSibling; + } + else + { + CFTreeRef previousSibling = parent->_firstChild; + + while (previousSibling != NULL + && previousSibling->_nextSibling != tree) + previousSibling = previousSibling->_nextSibling; + if (previousSibling != NULL) + previousSibling->_nextSibling = tree->_nextSibling; + } + + /* Fix up the parent's last-child pointer if we removed the last child. */ + if (parent->_lastChild == tree) + { + CFTreeRef last = parent->_firstChild; + + while (last != NULL && last->_nextSibling != NULL) + last = last->_nextSibling; + parent->_lastChild = last; + } + + tree->_parent = NULL; + tree->_nextSibling = NULL; } void diff --git a/Tests/CFTree/basic.m b/Tests/CFTree/basic.m index 7e655ecb..efa4261f 100644 --- a/Tests/CFTree/basic.m +++ b/Tests/CFTree/basic.m @@ -52,10 +52,47 @@ int main (void) CFRelease (t2); } + { + /* CFTreeRemove must unlink the node from its parent. It used to walk + the wrong list, never unlink, and prematurely finalize the node, + leaving a dangling pointer in the parent. */ + CFTreeRef r = CFTreeCreate (NULL, &ctxt); + CFTreeRef r1 = CFTreeCreate (NULL, &ctxt); + CFTreeRef r2 = CFTreeCreate (NULL, &ctxt); + CFTreeRef r3 = CFTreeCreate (NULL, &ctxt); + + CFTreeAppendChild (r, r1); + CFTreeAppendChild (r, r2); + CFTreeAppendChild (r, r3); + + CFTreeRemove (r2); + PASS_CF(CFTreeGetChildCount (r) == 2 + && CFTreeGetNextSibling (r1) == r3 + && CFTreeGetParent (r2) == NULL, + "CFTreeRemove unlinks a child from its parent."); + + /* Removing the last child updates _lastChild, so a following append + links onto the right node. */ + CFTreeRemove (r3); + CFTreeAppendChild (r, r2); + PASS_CF(CFTreeGetChildCount (r) == 2 + && CFTreeGetChildAtIndex (r, 1) == r2, + "Append after removing the last child works."); + + CFRelease (r); + CFRelease (r1); + CFRelease (r2); + CFRelease (r3); + } + + /* Release the parent before its children: CFTreeAppendChild() does not + retain the child, so the children must outlive the tree that links + them. */ + CFRelease (tree); + CFRelease (child1); CFRelease (child2); CFRelease (child3); - CFRelease (tree); - + return 0; }