Re: [PATCH 16/26] rv/rvgen: fix unbound initial_state variable

[Gabriele Monaco <gmonaco@redhat.com>]

On Tue, 2026-01-20 at 08:42 -0300, Wander Lairson Costa wrote:
> On Tue, Jan 20, 2026 at 09:21:01AM +0100, Gabriele Monaco wrote:
> 
> > 
> > I believe these newlines were added automatically by some tools, not sure if
> > we
> > really want them but they don't belong in this patch (since 1/26 added this
> > class).
> > 
> There are python formatting tools that put 2 lines after a type
> definition. Anyway, I can remove it in v2.

If that's common practice I'm fine keeping it, I just would create this class
directly with the newlines separators in 1/26 instead of adding these lines
later in the same series. Just to reduce the noise.

> 
> > >  class Automata:
> > >      """Automata class: Reads a dot file and parses it as an automata.
> > >  
> > > @@ -31,7 +33,9 @@ class Automata:
> > >          self.__dot_path = file_path
> > >          self.name = model_name or self.__get_model_name()
> > >          self.__dot_lines = self.__open_dot()
> > > -        self.states, self.initial_state, self.final_states =
> > > self.__get_state_variables()
> > > +        self.states, self.initial_state, self.final_states = (
> > > +            self.__get_state_variables()
> > > +        )
> > 
> > There is no strict 80 character limit for python code and I personally find
> > this
> > less readable. Is this again what the tool suggested?
> > 
> 
> In general, 100 lines is assumed the to a good limit. Anyway, this is
> minor and I can remove the change in v2 if desired.

Yeah 100 is probably also what checkpatch.pl uses (although I think it skips
python files), since this is below 90 columns, I wouldn't touch it.

Thanks,
Gabriele

> 
> > >          self.events = self.__get_event_variables()
> > >          self.function = self.__create_matrix()
> > >          self.events_start, self.events_start_run =
> > > self.__store_init_events()
> > > @@ -86,6 +90,7 @@ class Automata:
> > >          # wait for node declaration
> > >          states = []
> > >          final_states = []
> > > +        initial_state = None
> > >  
> > >          has_final_states = False
> > >          cursor = self.__get_cursor_begin_states()
> > > @@ -96,9 +101,9 @@ class Automata:
> > >              raw_state = line[-1]
> > >  
> > >              #  "enabled_fired"}; -> enabled_fired
> > > -            state = raw_state.replace('"', '').replace('};',
> > > '').replace(',',
> > > '_')
> > > +            state = raw_state.replace('"', "").replace("};",
> > > "").replace(",",
> > > "_")
> > 
> > Ok this change is good.
> > 
> > >              if state.startswith(self.init_marker):
> > > -                initial_state = state[len(self.init_marker):]
> > > +                initial_state = state[len(self.init_marker) :]
> > 
> > You fixed spacing in 12/26. We could either keep move this change there or
> > just
> > merge that patch with others touching the same files.
> > 
> 
> Np, I can move this change to there.
> 
> > >              else:
> > >                  states.append(state)
> > >                  if "doublecircle" in self.__dot_lines[cursor]:
> > > @@ -111,6 +116,9 @@ class Automata:
> > >  
> > >              cursor += 1
> > >  
> > > +        if initial_state is None:
> > > +            raise AutomataError("The automaton doesn't have a initial
> > > state")
> > > +
> > 
> > Yeah that's needed.
> > 
> > >          states = sorted(set(states))
> > >  
> > >          # Insert the initial state at the beginning of the states
> > > @@ -132,7 +140,7 @@ class Automata:
> > >              #  ------------ event is here ------------^^^^^
> > >              if self.__dot_lines[cursor].split()[1] == "->":
> > >                  line = self.__dot_lines[cursor].split()
> > > -                event = line[-2].replace('"', '')
> > > +                event = line[-2].replace('"', "")
> > >  
> > >                  # when a transition has more than one labels, they are
> > > like
> > > this
> > >                  # "local_irq_enable\nhw_local_irq_enable_n"
> > > @@ -162,7 +170,9 @@ class Automata:
> > >              nr_state += 1
> > >  
> > >          # declare the matrix....
> > > -        matrix = [[self.invalid_state_str for x in range(nr_event)] for y
> > > in
> > > range(nr_state)]
> > > +        matrix = [
> > > +            [self.invalid_state_str for x in range(nr_event)] for y in
> > > range(nr_state)
> > > +        ]
> > >  
> > >          # and we are back! Let's fill the matrix
> > >          cursor = self.__get_cursor_begin_events()
> > > @@ -170,9 +180,9 @@ class Automata:
> > >          while self.__dot_lines[cursor].lstrip()[0] == '"':
> > >              if self.__dot_lines[cursor].split()[1] == "->":
> > >                  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", "
> > > ")
> > > +                origin_state = line[0].replace('"', "").replace(",", "_")
> > > +                dest_state = line[2].replace('"', "").replace(",", "_")
> > > +                possible_events = line[-2].replace('"',
> > > "").replace("\\n", "
> > > ")
> > 
> > All in all looks good, thanks.
> > 
> > Reviewed-by: Gabriele Monaco <gmonaco@redhat.com>
> > 
> > >                  for event in possible_events.split():
> > >                      matrix[states_dict[origin_state]][events_dict[event]]
> > > =
> > > dest_state
> > >              cursor += 1