perfbook.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Leonardo Brás" <leobras.c@gmail.com>
To: Akira Yokosawa <akiyks@gmail.com>,
	"Paul E.McKenney" <paulmck@kernel.org>
Cc: perfbook@vger.kernel.org
Subject: [PATCH v2] CodeSamples/tree: Fix compiler warning on free
Date: Wed, 21 Jun 2023 02:18:31 -0300	[thread overview]
Message-ID: <b87a066b68747d3f22257355459734b759159516.camel@gmail.com> (raw)

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@gmail.com>
---
 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


             reply	other threads:[~2023-06-21  5:18 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-06-21  5:18 Leonardo Brás [this message]
2023-06-26  0:23 ` [PATCH v2] CodeSamples/tree: Fix compiler warning on free Paul E. McKenney

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=b87a066b68747d3f22257355459734b759159516.camel@gmail.com \
    --to=leobras.c@gmail.com \
    --cc=akiyks@gmail.com \
    --cc=paulmck@kernel.org \
    --cc=perfbook@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).