All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] perf list: Fix memory leaks in print_tracepoint_events()
@ 2023-04-27 23:05 Namhyung Kim
  2023-04-27 23:05 ` [PATCH 2/2] perf list: Modify the warning message about scandirat(3) Namhyung Kim
  2023-04-28  0:27 ` [PATCH 1/2] perf list: Fix memory leaks in print_tracepoint_events() Ian Rogers
  0 siblings, 2 replies; 5+ messages in thread
From: Namhyung Kim @ 2023-04-27 23:05 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo, Jiri Olsa
  Cc: Ian Rogers, Adrian Hunter, Peter Zijlstra, Ingo Molnar, LKML,
	linux-perf-users

It should free entries (not only the array) filled by scandirat()
after use.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/util/print-events.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/tools/perf/util/print-events.c b/tools/perf/util/print-events.c
index d416c5484cd5..0a97912fd894 100644
--- a/tools/perf/util/print-events.c
+++ b/tools/perf/util/print-events.c
@@ -83,11 +83,11 @@ void print_tracepoint_events(const struct print_callbacks *print_cb __maybe_unus
 		if (sys_dirent->d_type != DT_DIR ||
 		    !strcmp(sys_dirent->d_name, ".") ||
 		    !strcmp(sys_dirent->d_name, ".."))
-			continue;
+			goto next_sys;
 
 		dir_fd = openat(events_fd, sys_dirent->d_name, O_PATH);
 		if (dir_fd < 0)
-			continue;
+			goto next_sys;
 
 		evt_items = scandirat(events_fd, sys_dirent->d_name, &evt_namelist, NULL, alphasort);
 		for (int j = 0; j < evt_items; j++) {
@@ -98,12 +98,12 @@ void print_tracepoint_events(const struct print_callbacks *print_cb __maybe_unus
 			if (evt_dirent->d_type != DT_DIR ||
 			    !strcmp(evt_dirent->d_name, ".") ||
 			    !strcmp(evt_dirent->d_name, ".."))
-				continue;
+				goto next_evt;
 
 			snprintf(evt_path, sizeof(evt_path), "%s/id", evt_dirent->d_name);
 			evt_fd = openat(dir_fd, evt_path, O_RDONLY);
 			if (evt_fd < 0)
-				continue;
+				goto next_evt;
 			close(evt_fd);
 
 			snprintf(evt_path, MAXPATHLEN, "%s:%s",
@@ -119,9 +119,13 @@ void print_tracepoint_events(const struct print_callbacks *print_cb __maybe_unus
 					/*desc=*/NULL,
 					/*long_desc=*/NULL,
 					/*encoding_desc=*/NULL);
+next_evt:
+			free(evt_namelist[j]);
 		}
 		close(dir_fd);
 		free(evt_namelist);
+next_sys:
+		free(sys_namelist[i]);
 	}
 
 	free(sys_namelist);
-- 
2.40.1.495.gc816e09b53d-goog


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH 2/2] perf list: Modify the warning message about scandirat(3)
  2023-04-27 23:05 [PATCH 1/2] perf list: Fix memory leaks in print_tracepoint_events() Namhyung Kim
@ 2023-04-27 23:05 ` Namhyung Kim
  2023-04-28  0:30   ` Ian Rogers
  2023-04-28  0:27 ` [PATCH 1/2] perf list: Fix memory leaks in print_tracepoint_events() Ian Rogers
  1 sibling, 1 reply; 5+ messages in thread
From: Namhyung Kim @ 2023-04-27 23:05 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo, Jiri Olsa
  Cc: Ian Rogers, Adrian Hunter, Peter Zijlstra, Ingo Molnar, LKML,
	linux-perf-users

It should mention scandirat() instead of scandir().

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/util/print-events.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/util/print-events.c b/tools/perf/util/print-events.c
index 0a97912fd894..299973876550 100644
--- a/tools/perf/util/print-events.c
+++ b/tools/perf/util/print-events.c
@@ -131,7 +131,7 @@ void print_tracepoint_events(const struct print_callbacks *print_cb __maybe_unus
 	free(sys_namelist);
 }
 #else
-	printf("\nWARNING: Your libc doesn't have the scandir function, please ask its maintainers to implement it.\n"
+	printf("\nWARNING: Your libc doesn't have the scandirat function, please ask its maintainers to implement it.\n"
 	       "         As a rough fallback, please do 'ls %s' to see the available tracepoint events.\n", events_path);
 #endif
 	close(events_fd);
-- 
2.40.1.495.gc816e09b53d-goog


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/2] perf list: Fix memory leaks in print_tracepoint_events()
  2023-04-27 23:05 [PATCH 1/2] perf list: Fix memory leaks in print_tracepoint_events() Namhyung Kim
  2023-04-27 23:05 ` [PATCH 2/2] perf list: Modify the warning message about scandirat(3) Namhyung Kim
@ 2023-04-28  0:27 ` Ian Rogers
  2023-04-29  1:30   ` Arnaldo Carvalho de Melo
  1 sibling, 1 reply; 5+ messages in thread
From: Ian Rogers @ 2023-04-28  0:27 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: Arnaldo Carvalho de Melo, Jiri Olsa, Adrian Hunter,
	Peter Zijlstra, Ingo Molnar, LKML, linux-perf-users

On Thu, Apr 27, 2023 at 4:05 PM Namhyung Kim <namhyung@kernel.org> wrote:
>
> It should free entries (not only the array) filled by scandirat()
> after use.
>
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>

Reviewed-by: Ian Rogers <irogers@google.com>

Thanks,
Ian

> ---
>  tools/perf/util/print-events.c | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/tools/perf/util/print-events.c b/tools/perf/util/print-events.c
> index d416c5484cd5..0a97912fd894 100644
> --- a/tools/perf/util/print-events.c
> +++ b/tools/perf/util/print-events.c
> @@ -83,11 +83,11 @@ void print_tracepoint_events(const struct print_callbacks *print_cb __maybe_unus
>                 if (sys_dirent->d_type != DT_DIR ||
>                     !strcmp(sys_dirent->d_name, ".") ||
>                     !strcmp(sys_dirent->d_name, ".."))
> -                       continue;
> +                       goto next_sys;
>
>                 dir_fd = openat(events_fd, sys_dirent->d_name, O_PATH);
>                 if (dir_fd < 0)
> -                       continue;
> +                       goto next_sys;
>
>                 evt_items = scandirat(events_fd, sys_dirent->d_name, &evt_namelist, NULL, alphasort);
>                 for (int j = 0; j < evt_items; j++) {
> @@ -98,12 +98,12 @@ void print_tracepoint_events(const struct print_callbacks *print_cb __maybe_unus
>                         if (evt_dirent->d_type != DT_DIR ||
>                             !strcmp(evt_dirent->d_name, ".") ||
>                             !strcmp(evt_dirent->d_name, ".."))
> -                               continue;
> +                               goto next_evt;
>
>                         snprintf(evt_path, sizeof(evt_path), "%s/id", evt_dirent->d_name);
>                         evt_fd = openat(dir_fd, evt_path, O_RDONLY);
>                         if (evt_fd < 0)
> -                               continue;
> +                               goto next_evt;
>                         close(evt_fd);
>
>                         snprintf(evt_path, MAXPATHLEN, "%s:%s",
> @@ -119,9 +119,13 @@ void print_tracepoint_events(const struct print_callbacks *print_cb __maybe_unus
>                                         /*desc=*/NULL,
>                                         /*long_desc=*/NULL,
>                                         /*encoding_desc=*/NULL);
> +next_evt:
> +                       free(evt_namelist[j]);
>                 }
>                 close(dir_fd);
>                 free(evt_namelist);
> +next_sys:
> +               free(sys_namelist[i]);
>         }
>
>         free(sys_namelist);
> --
> 2.40.1.495.gc816e09b53d-goog
>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 2/2] perf list: Modify the warning message about scandirat(3)
  2023-04-27 23:05 ` [PATCH 2/2] perf list: Modify the warning message about scandirat(3) Namhyung Kim
@ 2023-04-28  0:30   ` Ian Rogers
  0 siblings, 0 replies; 5+ messages in thread
From: Ian Rogers @ 2023-04-28  0:30 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: Arnaldo Carvalho de Melo, Jiri Olsa, Adrian Hunter,
	Peter Zijlstra, Ingo Molnar, LKML, linux-perf-users

On Thu, Apr 27, 2023 at 4:05 PM Namhyung Kim <namhyung@kernel.org> wrote:
>
> It should mention scandirat() instead of scandir().
>
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>

Acked-by: Ian Rogers <irogers@google.com>

Fwiw, tracing_events__scandir_alphasort assumes scandir is present
unconditionally.

Thanks,
Ian

> ---
>  tools/perf/util/print-events.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/perf/util/print-events.c b/tools/perf/util/print-events.c
> index 0a97912fd894..299973876550 100644
> --- a/tools/perf/util/print-events.c
> +++ b/tools/perf/util/print-events.c
> @@ -131,7 +131,7 @@ void print_tracepoint_events(const struct print_callbacks *print_cb __maybe_unus
>         free(sys_namelist);
>  }
>  #else
> -       printf("\nWARNING: Your libc doesn't have the scandir function, please ask its maintainers to implement it.\n"
> +       printf("\nWARNING: Your libc doesn't have the scandirat function, please ask its maintainers to implement it.\n"
>                "         As a rough fallback, please do 'ls %s' to see the available tracepoint events.\n", events_path);
>  #endif
>         close(events_fd);
> --
> 2.40.1.495.gc816e09b53d-goog
>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/2] perf list: Fix memory leaks in print_tracepoint_events()
  2023-04-28  0:27 ` [PATCH 1/2] perf list: Fix memory leaks in print_tracepoint_events() Ian Rogers
@ 2023-04-29  1:30   ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 5+ messages in thread
From: Arnaldo Carvalho de Melo @ 2023-04-29  1:30 UTC (permalink / raw)
  To: Ian Rogers
  Cc: Namhyung Kim, Jiri Olsa, Adrian Hunter, Peter Zijlstra,
	Ingo Molnar, LKML, linux-perf-users

Em Thu, Apr 27, 2023 at 05:27:50PM -0700, Ian Rogers escreveu:
> On Thu, Apr 27, 2023 at 4:05 PM Namhyung Kim <namhyung@kernel.org> wrote:
> >
> > It should free entries (not only the array) filled by scandirat()
> > after use.
> >
> > Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> 
> Reviewed-by: Ian Rogers <irogers@google.com>
 

Thanks, applied the series.

- Arnaldo

> Thanks,
> Ian
> 
> > ---
> >  tools/perf/util/print-events.c | 12 ++++++++----
> >  1 file changed, 8 insertions(+), 4 deletions(-)
> >
> > diff --git a/tools/perf/util/print-events.c b/tools/perf/util/print-events.c
> > index d416c5484cd5..0a97912fd894 100644
> > --- a/tools/perf/util/print-events.c
> > +++ b/tools/perf/util/print-events.c
> > @@ -83,11 +83,11 @@ void print_tracepoint_events(const struct print_callbacks *print_cb __maybe_unus
> >                 if (sys_dirent->d_type != DT_DIR ||
> >                     !strcmp(sys_dirent->d_name, ".") ||
> >                     !strcmp(sys_dirent->d_name, ".."))
> > -                       continue;
> > +                       goto next_sys;
> >
> >                 dir_fd = openat(events_fd, sys_dirent->d_name, O_PATH);
> >                 if (dir_fd < 0)
> > -                       continue;
> > +                       goto next_sys;
> >
> >                 evt_items = scandirat(events_fd, sys_dirent->d_name, &evt_namelist, NULL, alphasort);
> >                 for (int j = 0; j < evt_items; j++) {
> > @@ -98,12 +98,12 @@ void print_tracepoint_events(const struct print_callbacks *print_cb __maybe_unus
> >                         if (evt_dirent->d_type != DT_DIR ||
> >                             !strcmp(evt_dirent->d_name, ".") ||
> >                             !strcmp(evt_dirent->d_name, ".."))
> > -                               continue;
> > +                               goto next_evt;
> >
> >                         snprintf(evt_path, sizeof(evt_path), "%s/id", evt_dirent->d_name);
> >                         evt_fd = openat(dir_fd, evt_path, O_RDONLY);
> >                         if (evt_fd < 0)
> > -                               continue;
> > +                               goto next_evt;
> >                         close(evt_fd);
> >
> >                         snprintf(evt_path, MAXPATHLEN, "%s:%s",
> > @@ -119,9 +119,13 @@ void print_tracepoint_events(const struct print_callbacks *print_cb __maybe_unus
> >                                         /*desc=*/NULL,
> >                                         /*long_desc=*/NULL,
> >                                         /*encoding_desc=*/NULL);
> > +next_evt:
> > +                       free(evt_namelist[j]);
> >                 }
> >                 close(dir_fd);
> >                 free(evt_namelist);
> > +next_sys:
> > +               free(sys_namelist[i]);
> >         }
> >
> >         free(sys_namelist);
> > --
> > 2.40.1.495.gc816e09b53d-goog
> >

-- 

- Arnaldo

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2023-04-29  1:30 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-04-27 23:05 [PATCH 1/2] perf list: Fix memory leaks in print_tracepoint_events() Namhyung Kim
2023-04-27 23:05 ` [PATCH 2/2] perf list: Modify the warning message about scandirat(3) Namhyung Kim
2023-04-28  0:30   ` Ian Rogers
2023-04-28  0:27 ` [PATCH 1/2] perf list: Fix memory leaks in print_tracepoint_events() Ian Rogers
2023-04-29  1:30   ` Arnaldo Carvalho de Melo

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.