On Mon, 21 Oct 2019 22:03:21 +0800 Qu Wenruo <[email protected]> wrote:
> On 2019/10/21 下午9:56, Steven Rostedt wrote: > > On Mon, 21 Oct 2019 17:47:30 +0800 > > Qu Wenruo <[email protected]> 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) > > Isn't this ( 1 <= i && i <= 4 ) easier to find out the lower and upper > boundary? only two numbers, both at the end of the expression. I don't read it like that. > > I feel that ( i >= 1 && i <= 4 ) easier to write, but takes me extra > half second to read, thus I changed to the current one. How do you read it in English? "If one is less than or equal to i and i is less than or equal to four." Or "If i is greater than or equal to one and i is less than or equal to four." ? I read it the second way, and I believe most English speakers read it that way too. It took me a minute or two to understand the original method, because my mind likes to take a variable and keep it on the same side of the comparison, and the variable should always be first. -- Steve
