Hi Steve, On Wed, 2020-01-29 at 16:09 -0500, Steven Rostedt wrote: > On Wed, 29 Jan 2020 12:59:27 -0600 > Tom Zanussi <zanussi@xxxxxxxxxx> wrote: > > > +static struct synth_field *find_synth_field(struct synth_event > > *event, > > + const char > > *field_name) > > +{ > > + struct synth_field *field = NULL; > > + unsigned int i; > > + > > + for (i = 0; i < event->n_fields; i++) { > > + field = event->fields[i]; > > + if (strcmp(field->name, field_name) == 0) > > + return field; > > + } > > + > > + return NULL; > > +} > > Why duplicate all theses functions and not use them in the > synth_event_trace() directly? > Yes, find_synth_field() is used only once and is short, so I can just add that into synth_event_add_val() directly. And looking at synth_event_add_val() and synth_event_add_next_val(), they're almost identical and so can be made into a single function with a param for the different parts (but still need to be exported separately so they can be used with the piecewise API). It would also be possible to have synth_event_trace() and synth_event_trace_array() use synth_event_add_next_val() instead of writing the fields directly but that would be more overhead for those functions, which is why I avoided doing that. Let me know if it's something else you're referring to, or if you want me to do a v5 or a follow-on patch to do the first part above. Thanks, Tom > -- Steve