On Mon, 21 Oct 2019 17:47:30 +0800
Qu Wenruo <w...@suse.com> 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, '-');
> + }
> +}
> +
