Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

etl::fsm receive function can call p_state->process_event() also when p_state is nullptr #869

Closed
kingtom22 opened this issue Mar 20, 2024 · 5 comments
Assignees

Comments

@kingtom22
Copy link

kingtom22 commented Mar 20, 2024

Hi,

I noticed, that there is no check of the p_state before calling its function.

void receive(const etl::imessage& message) ETL_OVERRIDE
{
  etl::fsm_state_id_t next_state_id = p_state->process_event(message);

  if (have_changed_state(next_state_id))
  {
    ETL_ASSERT(next_state_id < number_of_states, ETL_ERROR(etl::fsm_state_id_exception));
    etl::ifsm_state* p_next_state = state_list[next_state_id];

    do
    {
      p_state->on_exit_state();
      p_state = p_next_state;

      next_state_id = p_state->on_enter_state();

      if (have_changed_state(next_state_id))
      {
        ETL_ASSERT(next_state_id < number_of_states, ETL_ERROR(etl::fsm_state_id_exception));
        p_next_state = state_list[next_state_id];
      }
    } while (p_next_state != p_state); // Have we changed state again?
  }
  else if (is_self_transition(next_state_id))
  {
    p_state->on_exit_state();
    p_state->on_enter_state();
  }
}

What is the intended functionality? I did not found specifications of this functionality. Other functions of the etl::fsm asserting on the p_state before calling it.
for example:

etl::fsm_state_id_t get_state_id() const
{
  ETL_ASSERT(p_state != ETL_NULLPTR, ETL_ERROR(etl::fsm_null_state_exception));
  return p_state->get_state_id();
}
@kingtom22 kingtom22 changed the title etl::fsm receive function can call p_state->on_event() also when p_state is nullptr etl::fsm receive function can call p_state->process_event() also when p_state is nullptr Mar 20, 2024
@jwellbelove
Copy link
Contributor

jwellbelove commented Mar 20, 2024

You are correct in that, if an etl::fsm is constructed, but has not had set_states and start called, then a call to receive will try to call process_event on a nullptr state.

@kingtom22
Copy link
Author

And is it the intended functionality? Wouldnt be better to have an assert and a big if inside the function checking the pointer? I think it is a low hanging fruit and this would make the library safer.

@jwellbelove
Copy link
Contributor

I've already created a hotfix branch with the same change you describe.
The receive function will raise an etl::fsm_not_started if is_started() returns false and only execute the main function if is_started() returns true.

@kingtom22
Copy link
Author

Thank You for the quick reaction.

@jwellbelove
Copy link
Contributor

Fixed 20.38.11

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

No branches or pull requests

2 participants