expose list of nodes #88
Conversation
| for (size_t i = 0; i < name_count; i++) { | ||
| rmw_free(node_names->names[i]); | ||
| } | ||
| rmw_free(node_names->names); |
There was a problem hiding this comment.
This should set the values back to nullptr / 0 before deallocation (same as it is done for Connext).
| rmw_node_names_t * node_names) | ||
| { | ||
| size_t name_count = node_names->node_count; | ||
| for (size_t i = 0; i < name_count; i++) { |
| node_names->node_count = participant_names.size(); | ||
| node_names->names = static_cast<char **>(rmw_allocate(node_names->node_count * sizeof(char *))); | ||
| for (size_t i = 0; i < participant_names.size(); ++i) { | ||
| node_names->names[i] = __local_strdup(participant_names[i].c_str()); |
There was a problem hiding this comment.
It seems that the __local_strdup function bypasses the rmw allocator.
There was a problem hiding this comment.
not sure I understand.
Should we replace strdup with rmw_allocate... and memcpy/strcpy ?
There was a problem hiding this comment.
The assumption is that rmw_allocate should be able to provide a custom strategy (e.g. use memory from a preallocated pool to avoid any allocation at runtime). If any code directly allocates memory (like strdup does) it undermines that approach. E.g. this would immediately break any attempt to achieve realtime.
Therefore, yes, I think this should use rmw_allocate and memcpy.
Connect to ros2/cli_tools#1