On Thursday, September 7, 2017 4:58:53 PM CEST Namhyung Kim wrote:

Hi Milian,

Hey Namhyung!

quoted

quoted

quoted

quoted

@@ -511,10 +563,63 @@ void inline_node__delete(struct inline_node

*node)

 	list_for_each_entry_safe(ilist, tmp, &node->val, list) {
 	
 		list_del_init(&ilist->list);

-		zfree(&ilist->filename);
-		zfree(&ilist->funcname);
+		zfree(&ilist->srcline);
+		// only the inlined symbols are owned by the list
+		if (ilist->symbol && ilist->symbol->inlined)
+			symbol__delete(ilist->symbol);

Existing symbols are released at this moment.

Thanks for the review, I'll try to look into these issues once I have
more
time again.

OK, so I just dug into this part of the patch again. I don't think it's
actually a problem after all:

When an inline node reuses the real symbol, that symbol won't have its
`inlined` member set to true. Thus these symbols will never get deleted by
inline_node__delete.

But ilist->symbol is a dangling pointer so accessing ->inlined would
be a problem, no?

Sorry, but I can't follow. Why would it be a dangling pointer? Note, again, 
that I've tested this with both valgrind and ASAN and neither reports any 
issues about this code.

quoted

If you have suggestions on how to make this clearer, I'm
all ears. For now, I'll add a comment to where we alias/reuse the symbol.

I'll try to split up the patch now to make it somehow easier to review.

Thanks for doing this, but I'm afraid I don't have time to review
before going to OSS-NA.

No problem, enjoy!
-- 
Milian Wolff | milian.wolff@kdab.com | Senior Software Engineer
KDAB (Deutschland) GmbH&Co KG, a KDAB Group company
Tel: +49-30-521325470
KDAB - The Qt Experts