Re: [RFC PATCH] libtracefs: New APIs for getting the raw format of synth event

[Yordan Karadzhov <y.karadz@xxxxxxxxx>]

On 8.12.21 г. 0:13 ч., Steven Rostedt wrote:
> >   int tracefs_synth_create(struct tracefs_synth *synth)
> >   {
> > -	char *start_hist = NULL;
> > -	char *end_hist = NULL;
> >   	int ret;
> >   
> >   	if (!synth) {
> > @@ -1996,40 +2067,35 @@ int tracefs_synth_create(struct tracefs_synth *synth)
> >   	if (tracefs_dynevent_create(synth->dyn_event))
> >   		return -1;
> >   
> > -	start_hist = create_hist(synth->start_keys, synth->start_vars);
> > -	start_hist = append_filter(start_hist, synth->start_filter,
> > -				   synth->start_parens);
> > -	if (!start_hist)
> > +	synth->start_hist = create_hist(synth->start_keys, synth->start_vars);
> > +	synth->start_hist = append_filter(synth->start_hist, synth->start_filter,
> > +					  synth->start_parens);
> > +	if (!synth->start_hist)
> >   		goto remove_synthetic;
> >   
> > -	end_hist = create_end_hist(synth);
> > -	end_hist = append_filter(end_hist, synth->end_filter,
> > -				   synth->end_parens);
> > -	if (!end_hist)
> > +	synth->end_hist = create_end_hist(synth);
> > +	synth->end_hist = append_filter(synth->end_hist, synth->end_filter,
> > +					synth->end_parens);
> > +	if (!synth->end_hist)
> >   		goto remove_synthetic;
> >   
> >   	ret = tracefs_event_file_append(synth->instance, synth->start_event->system,
> >   					synth->start_event->name,
> > -					"trigger", start_hist);
> > +					"trigger", synth->start_hist);
> >   	if (ret < 0)
> >   		goto remove_synthetic;
> >   
> >   	ret = tracefs_event_file_append(synth->instance, synth->end_event->system,
> >   					synth->end_event->name,
> > -					"trigger", end_hist);
> > +					"trigger", synth->end_hist);
> >   	if (ret < 0)
> >   		goto remove_start_hist;
> >   
> > -	free(start_hist);
> > -	free(end_hist);
> > -
> >   	return 0;
> >   
> >    remove_start_hist:
> > -	remove_hist(synth->instance, synth->start_event, start_hist);
> > +	remove_hist(synth->instance, synth->start_event, synth->start_hist);
> >    remove_synthetic:
> > -	free(end_hist);
> > -	free(start_hist);
> >   	tracefs_dynevent_destroy(synth->dyn_event, false);
> I just realized that on error, we do not free the synthetic event
> descriptor. So there is a memory leak.

Do we need to free the synthetic event here?

The event gets allocated with 'tracefs_synth_alloc()' and the user is responsible for calling 'tracefs_synth_free()' in 
order to free the memory.
The fact that 'tracefs_synth_create()' may fail doesn't change this.

Thanks!
Yordan



> Also, I'm thinking that the specific functions for destroying the synthetic
> events and such, should live here, and trace-dynevents.c should call them.
>
> Because all the management for synthetic events should live in this file
> (or go to its own file).
>
> -- Steve