[PATCH 26/26] rv/rvgen: extract node marker string to class constant

Wander Lairson Costa posted 26 patches 2 weeks, 5 days ago
There is a newer version of this series
[PATCH 26/26] rv/rvgen: extract node marker string to class constant
Posted by Wander Lairson Costa 2 weeks, 5 days ago
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@redhat.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]
-- 
2.52.0
Re: [PATCH 26/26] rv/rvgen: extract node marker string to class constant
Posted by Gabriele Monaco 2 weeks, 5 days ago
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@redhat.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.

Anyway:

Reviewed-by: Gabriele Monaco <gmonaco@redhat.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]
Re: [PATCH 26/26] rv/rvgen: extract node marker string to class constant
Posted by Wander Lairson Costa 2 weeks, 5 days ago
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@redhat.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.

> 
> Anyway:
> 
> Reviewed-by: Gabriele Monaco <gmonaco@redhat.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]
Re: [PATCH 26/26] rv/rvgen: extract node marker string to class constant
Posted by Gabriele Monaco 2 weeks, 5 days ago
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@redhat.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@redhat.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]
Re: [PATCH 26/26] rv/rvgen: extract node marker string to class constant
Posted by Gabriele Monaco 2 weeks, 4 days ago
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@redhat.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@redhat.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]
Re: [PATCH 26/26] rv/rvgen: extract node marker string to class constant
Posted by Wander Lairson Costa 2 weeks, 4 days ago
On Tue, Jan 20, 2026 at 02:11:46PM +0100, Gabriele Monaco wrote:
> 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@redhat.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.

I created the patches on top of linux-trace/tools/for-next. Is this the
wrong branch?
> 
> Thanks,
> Gabriele
>
Re: [PATCH 26/26] rv/rvgen: extract node marker string to class constant
Posted by Gabriele Monaco 2 weeks, 4 days ago
On Tue, 2026-01-20 at 15:56 -0300, Wander Lairson Costa wrote:
> I created the patches on top of linux-trace/tools/for-next. Is this the
> wrong branch?
> 

Oh that tree is tricky, usually RV pull requests go to latency/for-next , that's
where you can find it this time (Steve created the specific rv/fixes and rv/for-
next but hasn't used them recently).

Anyway they're all linked to linux-next, so that's usually a safe bet.

Thanks,
Gabriele