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.hutte...@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);


_______________________________________________
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Reply via email to