[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <85c7d4762464e6a50b4318f4ea56a70202a4b287.camel@redhat.com>
Date: Tue, 20 Jan 2026 13:36:35 +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 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.
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