On Tue, 29 Sep 2020 15:01:59 -0700 Axel Rasmussen <axelrasmus...@google.com> wrote:
> > event->fields[i]->offset = n_u64; > > > > - if (event->fields[i]->is_string) { > > + if (event->fields[i]->is_string && > > !event->fields[i]->is_dynamic) { > > offset += STR_VAR_LEN_MAX; > > n_u64 += STR_VAR_LEN_MAX / sizeof(u64); > > } else { > > @@ -139,6 +139,9 @@ static int synth_field_string_size(char *type) > > if (len > 3) > > return -EINVAL; > > > > + if (len == 0) > > + return 0; /* variable-length string */ > > + > > strncpy(buf, start, len); > > buf[len] = '\0'; > > > > @@ -290,10 +293,25 @@ static enum print_line_t print_synth_event(struct > > trace_iterator *iter, > > > > /* parameter values */ > > if (se->fields[i]->is_string) { > > - trace_seq_printf(s, print_fmt, se->fields[i]->name, > > - (char *)&entry->fields[n_u64], > > - i == se->n_fields - 1 ? "" : " "); > > - n_u64 += STR_VAR_LEN_MAX / sizeof(u64); > > + if (se->fields[i]->is_dynamic) { > > + u32 offset, data_offset; > > + char *str_field; > > + > > + offset = (u32)entry->fields[n_u64]; > > + data_offset = offset & 0xffff; > > + > > + str_field = (char *)entry + data_offset; > > Is it better to re-use __get_str from include/trace/trace_events.h > instead of writing this out directly? > > > + > > + trace_seq_printf(s, print_fmt, > > se->fields[i]->name, > > + str_field, > > + i == se->n_fields - 1 ? "" > > : " "); > > + n_u64++; > > + } else { > > + trace_seq_printf(s, print_fmt, > > se->fields[i]->name, > > + (char > > *)&entry->fields[n_u64], > > + i == se->n_fields - 1 ? "" > > : " "); > > + n_u64 += STR_VAR_LEN_MAX / sizeof(u64); > > + } > > } else { > > struct trace_print_flags __flags[] = { > > __def_gfpflag_names, {-1, NULL} }; > > @@ -325,16 +343,52 @@ static struct trace_event_functions synth_event_funcs > > = { > > .trace = print_synth_event > > }; > > > > +static unsigned int trace_string(struct synth_trace_event *entry, > > + struct synth_event *event, > > + char *str_val, > > + bool is_dynamic, > > + unsigned int data_size, > > + unsigned int *n_u64) > > +{ > > + unsigned int len = 0; > > + char *str_field; > > + > > + if (is_dynamic) { > > + u32 data_offset; > > + > > + data_offset = offsetof(typeof(*entry), fields); > > + data_offset += event->n_u64 * sizeof(u64); > > + data_offset += data_size; > > + > > + str_field = (char *)entry + data_offset; > > + > > + len = strlen(str_val) + 1; > > + strscpy(str_field, str_val, len); > > + > > + data_offset |= len << 16; > > + *(u32 *)&entry->fields[*n_u64] = data_offset; > > Similar thing here, is it possible to reuse __dynamic_array or __string? Interesting idea. I'd prefer to keep it broken out for this patch set, and then we could look at incorporating this file to use the macros of <trace/trace_event.h>. But I much rather have that be a separate patch that does that as an enhancement than to include it in this file doing more work. -- Steve > > > + > > + (*n_u64)++; > > + } else { > > + str_field = (char *)&entry->fields[*n_u64]; > > + > > + strscpy(str_field, str_val, STR_VAR_LEN_MAX); > > + (*n_u64) += STR_VAR_LEN_MAX / sizeof(u64); > > + } > > + > > + return len; > > +} > > + > > static n