Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [Medium] Memory leak: L2 and L3 function-view child entries are not freed on cleanup because `perf c2c` does not set `symbol_conf.report_hierarchy`, bypassing the recursive deletion in `hist_entry__delete()`.
--
--- Patch [3]: [PATCH 3/4] perf c2c: Add stdio support for the function view ---
commit e1f0ed49bafb9da749fbfa14e02ab8497ae9d5d4
Author: Jiebin Sun <[email protected]>
perf c2c: Add stdio support for the function view
This commit adds a --function option to perf c2c to print the fully expanded
three-level hierarchy to stdout, allowing its use without SLANG support,
when piped, or in scripts. It reuses the common function-view model while
preserving the boundary between utilities and the UI.
> diff --git a/tools/perf/util/c2c-function.c b/tools/perf/util/c2c-function.c
> --- a/tools/perf/util/c2c-function.c
> +++ b/tools/perf/util/c2c-function.c
[ ... ]
> @@ -1519,7 +1516,7 @@ void c2c_function__reset(void)
> symbol_conf.use_callchain = saved_use_callchain;
> }
[Severity: Medium]
This is a pre-existing issue, but does this teardown logic in
c2c_function__reset() leak the L2 and L3 function-view child entries?
When cleaning up the hierarchy, hists__delete_all_entries() relies on
hist_entry__delete() for each node. However, in tools/perf/util/hist.c,
hist_entry__delete() only recursively deletes the child entries in hroot_out
if the hierarchy flag is set:
tools/perf/util/hist.c:hist_entry__delete() {
...
if (symbol_conf.report_hierarchy) {
struct rb_root *root = &he->hroot_out.rb_root;
struct hist_entry *child, *tmp;
rbtree_postorder_for_each_entry_safe(child, tmp, root, rb_node)
hist_entry__delete(child);
*root = RB_ROOT;
}
...
}
Since perf c2c does not appear to set symbol_conf.report_hierarchy, won't
these child entries be orphaned and skipped during cleanup?
--
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=3
lmpx.com only provides a reader for public news (NNTP) servers. It is not
affiliated with the servers or forums shown here and is not responsible for
the content of articles, which is written by their respective authors.