Skip to content

Copy recorder QoS profile to local variable so that temporary value isn't cleared#803

Merged
emersonknapp merged 1 commit intomasterfrom
emersonknapp/galactic-fix-qos-warning
Jul 2, 2021
Merged

Copy recorder QoS profile to local variable so that temporary value isn't cleared#803
emersonknapp merged 1 commit intomasterfrom
emersonknapp/galactic-fix-qos-warning

Conversation

@emersonknapp
Copy link
Copy Markdown
Collaborator

@emersonknapp emersonknapp commented Jun 30, 2021

Fixes #772
Fixes #802

This function was using a temporary variable that got cleared, and so had undefined behavior. Create a local variable to make the reference valid.

…sn't cleared

Signed-off-by: Emerson Knapp <eknapp@amazon.com>
@emersonknapp emersonknapp marked this pull request as ready for review July 2, 2021 17:07
@emersonknapp emersonknapp requested a review from a team as a code owner July 2, 2021 17:07
@emersonknapp emersonknapp requested review from MichaelOrlov and piraka9011 and removed request for a team July 2, 2021 17:07
Copy link
Copy Markdown
Contributor

@piraka9011 piraka9011 left a comment

Choose a reason for hiding this comment

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

Good to know about this behavior.

Note that this is the suggested usage in the release notes for ROS2 E here and is used in a similar manner in ros2/rclcpp

@emersonknapp
Copy link
Copy Markdown
Collaborator Author

Gist: https://gist.githubusercontent.com/emersonknapp/b7950c8a1ec619dc9e3a81d619059818/raw/0b1336f778774e97fa43f903388f4e42a120aa94/ros2.repos
BUILD args: --packages-up-to rosbag2_transport rosbag2 rosbag2_tests
TEST args: --packages-select rosbag2_transport rosbag2 rosbag2_tests
Job: ci_launcher
ci_launcher ran: https://ci.ros2.org/job/ci_launcher/8653

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

@emersonknapp emersonknapp merged commit 8234765 into master Jul 2, 2021
@delete-merged-branch delete-merged-branch bot deleted the emersonknapp/galactic-fix-qos-warning branch July 2, 2021 19:44
emersonknapp added a commit that referenced this pull request Jul 2, 2021
…sn't cleared (#803)

Signed-off-by: Emerson Knapp <eknapp@amazon.com>
emersonknapp added a commit that referenced this pull request Jul 2, 2021
…sn't cleared (#803)

Signed-off-by: Emerson Knapp <eknapp@amazon.com>
emersonknapp added a commit that referenced this pull request Jul 3, 2021
…sn't cleared (#803) (#805)

Signed-off-by: Emerson Knapp <eknapp@amazon.com>
@MichaelOrlov
Copy link
Copy Markdown
Contributor

https://github.com/Mergifyio backport foxy

@mergify
Copy link
Copy Markdown

mergify bot commented May 16, 2023

backport foxy

✅ Backports have been created

Details

mergify bot pushed a commit that referenced this pull request May 16, 2023
…sn't cleared (#803)

Signed-off-by: Emerson Knapp <eknapp@amazon.com>
(cherry picked from commit 8234765)

# Conflicts:
#	rosbag2_transport/src/rosbag2_transport/recorder.cpp
MichaelOrlov pushed a commit that referenced this pull request May 16, 2023
…sn't cleared (#803)

Co-authored-by: Emerson Knapp <eknapp@amazon.com>
Signed-off-by: Michael Orlov <michael.orlov@apex.ai>
(cherry picked from commit 8234765)
MichaelOrlov pushed a commit that referenced this pull request May 16, 2023
…sn't cleared (#803) (#1335)

Co-authored-by: Emerson Knapp <eknapp@amazon.com>
Signed-off-by: Michael Orlov <michael.orlov@apex.ai>
(cherry picked from commit 8234765)

Co-authored-by: Michael Orlov <michael.orlov@apex.ai>
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.

Overriding QoS Profiles Doesn't Work Incorrect QoS compatibility warning

3 participants