Fix memory leak on Windows

[codebot]

Unfortunately Windows throws a compiler warning when it sees strdup(); it wants to see _strdup() instead.

[tfoote]

Might it be better to use malloc and strncpy instead to avoid hidden mallocs: http://stackoverflow.com/a/14021507/604099 It's only one more line of code to explicitly malloc and copy.

[codebot]

I'm not sure what the rest of the Fast-RTPS codebase looks like; I was just trying to get rid of a compiler warning 🐐 how do people normally use TypeSupport::setName() ?

[jacquelinekay]

I haven't mucked around with this part of the codebase very much. @esteve would probably know since he worked a bunch with the TypeSupport to implement C typesupport. (Also, that reminds me--Esteve, you should probably watch the changes we're making to this repo because any changes to files that your pull request moves into a different file will essentially be lost if you don't rebase carefully.)

[codebot]

a ha! great catch Tully. Looks like setName comes from here:

https://github.com/eProsima/Fast-RTPS/blob/master/include/fastrtps/TopicDataType.h

which is looking for a const char * and just passing that to the std::string constructor. So indeed, this would be a memory leak, since setName() isn't taking ownership.

looks to me like we can pass name.c_str() to setName(). Is that how it looks to you as well?

[tfoote]

Yeah, it's just turning it back into a string. Passing the .c_str() to it should be adaquate. There are several other instances following that pattern. At least one in parameters and one in QoS.

[codebot]

I think this is looking OK now. CI results:
Linux: linux: http://ci.ros2.org/job/ci_linux/1145/
OSX: http://ci.ros2.org/job/ci_osx/927/
Windows: http://ci.ros2.org/job/ci_windows/1205/

The Linux and OSX results are clean. The Windows one has various test failures because it ran on Windows 10 on windshield. I'll look into figuring out what's going on with windshield, but I don't think any of those failures are new (for windshield). At least there is one less MSVC warning now, which actually led to getting rid of this memory leak 😮

[dirk-thomas]

The same warning is (twice) in the sibling file ServiceTypeSupport.cpp: http://ci.ros2.org/job/ci_windows/1205/warnings34Result/source.6589365479438003664/#14 http://ci.ros2.org/job/ci_windows/1205/warnings34Result/source.6589365479438003666/#28

[codebot]

@dirk-thomas yeah there are warnings in other files; I was just trying to fix up this file first.

[codebot]

if it's best to couple them together, I can also change ServiceTypeSupport.cpp and squash into this commit. Would that be better?

[dirk-thomas]

Since ServiceTypeSupport.cpp has the exact same code with the exact same warnings as this MessageTypeSupport.cpp I would say they can be fixed together in this single PR.

[codebot]

OK sounds good, thanks. I'll add those changes to this PR.

[codebot]

OK now we're down to only 5 warnings left on windows HOORAY 😀 I'll go after those in a different PR, since the remaining ones are unrelated to these fixes.

CI looks fine to me; all issues seem unrelated to these changes:
linux: http://ci.ros2.org/job/ci_linux/1147/
osx: http://ci.ros2.org/job/ci_osx/930/
windows: http://ci.ros2.org/job/ci_windows/1209/

[wjwwood]

lgtm

[dirk-thomas]

Can this be squashed and merged?

[codebot]

i'll squash it right now