memory leak in topic_name of subscription and publisher (at least)

[wjwwood]

See:

• new called here with no matching delete/free() that I can find:

  • rmw_fastrtps/rmw_fastrtps_cpp/src/functions.cpp

    Line 879 in 3a7538a

    rmw_publisher->topic_name = reinterpret_cast<const char *>(new char[strlen(topic_name) + 1]);

• new called here with no matching delete/free() that I can find:

  • rmw_fastrtps/rmw_fastrtps_cpp/src/functions.cpp

    Line 1120 in 3a7538a

    subscription->topic_name = reinterpret_cast<const char *>(new char[strlen(topic_name) + 1]);

Really the whole file should be audited. Also the new can throw a C++ exception that would raise through the rmw C interface which is bad. It should either use malloc or rmw_allocate or the new rcutils_allocator_t or it should try-catch the new.

[allenh1]

@wjwwood I'll take care of this one.

[allenh1]

There's also another problem here, in that valgrind usually complains about a mismatched free() / delete / delete[] somewhere in this file (noticed this while working on the python leaking).

[wjwwood]

There is almost certainly other issues in that file. It wasn't written by us originally, and so hasn't been subject to code review for much of its life. This is issue was about a very particular issue. I don't think you can use valgrind pass/fail as an indicator of whether or not this is fixed.

[allenh1]

I also see a lot of inconsistency wrt the use of nullptr.

[mikaelarguedas]

Yes as @wjwwood mentioned this file has not been written by us, has a lot of inconsistency and issues and should be both audited and refactored. This is out of the scope of this particular issue as pointed out in the previous comment.

[mikaelarguedas]

putting this back in ready with a more generic title to reflect the first comment:

Really the whole file should be audited

EDIT: Actually I'll open another ticket tracking the high level issue

[mikaelarguedas]

closed in favor of #122