* [PATCH] perf lock: Fix a memory leak on an error path
@ 2023-11-24 9:26 zhaimingbing
2023-11-24 9:53 ` Peter Zijlstra
0 siblings, 1 reply; 4+ messages in thread
From: zhaimingbing @ 2023-11-24 9:26 UTC (permalink / raw)
To: Namhyung Kim, Peter Zijlstra, Ingo Molnar,
Arnaldo Carvalho de Melo, Mark Rutland, Alexander Shishkin,
Jiri Olsa, Ian Rogers, Adrian Hunter, Sean Christopherson,
Li Dong
Cc: linux-perf-users, linux-kernel, zhaimingbing
if a strdup-ed string is NULL,the allocated memory needs freeing.
Signed-off-by: zhaimingbing <zhaimingbing@cmss.chinamobile.com>
---
tools/perf/builtin-lock.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/tools/perf/builtin-lock.c b/tools/perf/builtin-lock.c
index b141f2134..086041bcb 100644
--- a/tools/perf/builtin-lock.c
+++ b/tools/perf/builtin-lock.c
@@ -2228,8 +2228,10 @@ static int __cmd_record(int argc, const char **argv)
else
ev_name = strdup(contention_tracepoints[j].name);
- if (!ev_name)
+ if (!ev_name) {
+ free(rec_argv);
return -ENOMEM;
+ }
rec_argv[i++] = "-e";
rec_argv[i++] = ev_name;
--
2.33.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] perf lock: Fix a memory leak on an error path
2023-11-24 9:26 [PATCH] perf lock: Fix a memory leak on an error path zhaimingbing
@ 2023-11-24 9:53 ` Peter Zijlstra
2023-11-24 12:56 ` Ingo Molnar
0 siblings, 1 reply; 4+ messages in thread
From: Peter Zijlstra @ 2023-11-24 9:53 UTC (permalink / raw)
To: zhaimingbing
Cc: Namhyung Kim, Ingo Molnar, Arnaldo Carvalho de Melo,
Mark Rutland, Alexander Shishkin, Jiri Olsa, Ian Rogers,
Adrian Hunter, Sean Christopherson, Li Dong, linux-perf-users,
linux-kernel
On Fri, Nov 24, 2023 at 05:26:57PM +0800, zhaimingbing wrote:
> if a strdup-ed string is NULL,the allocated memory needs freeing.
>
> Signed-off-by: zhaimingbing <zhaimingbing@cmss.chinamobile.com>
> ---
> tools/perf/builtin-lock.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/tools/perf/builtin-lock.c b/tools/perf/builtin-lock.c
> index b141f2134..086041bcb 100644
> --- a/tools/perf/builtin-lock.c
> +++ b/tools/perf/builtin-lock.c
> @@ -2228,8 +2228,10 @@ static int __cmd_record(int argc, const char **argv)
> else
> ev_name = strdup(contention_tracepoints[j].name);
>
> - if (!ev_name)
> + if (!ev_name) {
> + free(rec_argv);
> return -ENOMEM;
> + }
Isn't this an error path straight into exit?
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] perf lock: Fix a memory leak on an error path
2023-11-24 9:53 ` Peter Zijlstra
@ 2023-11-24 12:56 ` Ingo Molnar
2023-11-27 13:18 ` Arnaldo Carvalho de Melo
0 siblings, 1 reply; 4+ messages in thread
From: Ingo Molnar @ 2023-11-24 12:56 UTC (permalink / raw)
To: Peter Zijlstra
Cc: zhaimingbing, Namhyung Kim, Ingo Molnar,
Arnaldo Carvalho de Melo, Mark Rutland, Alexander Shishkin,
Jiri Olsa, Ian Rogers, Adrian Hunter, Sean Christopherson,
Li Dong, linux-perf-users, linux-kernel
* Peter Zijlstra <peterz@infradead.org> wrote:
> On Fri, Nov 24, 2023 at 05:26:57PM +0800, zhaimingbing wrote:
> > if a strdup-ed string is NULL,the allocated memory needs freeing.
> >
> > Signed-off-by: zhaimingbing <zhaimingbing@cmss.chinamobile.com>
> > ---
> > tools/perf/builtin-lock.c | 4 +++-
> > 1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/tools/perf/builtin-lock.c b/tools/perf/builtin-lock.c
> > index b141f2134..086041bcb 100644
> > --- a/tools/perf/builtin-lock.c
> > +++ b/tools/perf/builtin-lock.c
> > @@ -2228,8 +2228,10 @@ static int __cmd_record(int argc, const char **argv)
> > else
> > ev_name = strdup(contention_tracepoints[j].name);
> >
> > - if (!ev_name)
> > + if (!ev_name) {
> > + free(rec_argv);
> > return -ENOMEM;
> > + }
>
> Isn't this an error path straight into exit?
It increases the quality of implementation if resources are free()d
consistently regardless of whether the task is going to exit() immediately,
for example it makes it easier to validate perf for the lack of memory
leaks with Valgrind.
Thanks,
Ingo
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] perf lock: Fix a memory leak on an error path
2023-11-24 12:56 ` Ingo Molnar
@ 2023-11-27 13:18 ` Arnaldo Carvalho de Melo
0 siblings, 0 replies; 4+ messages in thread
From: Arnaldo Carvalho de Melo @ 2023-11-27 13:18 UTC (permalink / raw)
To: Ingo Molnar
Cc: Peter Zijlstra, zhaimingbing, Namhyung Kim, Ingo Molnar,
Mark Rutland, Alexander Shishkin, Jiri Olsa, Ian Rogers,
Adrian Hunter, Sean Christopherson, Li Dong, linux-perf-users,
linux-kernel
Em Fri, Nov 24, 2023 at 01:56:35PM +0100, Ingo Molnar escreveu:
>
> * Peter Zijlstra <peterz@infradead.org> wrote:
>
> > On Fri, Nov 24, 2023 at 05:26:57PM +0800, zhaimingbing wrote:
> > > if a strdup-ed string is NULL,the allocated memory needs freeing.
> > >
> > > Signed-off-by: zhaimingbing <zhaimingbing@cmss.chinamobile.com>
> > > ---
> > > tools/perf/builtin-lock.c | 4 +++-
> > > 1 file changed, 3 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/tools/perf/builtin-lock.c b/tools/perf/builtin-lock.c
> > > index b141f2134..086041bcb 100644
> > > --- a/tools/perf/builtin-lock.c
> > > +++ b/tools/perf/builtin-lock.c
> > > @@ -2228,8 +2228,10 @@ static int __cmd_record(int argc, const char **argv)
> > > else
> > > ev_name = strdup(contention_tracepoints[j].name);
> > >
> > > - if (!ev_name)
> > > + if (!ev_name) {
> > > + free(rec_argv);
> > > return -ENOMEM;
> > > + }
> >
> > Isn't this an error path straight into exit?
>
> It increases the quality of implementation if resources are free()d
> consistently regardless of whether the task is going to exit() immediately,
> for example it makes it easier to validate perf for the lack of memory
> leaks with Valgrind.
I'm taking this positive comment about the patch as an Acked-by, ok?
- Arnaldo
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2023-11-27 13:18 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-11-24 9:26 [PATCH] perf lock: Fix a memory leak on an error path zhaimingbing
2023-11-24 9:53 ` Peter Zijlstra
2023-11-24 12:56 ` Ingo Molnar
2023-11-27 13:18 ` 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.