On Mon, 21 Oct 2019 17:47:30 +0800 Qu Wenruo <wqu@xxxxxxxx> wrote: > +static void print_uuid_arg(struct trace_seq *s, void *data, int size, > + struct tep_event *event, struct tep_print_arg *arg) > +{ > + unsigned char *buf; > + int i; > + > + if (arg->type != TEP_PRINT_FIELD) { > + trace_seq_printf(s, "ARG TYPE NOT FIELID but %d", arg->type); > + return; > + } > + > + if (!arg->field.field) { > + arg->field.field = tep_find_any_field(event, arg->field.name); > + if (!arg->field.field) { > + do_warning("%s: field %s not found", > + __func__, arg->field.name); > + return; > + } > + } > + if (arg->field.field->size < 16) { > + trace_seq_printf(s, "INVALID UUID: size have %u expect 16", > + arg->field.field->size); > + return; > + } > + buf = data + arg->field.field->offset; > + > + for (i = 0; i < 8; i++) { > + trace_seq_printf(s, "%02x", buf[2 * i]); > + trace_seq_printf(s, "%02x", buf[2 * i + 1]); > + if (1 <= i && i <= 4) I'm fine with this patch except for one nit. The above is hard to read (in my opinion), and I absolutely hate the "constant" compare to "variable" notation. Please change the above to: if (i >= 1 && i <= 4) Thanks, -- Steve > + trace_seq_putc(s, '-'); > + } > +} > +