perf ftrace: Proper error clean up in print_profile_result

[PATCH v1] perf ftrace: Proper error clean up in print_profile_result

Posted by Ian Rogers 1 month ago

The ftrace profile_hash is cleaned up on success but not on memory
allocation and empty paths. Fix by running the clean up for all. Free
the profile too, so make it NULL, just for clarity that this is all
clean up code.

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/builtin-ftrace.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/tools/perf/builtin-ftrace.c b/tools/perf/builtin-ftrace.c
index 6b6eec65f93f..19e796e58fb0 100644
--- a/tools/perf/builtin-ftrace.c
+++ b/tools/perf/builtin-ftrace.c
@@ -18,6 +18,7 @@
 #include <poll.h>
 #include <ctype.h>
 #include <linux/capability.h>
+#include <linux/err.h>
 #include <linux/string.h>
 #include <sys/stat.h>
 
@@ -1369,17 +1370,17 @@ static int cmp_profile_data(const void *a, const void *b)
 
 static void print_profile_result(struct perf_ftrace *ftrace)
 {
-	struct hashmap_entry *entry, **profile;
+	struct hashmap_entry *entry, **profile = NULL;
 	size_t i, nr, bkt;
 
 	nr = hashmap__size(ftrace->profile_hash);
 	if (nr == 0)
-		return;
+		goto out;
 
 	profile = calloc(nr, sizeof(*profile));
 	if (profile == NULL) {
 		pr_err("failed to allocate memory for the result\n");
-		return;
+		goto out;
 	}
 
 	i = 0;
@@ -1403,6 +1404,7 @@ static void print_profile_result(struct perf_ftrace *ftrace)
 		       p->st.max / 1000, p->st.max % 1000, p->st.n, name);
 	}
 
+out:
 	free(profile);
 
 	hashmap__for_each_entry(ftrace->profile_hash, entry, bkt) {
-- 
2.53.0.473.g4a7958ca14-goog

Re: [PATCH v1] perf ftrace: Proper error clean up in print_profile_result

Posted by Ian Rogers 2 weeks, 5 days ago

On Fri, Mar 6, 2026 at 10:50 AM Ian Rogers <irogers@google.com> wrote:
>
> The ftrace profile_hash is cleaned up on success but not on memory
> allocation and empty paths. Fix by running the clean up for all. Free
> the profile too, so make it NULL, just for clarity that this is all
> clean up code.
>
> Signed-off-by: Ian Rogers <irogers@google.com>

Ping.

Thanks,
Ian

> ---
>  tools/perf/builtin-ftrace.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/tools/perf/builtin-ftrace.c b/tools/perf/builtin-ftrace.c
> index 6b6eec65f93f..19e796e58fb0 100644
> --- a/tools/perf/builtin-ftrace.c
> +++ b/tools/perf/builtin-ftrace.c
> @@ -18,6 +18,7 @@
>  #include <poll.h>
>  #include <ctype.h>
>  #include <linux/capability.h>
> +#include <linux/err.h>
>  #include <linux/string.h>
>  #include <sys/stat.h>
>
> @@ -1369,17 +1370,17 @@ static int cmp_profile_data(const void *a, const void *b)
>
>  static void print_profile_result(struct perf_ftrace *ftrace)
>  {
> -       struct hashmap_entry *entry, **profile;
> +       struct hashmap_entry *entry, **profile = NULL;
>         size_t i, nr, bkt;
>
>         nr = hashmap__size(ftrace->profile_hash);
>         if (nr == 0)
> -               return;
> +               goto out;
>
>         profile = calloc(nr, sizeof(*profile));
>         if (profile == NULL) {
>                 pr_err("failed to allocate memory for the result\n");
> -               return;
> +               goto out;
>         }
>
>         i = 0;
> @@ -1403,6 +1404,7 @@ static void print_profile_result(struct perf_ftrace *ftrace)
>                        p->st.max / 1000, p->st.max % 1000, p->st.n, name);
>         }
>
> +out:
>         free(profile);
>
>         hashmap__for_each_entry(ftrace->profile_hash, entry, bkt) {
> --
> 2.53.0.473.g4a7958ca14-goog
>

Re: [PATCH v1] perf ftrace: Proper error clean up in print_profile_result

Posted by Ian Rogers 1 week, 1 day ago

On Fri, Mar 20, 2026 at 7:50 PM Ian Rogers <irogers@google.com> wrote:
>
> On Fri, Mar 6, 2026 at 10:50 AM Ian Rogers <irogers@google.com> wrote:
> >
> > The ftrace profile_hash is cleaned up on success but not on memory
> > allocation and empty paths. Fix by running the clean up for all. Free
> > the profile too, so make it NULL, just for clarity that this is all
> > clean up code.
> >
> > Signed-off-by: Ian Rogers <irogers@google.com>
>
> Ping.

Ping.

Thanks,
Ian

> Thanks,
> Ian
>
> > ---
> >  tools/perf/builtin-ftrace.c | 8 +++++---
> >  1 file changed, 5 insertions(+), 3 deletions(-)
> >
> > diff --git a/tools/perf/builtin-ftrace.c b/tools/perf/builtin-ftrace.c
> > index 6b6eec65f93f..19e796e58fb0 100644
> > --- a/tools/perf/builtin-ftrace.c
> > +++ b/tools/perf/builtin-ftrace.c
> > @@ -18,6 +18,7 @@
> >  #include <poll.h>
> >  #include <ctype.h>
> >  #include <linux/capability.h>
> > +#include <linux/err.h>
> >  #include <linux/string.h>
> >  #include <sys/stat.h>
> >
> > @@ -1369,17 +1370,17 @@ static int cmp_profile_data(const void *a, const void *b)
> >
> >  static void print_profile_result(struct perf_ftrace *ftrace)
> >  {
> > -       struct hashmap_entry *entry, **profile;
> > +       struct hashmap_entry *entry, **profile = NULL;
> >         size_t i, nr, bkt;
> >
> >         nr = hashmap__size(ftrace->profile_hash);
> >         if (nr == 0)
> > -               return;
> > +               goto out;
> >
> >         profile = calloc(nr, sizeof(*profile));
> >         if (profile == NULL) {
> >                 pr_err("failed to allocate memory for the result\n");
> > -               return;
> > +               goto out;
> >         }
> >
> >         i = 0;
> > @@ -1403,6 +1404,7 @@ static void print_profile_result(struct perf_ftrace *ftrace)
> >                        p->st.max / 1000, p->st.max % 1000, p->st.n, name);
> >         }
> >
> > +out:
> >         free(profile);
> >
> >         hashmap__for_each_entry(ftrace->profile_hash, entry, bkt) {
> > --
> > 2.53.0.473.g4a7958ca14-goog
> >

Re: [PATCH v1] perf ftrace: Proper error clean up in print_profile_result

Posted by Namhyung Kim 6 days, 11 hours ago

On Tue, Mar 31, 2026 at 10:53:57PM -0700, Ian Rogers wrote:
> On Fri, Mar 20, 2026 at 7:50 PM Ian Rogers <irogers@google.com> wrote:
> >
> > On Fri, Mar 6, 2026 at 10:50 AM Ian Rogers <irogers@google.com> wrote:
> > >
> > > The ftrace profile_hash is cleaned up on success but not on memory
> > > allocation and empty paths. Fix by running the clean up for all. Free
> > > the profile too, so make it NULL, just for clarity that this is all
> > > clean up code.
> > >
> > > Signed-off-by: Ian Rogers <irogers@google.com>
> >
> > Ping.
> 
> Ping.

Sorry for the delay.  Can you please rebase the change?  It doesn't
apply anymore.

Thanks,
Namhyung