Re: [PATCH 2/3] kernel-shark: Fix a memory leak in the sched_events plugin

[Steven Rostedt <rostedt@goodmis.org>]

On Tue, 5 Mar 2019 17:56:28 +0200
Slavomir Kaslev <kaslevs@vmware.com> wrote:

> On Tue, Mar 05, 2019 at 04:39:23PM +0200, Yordan Karadzhov wrote:
> > When the sched_events plugin fails to initialize not all the
> > memory allocated for the context of the plugin is freed properly.
> > The problem gets fixed by using the free_context helper function
> > defined in the previous patch.
> > 
> > Fixes: 4f392730e ("kernel-shark-qt: Make Sched event plugin use ...")
> > Signed-off-by: Yordan Karadzhov <ykaradzhov@vmware.com>
> > ---
> >  kernel-shark/src/plugins/sched_events.c | 11 ++++++-----
> >  1 file changed, 6 insertions(+), 5 deletions(-)
> > 
> > diff --git a/kernel-shark/src/plugins/sched_events.c b/kernel-shark/src/plugins/sched_events.c
> > index 0d6de2d..fe13e6a 100644
> > --- a/kernel-shark/src/plugins/sched_events.c
> > +++ b/kernel-shark/src/plugins/sched_events.c
> > @@ -74,8 +74,12 @@ static bool plugin_sched_init_context(struct kshark_context *kshark_ctx)
> >  
> >  	event = tep_find_event_by_name(plugin_ctx->pevent,
> >  				       "sched", "sched_switch");
> > -	if (!event)
> > +	if (!event) {
> > +		plugin_free_context(plugin_ctx);
> > +		plugin_sched_context_handler = NULL;
> > +  
> 
> Nit: drop the above empty line?

Yeah, it would be better to do that. But for now, I'll take the patch
anyway. Perhaps clean it up later.

> 
> >  		return false;
> > +	}
> >  
> >  	plugin_ctx->sched_switch_event = event;
> >  	plugin_ctx->sched_switch_next_field =
> > @@ -320,11 +324,8 @@ static int plugin_sched_init(struct kshark_context *kshark_ctx)
> >  {
> >  	struct plugin_sched_context *plugin_ctx;
> >  
> > -	if (!plugin_sched_init_context(kshark_ctx)) {
> > -		free(plugin_sched_context_handler);
> > -		plugin_sched_context_handler = NULL;
> > +	if (!plugin_sched_init_context(kshark_ctx))
> >  		return 0;
> > -	}
> >  
> >  	plugin_ctx = plugin_sched_context_handler;  
> 
> Looks good to me.
> 
> Reviewed-by: Slavomir Kaslev <kaslevs@vmware.com>

Thanks Slavomir!

-- Steve