public inbox for dtrace@lists.linux.dev
 help / color / mirror / Atom feed
* [PATCH] Fix memory leaks
@ 2024-11-21  6:40 Kris Van Hees
  2024-11-21 18:10 ` Eugene Loh
  0 siblings, 1 reply; 3+ messages in thread
From: Kris Van Hees @ 2024-11-21  6:40 UTC (permalink / raw)
  To: dtrace, dtrace-devel

The array of statements was never freed.
The PFM library data was never cleaned up.

Signed-off-by: Kris Van Hees <kris.van.hees@oracle.com>
---
 libdtrace/dt_open.c     | 2 ++
 libdtrace/dt_prov_cpc.c | 1 +
 2 files changed, 3 insertions(+)

diff --git a/libdtrace/dt_open.c b/libdtrace/dt_open.c
index e1972aa8..717a7ad0 100644
--- a/libdtrace/dt_open.c
+++ b/libdtrace/dt_open.c
@@ -1314,6 +1314,8 @@ dtrace_close(dtrace_hdl_t *dtp)
 		free(dirp);
 	}
 
+	free(dtp->dt_stmts);
+
 	free(dtp->dt_cpp_argv);
 	free(dtp->dt_cpp_path);
 	free(dtp->dt_ld_path);
diff --git a/libdtrace/dt_prov_cpc.c b/libdtrace/dt_prov_cpc.c
index 8f33cf58..57b11b13 100644
--- a/libdtrace/dt_prov_cpc.c
+++ b/libdtrace/dt_prov_cpc.c
@@ -484,6 +484,7 @@ static void destroy(dtrace_hdl_t *dtp, void *arg)
 		dt_free(dtp, probe_map);
 	}
 	dt_free(dtp, arg);
+	pfm_terminate();
 }
 
 dt_provimpl_t	dt_cpc = {
-- 
2.43.5


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

* Re: [PATCH] Fix memory leaks
  2024-11-21  6:40 [PATCH] Fix memory leaks Kris Van Hees
@ 2024-11-21 18:10 ` Eugene Loh
  2024-11-21 18:17   ` [DTrace-devel] " Kris Van Hees
  0 siblings, 1 reply; 3+ messages in thread
From: Eugene Loh @ 2024-11-21 18:10 UTC (permalink / raw)
  To: dtrace, dtrace-devel

Reviewed-by: Eugene Loh <eugene.loh@oracle.com>

(Oops by me.)

I don't have a good feel for these things, but in dtrace_close() might 
it make more sense to move the free(dt_stmts) to right after the 
dt_program_destroy() loop?

With this patch, it also makes sense to remove the "CPC will call 
pfm_terminate()" comment in dtrace_close()

On 11/21/24 01:40, Kris Van Hees wrote:
> The array of statements was never freed.
> The PFM library data was never cleaned up.
>
> Signed-off-by: Kris Van Hees <kris.van.hees@oracle.com>
> ---
>   libdtrace/dt_open.c     | 2 ++
>   libdtrace/dt_prov_cpc.c | 1 +
>   2 files changed, 3 insertions(+)
>
> diff --git a/libdtrace/dt_open.c b/libdtrace/dt_open.c
> index e1972aa8..717a7ad0 100644
> --- a/libdtrace/dt_open.c
> +++ b/libdtrace/dt_open.c
> @@ -1314,6 +1314,8 @@ dtrace_close(dtrace_hdl_t *dtp)
>   		free(dirp);
>   	}
>   
> +	free(dtp->dt_stmts);
> +
>   	free(dtp->dt_cpp_argv);
>   	free(dtp->dt_cpp_path);
>   	free(dtp->dt_ld_path);
> diff --git a/libdtrace/dt_prov_cpc.c b/libdtrace/dt_prov_cpc.c
> index 8f33cf58..57b11b13 100644
> --- a/libdtrace/dt_prov_cpc.c
> +++ b/libdtrace/dt_prov_cpc.c
> @@ -484,6 +484,7 @@ static void destroy(dtrace_hdl_t *dtp, void *arg)
>   		dt_free(dtp, probe_map);
>   	}
>   	dt_free(dtp, arg);
> +	pfm_terminate();
>   }
>   
>   dt_provimpl_t	dt_cpc = {

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

* Re: [DTrace-devel] [PATCH] Fix memory leaks
  2024-11-21 18:10 ` Eugene Loh
@ 2024-11-21 18:17   ` Kris Van Hees
  0 siblings, 0 replies; 3+ messages in thread
From: Kris Van Hees @ 2024-11-21 18:17 UTC (permalink / raw)
  To: Eugene Loh; +Cc: dtrace, dtrace-devel

On Thu, Nov 21, 2024 at 01:10:29PM -0500, Eugene Loh via DTrace-devel wrote:
> Reviewed-by: Eugene Loh <eugene.loh@oracle.com>
> 
> (Oops by me.)
> 
> I don't have a good feel for these things, but in dtrace_close() might it
> make more sense to move the free(dt_stmts) to right after the
> dt_program_destroy() loop?

Sure.  Sounds like a good idea.

> With this patch, it also makes sense to remove the "CPC will call
> pfm_terminate()" comment in dtrace_close()

Ah yes, I overlooked that one.  Thanks!

> On 11/21/24 01:40, Kris Van Hees wrote:
> > The array of statements was never freed.
> > The PFM library data was never cleaned up.
> > 
> > Signed-off-by: Kris Van Hees <kris.van.hees@oracle.com>
> > ---
> >   libdtrace/dt_open.c     | 2 ++
> >   libdtrace/dt_prov_cpc.c | 1 +
> >   2 files changed, 3 insertions(+)
> > 
> > diff --git a/libdtrace/dt_open.c b/libdtrace/dt_open.c
> > index e1972aa8..717a7ad0 100644
> > --- a/libdtrace/dt_open.c
> > +++ b/libdtrace/dt_open.c
> > @@ -1314,6 +1314,8 @@ dtrace_close(dtrace_hdl_t *dtp)
> >   		free(dirp);
> >   	}
> > +	free(dtp->dt_stmts);
> > +
> >   	free(dtp->dt_cpp_argv);
> >   	free(dtp->dt_cpp_path);
> >   	free(dtp->dt_ld_path);
> > diff --git a/libdtrace/dt_prov_cpc.c b/libdtrace/dt_prov_cpc.c
> > index 8f33cf58..57b11b13 100644
> > --- a/libdtrace/dt_prov_cpc.c
> > +++ b/libdtrace/dt_prov_cpc.c
> > @@ -484,6 +484,7 @@ static void destroy(dtrace_hdl_t *dtp, void *arg)
> >   		dt_free(dtp, probe_map);
> >   	}
> >   	dt_free(dtp, arg);
> > +	pfm_terminate();
> >   }
> >   dt_provimpl_t	dt_cpc = {
> 
> _______________________________________________
> DTrace-devel mailing list
> DTrace-devel@oss.oracle.com
> https://oss.oracle.com/mailman/listinfo/dtrace-devel

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

end of thread, other threads:[~2024-11-21 18:17 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-11-21  6:40 [PATCH] Fix memory leaks Kris Van Hees
2024-11-21 18:10 ` Eugene Loh
2024-11-21 18:17   ` [DTrace-devel] " Kris Van Hees

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox