| Message ID | 20230317212430.12675-1-krisman@suse.de (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [trace-cmd] trace-cmd report: Ensure filter is applied to single input file | expand |
On Fri, 17 Mar 2023 18:24:30 -0300 Gabriel Krisman Bertazi <krisman@suse.de> wrote: > Since 955d05fc7aee ("trace-cmd report: Make filter arguments match their > files"), the -F filtering is silently ignored when a trace file is > provided with -i and the filter comes after -i . The reason is that the > filter is now associated with input_files and not saved to the global > list only in this case, but process_filters still only checks the global > list when handles->input_file is not set. > > Avoid this by checking last_input_file first, which always contains a > pointer to the correct filter in this case. > > This was only lightly tested, using a single trace file. > > Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=217038 > Fixes: 955d05fc7aee ("trace-cmd report: Make filter arguments match their files") > Signed-off-by: Gabriel krisman Bertazi <krisman@suse.de> > --- > tracecmd/trace-read.c | 10 +++++++--- > 1 file changed, 7 insertions(+), 3 deletions(-) > > diff --git a/tracecmd/trace-read.c b/tracecmd/trace-read.c > index 52ba818..dbbd124 100644 > --- a/tracecmd/trace-read.c > +++ b/tracecmd/trace-read.c > @@ -567,11 +567,15 @@ static void process_filters(struct handle_list *handles) > struct tracecmd_filter *trace_filter; > struct filter_str *filter; > int filters = 0; > + struct input_files *input_file = handles->input_file; BTW, please keep the "upside-down x-mas tree" formatting of the above. ie. static void process_filters(struct handle_list *handles) { struct input_files *input_file = handles->input_file; struct tracecmd_filter *trace_filter; struct filter_str *filter; int filters = 0; > > - make_pid_filter(handles->handle, handles->input_file); > + if (!input_file) > + input_file = last_input_file; > > - if (handles->input_file) > - filter = handles->input_file->filter_str; > + make_pid_filter(handles->handle, input_file); > + > + if (input_file) > + filter = input_file->filter_str; > else > filter = filter_strings; > So I was playing with this, and I actually needed something else. If an -F comes before *any* file, I think it should be added to *all* files. That is: -F sched_switch -i trace1.dat -i trace2.dat Should filter sched_switch for both trace1.dat and trace2.dat. I actually wanted this just a few minutes ago ;-) Can you do this, or would you want me to? -- Steve
Steven Rostedt <rostedt@goodmis.org> writes: > On Fri, 17 Mar 2023 18:24:30 -0300 > Gabriel Krisman Bertazi <krisman@suse.de> wrote: > >> Since 955d05fc7aee ("trace-cmd report: Make filter arguments match their >> files"), the -F filtering is silently ignored when a trace file is >> provided with -i and the filter comes after -i . The reason is that the >> filter is now associated with input_files and not saved to the global >> list only in this case, but process_filters still only checks the global >> list when handles->input_file is not set. >> >> Avoid this by checking last_input_file first, which always contains a >> pointer to the correct filter in this case. >> >> This was only lightly tested, using a single trace file. >> >> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=217038 >> Fixes: 955d05fc7aee ("trace-cmd report: Make filter arguments match their files") >> Signed-off-by: Gabriel krisman Bertazi <krisman@suse.de> >> --- >> tracecmd/trace-read.c | 10 +++++++--- >> 1 file changed, 7 insertions(+), 3 deletions(-) >> >> diff --git a/tracecmd/trace-read.c b/tracecmd/trace-read.c >> index 52ba818..dbbd124 100644 >> --- a/tracecmd/trace-read.c >> +++ b/tracecmd/trace-read.c >> @@ -567,11 +567,15 @@ static void process_filters(struct handle_list *handles) >> struct tracecmd_filter *trace_filter; >> struct filter_str *filter; >> int filters = 0; >> + struct input_files *input_file = handles->input_file; > > BTW, please keep the "upside-down x-mas tree" formatting of the above. > > ie. Hey Steve, thanks for the review! > > static void process_filters(struct handle_list *handles) > { > struct input_files *input_file = handles->input_file; > struct tracecmd_filter *trace_filter; > struct filter_str *filter; > int filters = 0; > > >> >> - make_pid_filter(handles->handle, handles->input_file); >> + if (!input_file) >> + input_file = last_input_file; >> >> - if (handles->input_file) >> - filter = handles->input_file->filter_str; >> + make_pid_filter(handles->handle, input_file); >> + >> + if (input_file) >> + filter = input_file->filter_str; >> else >> filter = filter_strings; >> > > So I was playing with this, and I actually needed something else. > > If an -F comes before *any* file, I think it should be added to *all* files. > > That is: > > -F sched_switch -i trace1.dat -i trace2.dat > > Should filter sched_switch for both trace1.dat and trace2.dat. > > I actually wanted this just a few minutes ago ;-) > > Can you do this, or would you want me to? sure, i can do that. Unless you are in a hurry, let me take a look later this week.
On Tue, 28 Mar 2023 11:30:03 -0300 Gabriel Krisman Bertazi <krisman@suse.de> wrote: > > I actually wanted this just a few minutes ago ;-) > > > > Can you do this, or would you want me to? > > sure, i can do that. > > Unless you are in a hurry, let me take a look later this week. No hurry. Thanks. -- Steve
diff --git a/tracecmd/trace-read.c b/tracecmd/trace-read.c index 52ba818..dbbd124 100644 --- a/tracecmd/trace-read.c +++ b/tracecmd/trace-read.c @@ -567,11 +567,15 @@ static void process_filters(struct handle_list *handles) struct tracecmd_filter *trace_filter; struct filter_str *filter; int filters = 0; + struct input_files *input_file = handles->input_file; - make_pid_filter(handles->handle, handles->input_file); + if (!input_file) + input_file = last_input_file; - if (handles->input_file) - filter = handles->input_file->filter_str; + make_pid_filter(handles->handle, input_file); + + if (input_file) + filter = input_file->filter_str; else filter = filter_strings;
Since 955d05fc7aee ("trace-cmd report: Make filter arguments match their files"), the -F filtering is silently ignored when a trace file is provided with -i and the filter comes after -i . The reason is that the filter is now associated with input_files and not saved to the global list only in this case, but process_filters still only checks the global list when handles->input_file is not set. Avoid this by checking last_input_file first, which always contains a pointer to the correct filter in this case. This was only lightly tested, using a single trace file. Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=217038 Fixes: 955d05fc7aee ("trace-cmd report: Make filter arguments match their files") Signed-off-by: Gabriel krisman Bertazi <krisman@suse.de> --- tracecmd/trace-read.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-)