Messages in this thread | | | From | Will Hawkins <> | Date | Fri, 13 Oct 2017 14:32:26 -0400 | Subject | Re: [PATCH] trace-cmd record: Fix syntax in make_pid_filter |
| |
On Fri, Oct 13, 2017 at 2:24 PM, Steven Rostedt <rostedt@goodmis.org> wrote: > > On Fri, 13 Oct 2017 13:57:12 -0400 > Will Hawkins <hawkinsw@borlaugic.com> wrote: > > > In older versions of the kernel, event filtering by > > pid does not use the set_event_pid file. Instead, > > ftrace uses a common filter (created in make_pid_filter) > > to filter events based on PID. > > > > The syntax of the filter generated by this function is > > overly permissive. When filtering by pid, || is used > > where && should be (in certain cases) which means that > > unrelated events are captured. > > > > Thanks for sending this. > > > Signed-off-by: Will Hawkins <hawkinsw@borlaugic.com> > > --- > > trace-record.c | 17 +++++++++++------ > > 1 file changed, 11 insertions(+), 6 deletions(-) > > > > diff --git a/trace-record.c b/trace-record.c > > index a0b6541..d9c2959 100644 > > --- a/trace-record.c > > +++ b/trace-record.c > > @@ -935,14 +935,19 @@ static char *make_pid_filter(char *curr_filter, const char *field) > > str = filter + curr_len; > > > > for (p = filter_pids; p; p = p->next) { > > - if (p == filter_pids) > > - orit = ""; > > - else > > - orit = "||"; > > - if (p->exclude) > > + if (p->exclude) { > > match = "!="; > > - else > > + if (p == filter_pids) > > + orit = ""; > > + else > > + orit = "&&"; > > + } else { > > match = "=="; > > + if (p == filter_pids) > > + orit = ""; > > + else > > + orit = "||"; > > + } > > A little nicer way of doing this is: > > > if (p->exclude) { > match = "!="; > orit = "&&"; > } else { > match = "=="; > orit = "||"; > } > if (p == filter_pids) > orit = ""; > > Care to send a v2?
Absolutely! On its way now. Sorry for not having cleaned it up properly. I was going for the quick-and-dirty. Never thought about collapsing the cases!
Will
> > -- Steve > > > len = sprintf(str, "%s(%s%s%d)", orit, field, match, p->pid); > > str += len; > > } >
| |