While building the CodeSamples/datastruct/Issaquah/ directory, I can see
a couple instances of this warning:
In function ‘free_treenode_cache’,
inlined from ‘tree_remove_all’ at tree.c:102:2,
inlined from ‘tree_free’ at tree.c:128:2:
tree.c:251:9: warning: ‘free’ called on pointer ‘trp’ with nonzero offset 96 [-Wfree-nonheap-object]
251 | free(tnp);
| ^~~~~~~~~
I took a look and tried to understand what was happening:
- tree_remove_all() calls free_treenode_cache() on it's input, which ends
up free()'ing it (!BAD_MALLOC)
- It makes sense in most treenodes, since they are allocated with
alloc_treenode_cache() and the malloc() output is the same as the free()
input.
- tree_free() calls tree_remove_all() on &trp->max, which ends up trying
to free() this same address.
- trp is a struct treeroot, which is composed of 2 treenodes: min & max
- The output of malloc() for trp ends up being different from the address
used for free(), since &trp->max is used instead, and there is an offset
since max is the second element of struct treeroot.
To solve this while keeping the tree_remove_all() generic, add a boolean
free_node parameter to tree_remove_all() so the caller can decide if the
node should be freed. The new boolean is true for normal treenodes, and
false if the pointed treenode is contained in the struct treeroot.
Signed-off-by: Leonardo Bras <leobras.c@xxxxxxxxx>
---
CodeSamples/datastruct/Issaquah/tree.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/CodeSamples/datastruct/Issaquah/tree.c b/CodeSamples/datastruct/Issaquah/tree.c
index 052fa870..26600118 100644
--- a/CodeSamples/datastruct/Issaquah/tree.c
+++ b/CodeSamples/datastruct/Issaquah/tree.c
@@ -89,17 +89,19 @@ struct treeroot *tree_alloc(void)
* readers accessing the tree.
*/
static void tree_remove_all(struct treenode *cur,
- void (*freefunc)(void *p))
+ void (*freefunc)(void *p),
+ bool free_node)
{
if (cur == NULL)
return;
- tree_remove_all(cur->lc, freefunc);
- tree_remove_all(cur->rc, freefunc);
+ tree_remove_all(cur->lc, freefunc, true);
+ tree_remove_all(cur->rc, freefunc, true);
if (cur->perm)
return;
if (cur->data && freefunc)
freefunc(cur->data);
- free_treenode_cache(cur);
+ if (free_node)
+ free_treenode_cache(cur);
}
/*
@@ -125,7 +127,7 @@ static void tree_check_insertion(struct treeroot *trp, struct treenode *new)
*/
void tree_free(struct treeroot *trp, void (*freefunc)(void *p))
{
- tree_remove_all(&trp->max, freefunc);
+ tree_remove_all(&trp->max, freefunc, false);
free(trp);
}
--
2.41.0
