Skip to content

TangoTest::platform::start_server: Prefer std::vector<std::string>

Thomas Ives requested to merge catch2-tidy-up-arg-env-string-ownship into main

std::vector<std::string> makes the ownership of the argument and environment strings clearer. I.e. these arguments are owned by the called and the start_server implementation is responsible for ensuring they are copied to the child process as appropriate. For the unix implementation this copy happens when we call fork() and execv().

Using a std::vector<const char *> leaked details about the unix implementation through this interface and required callers to do an "ownership dance" to make sure the const char * lasted long enough. This ownership dance is confusing and requires a lot of tedious code in the TestServer and Context. Having start_server accept std::vector<std::string> pushes this complexity down to the implementations. Now they are responsible for converting from "C++ types" to their OS interface.

This avoids the need to store m_args/m_env in the TestServer class.

Before m_args contained invalid pointers to instance_name and end_point, introduced in 0adf58a2 (Merge branch 'fix-tests-on-mac' into 'main', 2024-05-15). This hasn't caused an issue yet as m_args was never actually used when these pointers are invalid.

Rather than just fixing the above invalid pointer issue directly, we are fixing the root cause of the problems: Unclear memory ownership with the platform::start_server function.

Merge request reports