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

Do not change the contents of a std::set within a range-for loop, as it #88

Merged
merged 1 commit into from
Jul 3, 2023

Conversation

eflumerf
Copy link
Member

can invalidate the iterators used for loop control.

Found because the ZmqPubSub unit test was failing. Don't know how it was missed before.

can invalidate the iterators used for loop control.
@eflumerf eflumerf added the bug Something isn't working label Jun 28, 2023
@eflumerf eflumerf self-assigned this Jun 28, 2023
Copy link
Contributor

@bieryAtFnal bieryAtFnal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.
For me, the unit tests passed (in a couple of attempts) both before and after the change. However, I can definitely see how the "erase" can cause problems. And, I can see that removing the "erase" call won't noticeably change the behavior of the code since the full m_connection data member will be cleaned up after the destructor is finished.

@eflumerf eflumerf merged commit f09e0b4 into develop Jul 3, 2023
3 checks passed
@eflumerf eflumerf deleted the eflumerf/DontUpdateSetInRangeForLoop branch July 3, 2023 14:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants