[Dashing] Avoid memory leaks and undefined behavior in rmw_fastrtps_dynamic_cpp typesupport code

[nuclearsandwich]

Backport of #429 for Dashing

[hidmic]

LGTM pending green Dashing CI

On a related note: should we send a notification about this API break?

[nuclearsandwich]

On a related note: should we send a notification about this API break?

Ack. I'll admit to getting ahead of myself here. As I was reviewing the change I saw no mention of public API breaks and assumed that the changed functions were internal to rmw_fastrtps_dynamic_cpp if that isn't the case then it makes zero sense to break API for the final Dashing patch release and this can just be closed.

[clalancette]

Ack. I'll admit to getting ahead of myself here. As I was reviewing the change I saw no mention of public API breaks and assumed that the changed functions were internal to rmw_fastrtps_dynamic_cpp if that isn't the case then it makes zero sense to break API for the final Dashing patch release and this can just be closed.

The include files for rmw_fastrtps_dynamic_cpp are indeed installed into /opt/ros/dashing/include/rmw_fastrtps_dynamic_cpp, so it can be considered "public". The likelihood that someone is directly using them is very close to zero, as it is a) kind of internal rmw implementation details, and b) almost nobody is using fastrtps dynamic. But given that this is the end-of-life of Dashing, I'd err on the side of caution and not put this in.

[hidmic]

I saw no mention of public API breaks and assumed that the changed functions were internal to rmw_fastrtps_dynamic_cpp

Ha, I assumed that the we were backporting anyways because the advantages (i.e. fixing UBs) outweighed the disadvantages (i.e. the infinitesimal probability of breaking downstream code that makes use of this "public" API).

But given that this is the end-of-life of Dashing, I'd err on the side of caution and not put this in.

Yeah, perhaps it's somewhat late. Though I will say that the code this patch replaces was pretty controversial, to say the least.

[nuclearsandwich]

Thanks for the feedback y'all. If there was more clamor for these changes I'd consider it but given that no one is asking for this it makes sense to go out with the API (and bugs) we came in with.