On Wed, 3 Feb 2021 18:09:27 +0100
Peter Zijlstra <pet...@infradead.org> wrote:
> On Wed, Feb 03, 2021 at 11:05:31AM -0500, Steven Rostedt wrote:
> > + if (new) {
> > + for (i = 0; old[i].func; i++)
> > + if ((old[i].func != tp_func->func
> > + || old[i].data != tp_func->data)
> > + && old[i].func != tp_stub_func)
>
> logical operators go at the end..
Agreed. I just added that "if (new) {" around the original block, didn't
think about the formatting when doing so.
>
> > + new[j++] = old[i];
> > + new[nr_probes - nr_del].func = NULL;
> > + *funcs = new;
> > + } else {
> > + /*
> > + * Failed to allocate, replace the old function
> > + * with calls to tp_stub_func.
> > + */
> > + for (i = 0; old[i].func; i++)
>
> {
>
> > + if (old[i].func == tp_func->func &&
> > + old[i].data == tp_func->data) {
>
> like here.
>
> > + old[i].func = tp_stub_func;
> > + /* Set the prio to the next event. */
> > + if (old[i + 1].func)
> > + old[i].prio =
> > + old[i + 1].prio;
>
> multi line demands { }, but in this case just don't line-break.
Sure.
>
> > + else
> > + old[i].prio = -1;
> > + }
>
> }
>
> > + *funcs = old;
> > + }
> > }
> > debug_print_probes(*funcs);
> > return old;
> > @@ -295,10 +341,12 @@ static int tracepoint_remove_func(struct tracepoint
> > *tp,
> > tp_funcs = rcu_dereference_protected(tp->funcs,
> > lockdep_is_held(&tracepoints_mutex));
> > old = func_remove(&tp_funcs, func);
> > - if (IS_ERR(old)) {
> > - WARN_ON_ONCE(PTR_ERR(old) != -ENOMEM);
> > + if (WARN_ON_ONCE(IS_ERR(old)))
> > return PTR_ERR(old);
> > - }
> > +
> > + if (tp_funcs == old)
> > + /* Failed allocating new tp_funcs, replaced func with stub */
> > + return 0;
>
> { }
Even if it's just a comment that causes multiple lines? I could just move
the comment above the if.
This has already been through my test suite, and since the changes
requested are just formatting and non-functional, I'll just add a clean up
patch on top.
Thanks!
-- Steve
