On Thu, Dec 06, 2018 at 05:20:54PM +0000, Quentin Monnet wrote: > 2018-12-05 19:18 UTC-0800 ~ Alexei Starovoitov > <alexei.starovoi...@gmail.com> > > On Wed, Dec 05, 2018 at 06:15:23PM +0000, Quentin Monnet wrote: > >>>> + > >>>> + /* Allow room for NULL terminating byte and pipe file name */ > >>>> + snprintf(format, sizeof(format), "%%*s %%%zds %%99s %%*s %%*d > >>>> %%*d\\n", > >>>> + PATH_MAX - strlen(pipe_name) - 1); > >>> > >>> before scanning trace_pipe could you add a check that trace_options are > >>> compatible? > >>> Otherwise there will be a lot of garbage printed. > >>> afaik default is rarely changed, so the patch is ok as-is. > >>> The followup some time in the future would be perfect. > >> > >> Sure. What do you mean exactly by compatible options? I can check that > >> "trace_printk" is set, is there any other option that would be relevant? > > > > See Documentation/trace/ftrace.rst > > a lot of the flags will change the format significantly. > > Like 'bin' will make it binary. > > I'm not suggesting to support all possible output formats. > > Only to check that trace flags match scanf. > > fscanf() is only used to retrieve the name of the sysfs directory where > the pipe is located, when listing all the mount points on the system. It > is not used to dump the content from the pipe (which is done with > getline(), so formatting does not matter much). > > If the "bin" option is set, "bpftool prog tracelog" will dump the same > binary content as "cat /sys/kernel/debug/tracing/trace_pipe", which is > the expected behaviour (at least with the current patch). Let me know if > you would like me to change this somehow.
I misread the patch :) thanks for explaining. all good then.
