linux-kernel - Re: [PATCH v3] tracing/selftests: Add test to test the trace

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Wed, 13 Dec 2023 17:08:00 +0200
From:   Alexander Kapshuk <alexander.kapshuk@...il.com>
To:     Steven Rostedt <rostedt@...dmis.org>
Cc:     LKML <linux-kernel@...r.kernel.org>,
        Linux Trace Kernel <linux-trace-kernel@...r.kernel.org>,
        Masami Hiramatsu <mhiramat@...nel.org>,
        Mark Rutland <mark.rutland@....com>,
        Mathieu Desnoyers <mathieu.desnoyers@...icios.com>,
        Shuah Khan <shuah@...nel.org>, linux-kselftest@...r.kernel.org
Subject: Re: [PATCH v3] tracing/selftests: Add test to test the trace_marker

On Wed, Dec 13, 2023 at 4:57 PM Steven Rostedt <rostedt@...dmis.org> wrote:
>
> On Wed, 13 Dec 2023 10:09:50 +0200
> Alexander Kapshuk <alexander.kapshuk@...il.com> wrote:
>
> > The REs used in the sed commands below may be simplified as shown if desired.
> >
> > On Wed, Dec 13, 2023 at 2:22 AM Steven Rostedt <rostedt@...dmis.org> wrote:
> > >
> > > From: "Steven Rostedt (Google)" <rostedt@...dmis.org>
> > >
> > > Add a test that writes longs strings, some over the size of the sub buffer
> > > and make sure that the entire content is there.
> > >
> > > Signed-off-by: Steven Rostedt (Google) <rostedt@...dmis.org>
> > > ---
> > > Changes since v2: https://lore.kernel.org/linux-trace-kernel/20231212151632.25c9b67d@gandalf.local.home
> > >
> > > - Realized with the upcoming change of the dynamic subbuffer sizes, that
> > >   this test will fail if the subbuffer is bigger than what the trace_seq
> > >   can hold. Now the trace_marker does not always utilize the full subbuffer
> > >   but the size of the trace_seq instead. As that size isn't available to
> > >   user space, we can only just make sure all content is there.
> > >
> > >  .../ftrace/test.d/00basic/trace_marker.tc     | 82 +++++++++++++++++++
> > >  1 file changed, 82 insertions(+)
> > >  create mode 100755 tools/testing/selftests/ftrace/test.d/00basic/trace_marker.tc
> > >
> > > diff --git a/tools/testing/selftests/ftrace/test.d/00basic/trace_marker.tc b/tools/testing/selftests/ftrace/test.d/00basic/trace_marker.tc
> > > new file mode 100755
> > > index 000000000000..b24aff5807df
> > > --- /dev/null
> > > +++ b/tools/testing/selftests/ftrace/test.d/00basic/trace_marker.tc
> > > @@ -0,0 +1,82 @@
> > > +#!/bin/sh
> > > +# SPDX-License-Identifier: GPL-2.0
> > > +# description: Basic tests on writing to trace_marker
> > > +# requires: trace_marker
> > > +# flags: instance
> > > +
> > > +get_buffer_data_size() {
> > > +       sed -ne 's/^.*data.*size:\([0-9][0-9]*\).*/\1/p' events/header_page
> >             sed -n 's!.*data.*size:\([^;]*\).*!\1!p' events/header_page
>
> Not sure the above change can be considered simpler, but it also loses out
> showing what exactly is being done.
>
> With the original, I have:
>
>        sed -ne 's/^.*data.*size:\([0-9][0-9]*\).*/\1/p' events/header_page
>
> Which is obvious that I'm grabbing a number for the size field.
>
>        sed -n 's!.*data.*size:\([^;]*\).*!\1!p' events/header_page
>
> Shows that I'm grabbing something after size.
>
>
>
> > > +}
> > > +
> > > +get_buffer_data_offset() {
> > > +       sed -ne 's/^.*data.*offset:\([0-9][0-9]*\).*/\1/p' events/header_page
> >             sed -n 's!.*data.*offset:\([^;]*\).*!\1!p' events/header_page
>
> Same here.
>
> > > +}
> > > +
> > > +get_event_header_size() {
> > > +       type_len=`sed -ne 's/^.*type_len.*:[^0-9]*\([0-9][0-9]*\).*/\1/p' events/header_event`
> >             type_len=`sed -n '/type_len.*bits/s![^0-9]*!!gp'
> > events/header_event`
>
> Honestly, the above may be "simplier" but I can't make out what exactly
> that line is doing without going back and looking at the text that's in the
> format field.
>
> >
> > > +       time_len=`sed -ne 's/^.*time_delta.*:[^0-9]*\([0-9][0-9]*\).*/\1/p' events/header_event`
> >             time_len=`sed -n '/time_delta/s![^0-9]*!!gp' events/header_event`
> >
> > > +       array_len=`sed -ne 's/^.*array.*:[^0-9]*\([0-9][0-9]*\).*/\1/p' events/header_event`
> >             array_len=`sed -n '/array/s![^0-9]*!!gp' events/header_event`
> >
> > > +       total_bits=$((type_len+time_len+array_len))
> > > +       total_bits=$((total_bits+7))
> > > +       echo $((total_bits/8))
> > > +}
> > > +
> > > +get_print_event_buf_offset() {
> > > +       sed -ne 's/^.*buf.*offset:\([0-9][0-9]*\).*/\1/p' events/ftrace/print/format
> >             sed -n 's!.*buf.*offset:\([^;]*\).*!\1!p' events/ftrace/print/format
> > > +}
> > > +
>
> Yeah, thanks for the suggestions, but I rather have it be more readable
> than "simplified". I write perl code the same way. I do not write it like
> any perl developer would, because I write it like C code. I want my code to
> be easily understandable.
>
> RE can become extremely obfuscated. So, even though my REs are not the
> simplest, they tend to be rather easy to understand what they are doing,
> and why.
>
> Cheers,
>
> -- Steve

Fair enough.
Thanks.