On Mon, Jan 08, 2024 at 01:37:23PM -0500, Steven Rostedt wrote: > From: "Steven Rostedt (Google)" <[email protected]> > > Instead of having a bunch of if statements with: > > len = str_has_prefix(field, "__data_loc unsigned "); > if (len) > goto skip_next; > > len = str_has_prefix(field, "__data_loc "); > if (len) > goto skip_next; > > len = str_has_prefix(field, "__rel_loc unsigned "); > if (len) > goto skip_next; > > len = str_has_prefix(field, "__rel_loc "); > if (len) > goto skip_next; > > goto parse; > > skip_next: > > Consolidate it into a negative check and jump to parse if all the > str_has_prefix() calls fail. If one succeeds, it will just continue with > len equal to the proper string: > > if (!(len = str_has_prefix(field, "__data_loc unsigned ")) && > !(len = str_has_prefix(field, "__data_loc ")) && > !(len = str_has_prefix(field, "__rel_loc unsigned ")) && > !(len = str_has_prefix(field, "__rel_loc "))) { > goto parse; > } > > skip_next: > > Signed-off-by: Steven Rostedt (Google) <[email protected]> > --- > kernel/trace/trace_events_user.c | 22 ++++++---------------- > 1 file changed, 6 insertions(+), 16 deletions(-) > > diff --git a/kernel/trace/trace_events_user.c > b/kernel/trace/trace_events_user.c > index 9365ce407426..ce0c5f1ded48 100644 > --- a/kernel/trace/trace_events_user.c > +++ b/kernel/trace/trace_events_user.c > @@ -1175,23 +1175,13 @@ static int user_event_parse_field(char *field, struct > user_event *user, > goto skip_next; > } > > - len = str_has_prefix(field, "__data_loc unsigned "); > - if (len) > - goto skip_next; > - > - len = str_has_prefix(field, "__data_loc "); > - if (len) > - goto skip_next; > - > - len = str_has_prefix(field, "__rel_loc unsigned "); > - if (len) > - goto skip_next; > - > - len = str_has_prefix(field, "__rel_loc "); > - if (len) > - goto skip_next; > + if (!(len = str_has_prefix(field, "__data_loc unsigned ")) && > + !(len = str_has_prefix(field, "__data_loc ")) && > + !(len = str_has_prefix(field, "__rel_loc unsigned ")) && > + !(len = str_has_prefix(field, "__rel_loc "))) { > + goto parse; > + }
This now triggers a checkpatch error: ERROR: do not use assignment in if condition #1184: FILE: kernel/trace/trace_events_user.c:1184: + if (!(len = str_has_prefix(field, "__data_loc unsigned ")) && I personally prefer to keep these files fully checkpatch clean. However, I did test these changes under the self-tests and it passed. Do they bug you that much? :) Thanks, -Beau > > - goto parse; > skip_next: > type = field; > field = strpbrk(field + len, " "); > -- > 2.43.0
