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

Match alloc and free calls #10

Merged
merged 1 commit into from
Dec 30, 2015
Merged

Match alloc and free calls #10

merged 1 commit into from
Dec 30, 2015

Conversation

gerkey
Copy link
Member

@gerkey gerkey commented Dec 30, 2015

This test result:

http://ci.ros2.org/view/nightly/job/nightly_linux/143/testReport/%28root%29/test_services_cpp__rmw_fastrtps_cpp/test_services_cpp/

shows a service client crashing with a SIGABRT (-6). valgrind identified mismatched allocation and destruction calls. Here I'm matching them up. With this change, valgrind gives a zero-error report on the service client (there still seem to be situations in which the test can hang instead of passing, but that's a different matter).

@wjwwood
Copy link
Member

wjwwood commented Dec 30, 2015

lgtm, I guess was malloc being used with delete and not free?

@gerkey
Copy link
Member Author

gerkey commented Dec 30, 2015

The node->name was being malloced, then deleted, which was definitely wrong. The node itself was being rmw_node_allocated, then deleted, which could be right or wrong, depending on what happens in the allocate function. So I normalized them both to use malloc and free.

@gerkey
Copy link
Member Author

gerkey commented Dec 30, 2015

@wjwwood, if you like this change, can you merge it? (I don't have commit permissions on this repo.)

wjwwood added a commit that referenced this pull request Dec 30, 2015
Match alloc and free calls
@wjwwood wjwwood merged commit e04a32d into ros2:master Dec 30, 2015
@sloretz sloretz mentioned this pull request Oct 18, 2018
24 tasks
mauropasse pushed a commit to mauropasse/rmw_fastrtps that referenced this pull request Dec 1, 2020
Rename set_events_executor_callback->set_listener_callback
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