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

[Yordan Karadzhov <y.karadz@gmail.com>]

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
> 
>