Messages in this thread | | | Date | Fri, 22 Apr 2022 18:59:22 -0400 | From | Steven Rostedt <> | Subject | Re: [PATCH v2 1/4] tracing: Remove usage of list iterator after the loop body |
| |
On Sat, 2 Apr 2022 12:33:38 +0200 Jakob Koschel <jakobkoschel@gmail.com> wrote:
> In preparation to limit the scope of the list iterator variable to the > traversal loop, use a dedicated pointer to point to the found element > [1]. > > Before, the code implicitly used the head when no element was found > when using &pos->list. Since the new variable is only set if an > element was found, the head needs to be used explicitly if the > variable is NULL. > > Link: https://lore.kernel.org/all/CAHk-=wgRr_D8CB-D9Kg-c=EHreAsk5SqXPwr9Y7k9sA6cWXJ6w@mail.gmail.com/ [1] > Signed-off-by: Jakob Koschel <jakobkoschel@gmail.com> > --- > kernel/trace/trace_output.c | 13 +++++++++---- > 1 file changed, 9 insertions(+), 4 deletions(-) > > diff --git a/kernel/trace/trace_output.c b/kernel/trace/trace_output.c > index 8aa493d25c73..733a4d6c20e2 100644 > --- a/kernel/trace/trace_output.c > +++ b/kernel/trace/trace_output.c > @@ -692,7 +692,7 @@ static LIST_HEAD(ftrace_event_list); > > static int trace_search_list(struct list_head **list) > { > - struct trace_event *e; > + struct trace_event *e = NULL, *iter; > int next = __TRACE_LAST_TYPE; > > if (list_empty(&ftrace_event_list)) { > @@ -704,9 +704,11 @@ static int trace_search_list(struct list_head **list) > * We used up all possible max events, > * lets see if somebody freed one. > */ > - list_for_each_entry(e, &ftrace_event_list, list) { > - if (e->type != next) > + list_for_each_entry(iter, &ftrace_event_list, list) { > + if (iter->type != next) { > + e = iter; > break; > + } > next++; > } > > @@ -714,7 +716,10 @@ static int trace_search_list(struct list_head **list) > if (next > TRACE_EVENT_TYPE_MAX) > return 0; > > - *list = &e->list; > + if (e)
Technically, if e was NULL, then next would be too big. But as that's very subtle with the algorithm, I'm fine adding this.
-- Steve
> + *list = &e->list; > + else > + *list = &ftrace_event_list; > return next; > } >
| |