[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <d15348f9f6d8ed71ba3e928ae1b5da15711894e3.camel@redhat.com>
Date: Tue, 20 Jan 2026 14:11:46 +0100
From: Gabriele Monaco <gmonaco@...hat.com>
To: Wander Lairson Costa <wander@...hat.com>
Cc: Steven Rostedt <rostedt@...dmis.org>, Nam Cao <namcao@...utronix.de>,
open list <linux-kernel@...r.kernel.org>, "open list:RUNTIME VERIFICATION
(RV)" <linux-trace-kernel@...r.kernel.org>
Subject: Re: [PATCH 26/26] rv/rvgen: extract node marker string to class
constant
On Tue, 2026-01-20 at 13:36 +0100, Gabriele Monaco wrote:
> On Tue, 2026-01-20 at 08:34 -0300, Wander Lairson Costa wrote:
> > On Tue, Jan 20, 2026 at 10:03:20AM +0100, Gabriele Monaco wrote:
> > > On Mon, 2026-01-19 at 17:46 -0300, Wander Lairson Costa wrote:
> > > > Add a node_marker class constant to the Automata class to replace the
> > > > hardcoded "{node" string literal used throughout the DOT file parsing
> > > > logic. This follows the existing pattern established by the init_marker
> > > > and invalid_state_str class constants in the same class.
> > > >
> > > > The "{node" string is used as a marker to identify node declaration
> > > > lines in DOT files during state variable extraction and cursor
> > > > positioning. Extracting it to a named constant improves code
> > > > maintainability and makes the marker's purpose explicit.
> > > >
> > > > Signed-off-by: Wander Lairson Costa <wander@...hat.com>
> > >
> > > Looks fine for me, thanks!
> > >
> > > I wonder if we could merge this patch with 15/26 that is introducing a
> > > very
> > > similar change on init_marker.
> >
> > The idea was to make each patch doing one thing to make the reviewer's
> > life easier (I think I broke my own rule in a couple of patch). But if
> > there is a strong feeling about the merge, I could merged them in a
> > possible v2 patch series.
> >
>
> Yeah I get the idea, I guess I could just pick the most obvious patches and
> send
> a PR to Steve before the merge window, so I can see directly if it makes sense
> to squash them and you don't need to send them all in the v2.
Screamed victory too fast.. I tried the patches on the latest version of the
tree (which reached already linux-next) and they're have several conflicts.
I don't have time to rebase them one by one right now, let's continue with this
series as a whole.
As a rule of thumb, considering these patches get first reviewed, then PR-ed to
Steve and then to Linus, I'd favour to lower the number if possible, but feel
free to choose where it makes sense for them to stay separate.
Thanks,
Gabriele
>
> I'm leaving the longer or trickier ones for the next cycle so that I can test
> them a bit better and perhaps get some rvgen selftest help with that. Also for
> some it's better to wait for Nam's comments.
>
> Will let you know the ones I pick. Thanks again for the extensive work!
>
> Gabriele
>
> > >
> > > Anyway:
> > >
> > > Reviewed-by: Gabriele Monaco <gmonaco@...hat.com>
> > >
> > > > ---
> > > > tools/verification/rvgen/rvgen/automata.py | 9 +++++----
> > > > 1 file changed, 5 insertions(+), 4 deletions(-)
> > > >
> > > > diff --git a/tools/verification/rvgen/rvgen/automata.py
> > > > b/tools/verification/rvgen/rvgen/automata.py
> > > > index a6889d0c26c3f..5f23db1855cd3 100644
> > > > --- a/tools/verification/rvgen/rvgen/automata.py
> > > > +++ b/tools/verification/rvgen/rvgen/automata.py
> > > > @@ -29,6 +29,7 @@ class Automata:
> > > >
> > > > invalid_state_str = "INVALID_STATE"
> > > > init_marker = "__init_"
> > > > + node_marker = "{node"
> > > >
> > > > def __init__(self, file_path, model_name=None):
> > > > self.__dot_path = file_path
> > > > @@ -76,7 +77,7 @@ class Automata:
> > > > for cursor, line in enumerate(self.__dot_lines):
> > > > split_line = line.split()
> > > >
> > > > - if len(split_line) and split_line[0] == "{node":
> > > > + if len(split_line) and split_line[0] == self.node_marker:
> > > > return cursor
> > > >
> > > > raise AutomataError("Could not find a beginning state")
> > > > @@ -91,9 +92,9 @@ class Automata:
> > > > continue
> > > >
> > > > if state == 0:
> > > > - if line[0] == "{node":
> > > > + if line[0] == self.node_marker:
> > > > state = 1
> > > > - elif line[0] != "{node":
> > > > + elif line[0] != self.node_marker:
> > > > break
> > > > else:
> > > > raise AutomataError("Could not find beginning event")
> > > > @@ -116,7 +117,7 @@ class Automata:
> > > > # process nodes
> > > > for line in islice(self.__dot_lines, cursor, None):
> > > > split_line = line.split()
> > > > - if not split_line or split_line[0] != "{node":
> > > > + if not split_line or split_line[0] != self.node_marker:
> > > > break
> > > >
> > > > raw_state = split_line[-1]
Powered by blists - more mailing lists