On 07/05/2012 04:27 PM, Peter Hutterer wrote:
On Thu, Jul 05, 2012 at 01:46:42PM -0700, Chase Douglas wrote:
On 07/03/2012 10:34 PM, Peter Hutterer wrote:
On Tue, Jul 03, 2012 at 10:30:28AM -0700, Chase Douglas wrote:
On 07/02/2012 11:44 PM, Peter Hutterer wrote:
Signed-off-by: Peter Hutterer <[email protected]>

It makes sense to move the startup to the XServer object, but the
environment should then contain an XServer object and start it. The
point of the environment is that it starts up an xserver
automatically.

The xorg-gtest user is free to not use the environment. But when
they use the environment, it should start a server.

It still does, it's just cut off from the Environment::SetUp() hunk though.
see below

---
  include/xorg/gtest/xorg-gtest-xserver.h |    2 ++
  src/environment.cpp                     |   35 ---------------------------
  src/xserver.cpp                         |   40 +++++++++++++++++++++++++++++++
  3 files changed, 42 insertions(+), 35 deletions(-)

diff --git a/include/xorg/gtest/xorg-gtest-xserver.h 
b/include/xorg/gtest/xorg-gtest-xserver.h
index 5c5ce99..52a2fd0 100644
--- a/include/xorg/gtest/xorg-gtest-xserver.h
+++ b/include/xorg/gtest/xorg-gtest-xserver.h
@@ -129,6 +129,8 @@ class XServer : public xorg::testing::Process {
      XServer(const XServer&);
      XServer& operator=(const XServer&);

+    void TestStartup(void);
+
  };
  } // namespace testing
  } // namespace xorg
diff --git a/src/environment.cpp b/src/environment.cpp
index 7ed23b3..69972a4 100644
--- a/src/environment.cpp
+++ b/src/environment.cpp
@@ -106,41 +106,6 @@ void xorg::testing::Environment::SetUp() {
    static char display_string[6];
    snprintf(display_string, 6, ":%d", d_->display);

-  Display* test_display = XOpenDisplay(display_string);
-  if (test_display) {
-    XCloseDisplay(test_display);
-    std::string message;
-    message += "A server is already running on ";
-    message += display_string;
-    message += ".";
-    throw std::runtime_error(message);
-  }
-
-  /* The Xorg server won't start unless the log file and the old log file are
-   * writable. */
-  std::ofstream log_test;
-  log_test.open(d_->path_to_log_file.c_str(), std::ofstream::out);
-  log_test.close();
-  if (log_test.fail()) {
-    std::string message;
-    message += "X.org server log file ";
-    message += d_->path_to_log_file;
-    message += " is not writable.";
-    throw std::runtime_error(message);
-  }
-
-  std::string old_log_file = d_->path_to_log_file.c_str();
-  old_log_file += ".old";
-  log_test.open(old_log_file.c_str(), std::ofstream::out);
-  log_test.close();
-  if (log_test.fail()) {
-    std::string message;
-    message += "X.org old server log file ";
-    message += old_log_file;
-    message += " is not writable.";
-    throw std::runtime_error(message);
-  }
-
    d_->server.SetDisplayNumber(d_->display);
    d_->server.SetLogfilePath(d_->path_to_log_file);
    d_->server.SetConfigPath(d_->path_to_conf);

is followed by
     d_->server.Start(d_->path_to_server);
     d_->server.WaitForConnections();

So nothing should change over the previous code. I haven't modified the
xi2.cpp code in the server which uses this environment and it still works.

Ahh, right, the code still starts the server, but shouldn't we be
calling the server TestStartup() method before? As it is in this
patch, we've removed a bunch of code, moved it to a method of a
different object, and haven't called the method to ensure the same
code is executed. Unless I'm misreading again :).

hard to see from the patch I guess, but the new sequence is
     Environment::SetUp()
        ...
        XServer::Start()
             XServer::TestStartup()
             Process::Start()

Ok, that looks good. I guess some of the context is just getting strewn across a couple different patches.

-- Chase
_______________________________________________
[email protected]: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Reply via email to