[PATCH xorg-gtest 12/16] process: add Start(program, vector<char*>)

[Chase Douglas]

On 07/02/2012 11:44 PM, Peter Hutterer wrote:
> Same thing as the va_list but takes a std::vector of arguments instead.
>
> Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
> ---
>   include/xorg/gtest/xorg-gtest-process.h |   16 ++++++++++++++++
>   src/process.cpp                         |   20 +++++++++++++-------
>   2 files changed, 29 insertions(+), 7 deletions(-)
>
> diff --git a/include/xorg/gtest/xorg-gtest-process.h b/include/xorg/gtest/xorg-gtest-process.h
> index 47aefcf..c41cd9e 100644
> --- a/include/xorg/gtest/xorg-gtest-process.h
> +++ b/include/xorg/gtest/xorg-gtest-process.h
> @@ -93,6 +93,22 @@ class Process {
>     /**
>      * Starts a program as a child process.
>      *
> +   * See 'man execvp' for further information on the elements in
> +   * the vector.
> +   *
> +   * @param program The program to start.
> +   * @param args Vector of arguments passed to the program.
> +   *
> +   * @throws std::runtime_error on failure.
> +   *
> +   * @post If successful: Child process forked and program started.
> +   * @post If successful: Subsequent calls to Pid() return child process pid.
> +   */
> +  void Start(const std::string& program, std::vector<char*> &args);

I don't like exporting a public API that is bad merely because it makes 
the implementation easier. In this case, one would expect that we pass 
in a vector of std::string arguments. Requiring the user to manually 
allocate c strings is a pain, especially if you want to do it in an 
exception-safe manner. Let's use a vector of strings here and do 
whatever we need to in the implementation to make it work.

> +
> +  /**
> +   * Starts a program as a child process.
> +   *
>      * See 'man execvp' for further information on the variadic argument list.
>      *
>      * @param program The program to start.
> diff --git a/src/process.cpp b/src/process.cpp
> index e79b694..4c4a738 100644
> --- a/src/process.cpp
> +++ b/src/process.cpp
> @@ -48,7 +48,7 @@ xorg::testing::Process::Process() : d_(new Private) {
>     d_->pid = -1;
>   }
>
> -void xorg::testing::Process::Start(const std::string& program, va_list args) {
> +void xorg::testing::Process::Start(const std::string &program, std::vector<char*> &argv) {
>     if (d_->pid != -1)
>       throw std::runtime_error("Attempting to start an already started process");
>
> @@ -61,18 +61,24 @@ void xorg::testing::Process::Start(const std::string& program, va_list args) {
>       close(1);
>       close(2);
>
> -    std::vector<char*> argv;
> -
> -    do
> -      argv.push_back(va_arg(args, char*));
> -    while (argv.back());
> -
> +    if (argv.back())
> +      argv.push_back(NULL);
>       execvp(program.c_str(), &argv[0]);
>
>       throw std::runtime_error("Failed to start process");
>     }
>   }
>
> +void xorg::testing::Process::Start(const std::string& program, va_list args) {
> +  std::vector<char*> argv;
> +
> +  do {
> +    argv.push_back(va_arg(args, char*));
> +  } while (argv.back());
> +
> +  Start(program, argv);
> +}
> +
>   void xorg::testing::Process::Start(const std::string& program, ...) {
>     va_list list;
>     va_start(list, program);
>