Some communication unit testing failed with older C++ compilers.

[Terae]

Describe the bug The unit testing file /src/test/unit-communication.cpp fails on the section Serial_Protocols::UDP while creating a new Communication::protocol_udp.

To Reproduce Steps to reproduce the behavior:

1. Go to /build
2. Run cmake ..
3. Run make -j 4 unit_testing_communication
4. Launch /build/test/unit_testing_communication
5. Eventually see error

Expected behavior A green message All tests passed

Desktop (please complete the following information):

• OS: Ubuntu
• Version 16.04
• Executable that fail: unit_testing_communication
• C++ compiler: G++ 7.2

Additional context This bug appears only on @BynaryCobweb desktop, but all tests passed on mine (G++ version: 8.2.1).

On the file /src/robot/Communication/Protocol.h, line 161, few unit tests pass by replacing:

SerialProtocol(std::string address, uint16_t local_port, uint16_t remote_port)
        : AbstractSerialProtocol(std::make_shared<UDP>(std::move(address), local_port, remote_port)) {}

by:

SerialProtocol(std::string address, uint16_t local_port, uint16_t remote_port)
        : AbstractSerialProtocol(std::shared_ptr<UDP>(new UDP(std::move(address), local_port, remote_port))) {}

[gbip]

If think that make_shared should be preferred other calling new. See this StackOverflow answer for example.

For the bug side of thing, maybe that it is not the same constructor that is invoked for both call ?

Maybe that there is some undefined behavior (UB) hiding somewhere that blows up in the first case ?

Have you tried to run UB detection tools, address sanitizers and static analyzer on the current codebase ?

[Terae]

This bug is not from a compiler's version issue: all tests passed on different computers with both G++7.1 and G++8.2 with no problem.

It seems to be a distro problem because only computers under Ubuntu 16.04 are not able to pass the tests at the construction of a protocol_udp object. However, their usage is working outside of the tests so it is maybe the combination of catch and Ubuntu which failed in this precise case; I don't have any explication about it.

Because I cannot reproduce it in my side, I won't work on this bug, and if effectively it does not represent an issue to use it outside of tests maybe we could suspend this issue.

Btw, I ran clang-tidy, static and dynamic problems on this code code and no problems was found.