On Thu, 20 Sep 2018 16:29:00 +0300
"Yordan Karadzhov (VMware)" <y.karadz@xxxxxxxxx> wrote:
> On 20.09.2018 06:24, Steven Rostedt wrote:
> >> static size_t get_records(struct kshark_context *kshark_ctx,
> >> struct rec_list ***rec_list, enum rec_type type)
> >> {
> >> + struct kshark_event_handler *evt_handler;
> >> struct event_filter *adv_filter;
> >> struct kshark_task_list *task;
> >> struct tep_record *rec;
> >> @@ -608,6 +618,17 @@ static size_t get_records(struct kshark_context *kshark_ctx,
> >>
> >> entry = &temp_rec->entry;
> >> kshark_set_entry_values(kshark_ctx, rec, entry);
> >> +
> >> + /* Execute all plugin-provided actions (if any). */
> >> + evt_handler = kshark_ctx->event_handlers;
> >> + while ((evt_handler = find_event_handler(evt_handler,
> >> + entry->event_id))) {
> >> + evt_handler->event_func(kshark_ctx, rec, entry);
> >> +
> >> + if ((evt_handler = evt_handler->next))
> > Please break the above if, to remove the assignment. It's just easier
> > to read. That is:
> >
> > event_handler = event_hanndler->next;
> > if (event_handler)
> >> + entry->visible &= ~KS_PLUGIN_UNTOUCHED_MASK;
> >> + }
> >> +
>
> Actually this is a bug (typo). I meant to have
:-)
That's actually one of the reasons the combination is looked down upon
in the Linux kernel. It's more likely to hide bugs from reviewers.
-- Steve
>
> evt_handler = evt_handler->next;
> if (!evt_handler)
> entry->visible &= ~KS_PLUGIN_UNTOUCHED_MASK;
>
> This way the KS_PLUGIN_UNTOUCHED flag gets unset only once and it is
> guaranteed that the value of the bit cannot be changed by the plugin
> callback function.
>
|