[PATCH xorg-gtest 12/16] process: add Start(program, vectorchar*)

2012-07-03 Thread Peter Hutterer
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::vectorchar* args);
+
+  /**
+   * 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::vectorchar* 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::vectorchar* 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::vectorchar* 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);
-- 
1.7.10.4

___
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


Re: [PATCH xorg-gtest 12/16] process: add Start(program, vectorchar*)

2012-07-03 Thread 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.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::vectorchar* 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::vectorchar* 
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::vectorchar* 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::vectorchar* 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