Currently the automata parser assumes event strings don't have any
space, this stands true for event names, but can be a wrong assumption
if we want to store other information in the event strings (e.g.
constraints for hybrid automata).

Adapt the parser logic to allow spaces in the event strings.

Signed-off-by: Gabriele Monaco <gmonaco@redhat.com>
---
 tools/verification/rvgen/rvgen/automata.py | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/tools/verification/rvgen/rvgen/automata.py b/tools/verification/rvgen/rvgen/automata.py
index 3f06aef8d4fd..977ba859c34e 100644
--- a/tools/verification/rvgen/rvgen/automata.py
+++ b/tools/verification/rvgen/rvgen/automata.py
@@ -127,14 +127,13 @@ class Automata:
             #  ------------ event is here ------------^^^^^
             if self.__dot_lines[cursor].split()[1] == "->":
                 line = self.__dot_lines[cursor].split()
-                event = line[-2].replace('"','')
+                event = "".join(line[line.index("label")+2:-1]).replace('"','')
 
                 # when a transition has more than one lables, they are like this
                 # "local_irq_enable\nhw_local_irq_enable_n"
                 # so split them.
 
-                event = event.replace("\\n", " ")
-                for i in event.split():
+                for i in event.split("\\n"):
                     events.append(i)
             cursor += 1
 
@@ -167,8 +166,8 @@ class Automata:
                 line = self.__dot_lines[cursor].split()
                 origin_state = line[0].replace('"','').replace(',','_')
                 dest_state = line[2].replace('"','').replace(',','_')
-                possible_events = line[-2].replace('"','').replace("\\n", " ")
-                for event in possible_events.split():
+                possible_events = "".join(line[line.index("label")+2:-1]).replace('"','')
+                for event in possible_events.split("\\n"):
                     matrix[states_dict[origin_state]][events_dict[event]] = dest_state
             cursor += 1
 
-- 
2.51.0

Gabriele Monaco <gmonaco@redhat.com> writes:

> Currently the automata parser assumes event strings don't have any
> space, this stands true for event names, but can be a wrong assumption
> if we want to store other information in the event strings (e.g.
> constraints for hybrid automata).
>
> Adapt the parser logic to allow spaces in the event strings.

I probably misunderstand something, but isn't the description
misleading? After reading this description, I expect the patch to ignore
spaces or something similar. But from my understanding, the script only
allowed a single event, and this patch allows conditions as well.

Shouldn't this be squashed to the next patch?

Nam

On Thu, 2025-10-02 at 13:03 +0200, Nam Cao wrote:
> Gabriele Monaco <gmonaco@redhat.com> writes:
> 
> > Currently the automata parser assumes event strings don't have any
> > space, this stands true for event names, but can be a wrong assumption
> > if we want to store other information in the event strings (e.g.
> > constraints for hybrid automata).
> > 
> > Adapt the parser logic to allow spaces in the event strings.
> 
> I probably misunderstand something, but isn't the description
> misleading? After reading this description, I expect the patch to ignore
> spaces or something similar. But from my understanding, the script only
> allowed a single event, and this patch allows conditions as well.

The script allows multiple events, all separated by \n, strictly speaking there
is nothing saying spaces are not allowed in event names, but the parser breaks
if there's any space.

This patch allows spaces in event names, conditions (separated by a ; ) are not
supported yet.

> Shouldn't this be squashed to the next patch?

I kept it separated to avoid pushing too many changes in the next one, which
mostly adds new functionality (and lines) instead of changing the current ones.

Apparently that didn't make it any clearer, and there isn't really any use case
needing event names with spaces, so if it looks cleaner to you I can just squash
it.

Thanks,
Gabriele

Gabriele Monaco <gmonaco@redhat.com> writes:
> On Thu, 2025-10-02 at 13:03 +0200, Nam Cao wrote:
>> Gabriele Monaco <gmonaco@redhat.com> writes:
>> 
>> > Currently the automata parser assumes event strings don't have any
>> > space, this stands true for event names, but can be a wrong assumption
>> > if we want to store other information in the event strings (e.g.
>> > constraints for hybrid automata).
>> > 
>> > Adapt the parser logic to allow spaces in the event strings.
>> 
>> I probably misunderstand something, but isn't the description
>> misleading? After reading this description, I expect the patch to ignore
>> spaces or something similar. But from my understanding, the script only
>> allowed a single event, and this patch allows conditions as well.
>
> The script allows multiple events, all separated by \n, strictly speaking there
> is nothing saying spaces are not allowed in event names, but the parser breaks
> if there's any space.
>
> This patch allows spaces in event names, conditions (separated by a ; ) are not
> supported yet.
>
>> Shouldn't this be squashed to the next patch?
>
> I kept it separated to avoid pushing too many changes in the next one, which
> mostly adds new functionality (and lines) instead of changing the current ones.
>
> Apparently that didn't make it any clearer, and there isn't really any use case
> needing event names with spaces, so if it looks cleaner to you I can just squash
> it.

Nah, you can keep it. I was just confused. Now I looked again, it starts
to make sense.

Reviewed-by: Nam Cao <namcao@linutronix.de>

I am thinking about converting the entire thing to be ply-based (after
we are done with this series). It should make things easier to
follow. Would you object that?

Nam

On Mon, 2025-10-06 at 15:20 +0200, Nam Cao wrote:
> Nah, you can keep it. I was just confused. Now I looked again, it starts
> to make sense.
> 
> Reviewed-by: Nam Cao <namcao@linutronix.de>
> 

Alright, thanks!

> I am thinking about converting the entire thing to be ply-based (after
> we are done with this series). It should make things easier to
> follow. Would you object that?

I'm on board with that, I left out some other changes to the parser in light of
a major refactoring. I'd just need to see how this thing works but it really
seems the right thing to do.

Gabriele