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

[foxy] Fix memory leak in rcl_subscription_init()/rcl_publisher_init() (#794) #832

Merged
merged 3 commits into from
Oct 16, 2020

Conversation

jacobperron
Copy link
Member

Backport #794 to Foxy.

* Fix memory leak in rcl_subscription_init()/rcl_publisher_init()

In rcl_subscription_init(), while rmw_subscription_get_actual_qos()
return failure, created rmw subscription handle isn't freed.
In rcl_publisher_init(), while rmw_publisher_get_actual_qos()
return failure, created rmw publisher handle isn't freed.

Signed-off-by: Barry Xu <barry.xu@sony.com>

* Remove codes on the cascading errors.

Signed-off-by: Barry Xu <barry.xu@sony.com>

* Change code style

Signed-off-by: Barry Xu <barry.xu@sony.com>

* Output error message to stderr

Signed-off-by: Barry Xu <barry.xu@sony.com>
@jacobperron jacobperron self-assigned this Oct 14, 2020
@jacobperron jacobperron changed the title Fix memory leak in rcl_subscription_init()/rcl_publisher_init() (#794) [foxy] Fix memory leak in rcl_subscription_init()/rcl_publisher_init() (#794) Oct 14, 2020
This version of the macro is not available in Foxy.

Signed-off-by: Jacob Perron <jacob@openrobotics.org>
@jacobperron
Copy link
Member Author

The rcutils macro being used isn't available in Foxy, so I've opted to not use a format string to print the file and line number (4bf6989).

rcl/src/rcl/publisher.c Show resolved Hide resolved
rcl/src/rcl/publisher.c Show resolved Hide resolved
Signed-off-by: Jacob Perron <jacob@openrobotics.org>
@jacobperron jacobperron merged commit 76a706d into foxy Oct 16, 2020
@delete-merged-branch delete-merged-branch bot deleted the jacob/backport_foxy_794 branch October 16, 2020 20:42
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.

4 participants