Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions ChangeLog
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,12 @@
multi-batch, and repeated-enumeration cases.

2026-06-28 Todd White <todd.white@thalion.global>
* 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
Expand Down
45 changes: 33 additions & 12 deletions Source/CFTree.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could be done faster by having the test after line 206 (where we just set _lastChild to NULL) and after line 216 (where we set it to previousSibling). But we can leave that optimization to later.

This reminds me of having to implement linked lists in Pascal some 40 something years ago :-)

{
CFTreeRef last = parent->_firstChild;

while (last != NULL && last->_nextSibling != NULL)
last = last->_nextSibling;
parent->_lastChild = last;
}

tree->_parent = NULL;
tree->_nextSibling = NULL;
}

void
Expand Down
41 changes: 39 additions & 2 deletions Tests/CFTree/basic.m
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Loading