Skip to content

[rclcpp_lifecycle] Change uint8_t iterator variables to size_t#1461

Merged
brawner merged 2 commits intomasterfrom
brawner/for-loop-uint8_t-unsigned-int
Nov 17, 2020
Merged

[rclcpp_lifecycle] Change uint8_t iterator variables to size_t#1461
brawner merged 2 commits intomasterfrom
brawner/for-loop-uint8_t-unsigned-int

Conversation

@brawner
Copy link
Copy Markdown
Contributor

@brawner brawner commented Nov 16, 2020

This is a small theoretical issue I found. The type of states_size and transitions_size is unsigned int, but the for loop int type is uint8_t. While default_state_machine has only 11 states and 25 transitions, looping with a small integer type that is compared to a larger type could lead to an infinite loop. While size_t here is large enough to avoid problems, I'd also be happy to use also unsigned int.

Signed-off-by: Stephen Brawner brawner@gmail.com

Signed-off-by: Stephen Brawner <brawner@gmail.com>
@brawner
Copy link
Copy Markdown
Contributor Author

brawner commented Nov 16, 2020

Testing --packages-select rclcpp_lifecycle

  • Linux Build Status
  • Linux-aarch64 Build Status
  • macOS Build Status
  • Windows Build Status

@clalancette
Copy link
Copy Markdown
Contributor

I'd also be happy to use also unsigned int.

Given that states_size is defined as an unsigned int, I suggest we stick with that type here. It just ensures that we are comparing the same type always. I don't think this change has any practical difference on the platforms we support, but it makes it easy to tell that we are doing the correct thing.

Signed-off-by: Stephen Brawner <brawner@gmail.com>
@brawner
Copy link
Copy Markdown
Contributor Author

brawner commented Nov 17, 2020

Sounds good, fixed.

@brawner
Copy link
Copy Markdown
Contributor Author

brawner commented Nov 17, 2020

  • Linux Build Status
  • Linux-aarch64 Build Status
  • macOS Build Status
  • Windows Build Status

@brawner brawner merged commit add6d61 into master Nov 17, 2020
@delete-merged-branch delete-merged-branch bot deleted the brawner/for-loop-uint8_t-unsigned-int branch November 17, 2020 19:22
jacobperron pushed a commit that referenced this pull request Dec 8, 2020
* Change uint8_t iterator variables to size_t

Signed-off-by: Stephen Brawner <brawner@gmail.com>

* Change to unsigned int

Signed-off-by: Stephen Brawner <brawner@gmail.com>
jacobperron added a commit that referenced this pull request Dec 8, 2020
#1488)

* Change uint8_t iterator variables to size_t

Signed-off-by: Stephen Brawner <brawner@gmail.com>

* Change to unsigned int

Signed-off-by: Stephen Brawner <brawner@gmail.com>

Co-authored-by: brawner <brawner@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants