Re: [PATCH v6 5/5] libtraceevent: Add tep_print_selected_fields()

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

On 20.08.21 г. 17:35, Steven Rostedt wrote:
> On Fri, 20 Aug 2021 16:36:12 +0300
> Yordan Karadzhov <y.karadz@gmail.com> wrote:
> 		if (field_mask & ignore_mask)
>>>> +			continue;
>>>> +
>>>>    		trace_seq_printf(s, " %s=", field->name);
>>>>    		_tep_print_field(s, data, field, &parse);
>>>> -		field = field->next;
>>>>    	}
>>>>    }
>>>>    
>>>> +void tep_print_selected_fields(struct trace_seq *s, void *data,
>>>
>>> As the above is an API, it needs a kernel doc type comment, and also an
>>> addition to the man pages. The man page may be a separate patch.
>>>    
>>
>> I was thinking of maybe changing the second argument of the function to
>>
>> void tep_print_selected_fields(struct trace_seq *s,
>> 			       struct tep_record *record,
>> 			       struct tep_event *event,
>> 			       unsigned long long ignore_mask)
>>
>>
>>> -- Steve
>>>
>>>    
>>>> +			       struct tep_event *event,
>>>> +			       unsigned long long ignore_mask)
>>>> +{
>>>> +	print_selected_fields(s, data, event, ignore_mask);
>>
>> respectively here we will have
>>
>> 	print_selected_fields(s, record->data, event, ignore_mask);
>>>> +}
>>
>> This way the call will look cleaner.
>>
>> 	tep_print_selected_fields(s, record, event, mask);
>>
>> instead of
>>
>> 	tep_print_selected_fields(s, record->data, event, mask);
>>
>> But on the other hand, this will make the new API inconsistent with the existing
>> "tep_print_fields()" API bellow.
> 
> I think we rushed the libtraceevent APIs :-(
> 
> There's a lot of them I hate, and I agree, passing record would have made more sense.
> 
> I've been thinking of reworking a lot of them, but we need to add new APIs.
> 
> tep_print_record_fields()
> tep_print_record_selected_fields()


OK I can add those two.
Thanks!
Y.


> 
> ??
> 
> -- Steve
>