[PATCH xorg-gtest v2 7/8] environment: remove default settings

Peter Hutterer peter.hutterer at who-t.net
Wed Jul 11 16:26:36 PDT 2012


On Wed, Jul 11, 2012 at 12:24:18PM -0700, Chase Douglas wrote:
> On 07/10/2012 08:28 PM, Peter Hutterer wrote:
> >Keep those in the server only, not the environment. And only override the
> >build-in ones when they've been set by main.
> 
> I'm a little confused by this patch. When I read "Keep those in the
> server only, not the environment," I expect that the members like
> path_to_conf are removed altogether from Environment::Private in
> lieu of just the XServer object that is initialized properly. For
> example, the constructor for the private struct would be:
> 
> Private()
> {
>     server.SetOption("-config", DUMMY_CONF_PATH);
>     server.SetOption("-logfile", DEFAULT_XORG_LOGFILE);
>     server.SetDisplayNumber(DEFAULT_DISPLAY);
> }

this part isn't necessary though - the server class initialises all these
defaults already. with this commit the environment can just call Start() and
everything works as before.

(see "xserver: store config, logfile, binary paths in the XServer object")

> The second part of the commit is: "And only override the build-in
> ones when they've been set by main." The code seems to remove all
> the initialization of the default built-in options, so I'm not sure
> what you are meaning to do here.

xorg-gtest_main.cpp's main takes arguments to set the various server
options. if none of them are given, Environment just calls Start(),
otherwise it'll override the bits that are provided. the rest are still on
the defaults.

Cheers,
  Peter

> >
> >Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
> >---
> >Changes to v1:
> >- uses SetDisplayNumber/GetDisplayString() now
> >
> >  src/environment.cpp |   20 ++++++++------------
> >  1 file changed, 8 insertions(+), 12 deletions(-)
> >
> >diff --git a/src/environment.cpp b/src/environment.cpp
> >index 64907e5..19066bb 100644
> >--- a/src/environment.cpp
> >+++ b/src/environment.cpp
> >@@ -28,7 +28,6 @@
> >  #include "xorg/gtest/xorg-gtest-environment.h"
> >  #include "xorg/gtest/xorg-gtest-process.h"
> >  #include "xorg/gtest/xorg-gtest-xserver.h"
> >-#include "defines.h"
> >
> >  #include <sys/types.h>
> >  #include <unistd.h>
> >@@ -44,11 +43,8 @@
> >  #include <X11/Xlib.h>
> >
> >  struct xorg::testing::Environment::Private {
> >-  Private()
> >-      : path_to_conf(DUMMY_CONF_PATH), path_to_log_file(DEFAULT_XORG_LOGFILE),
> >-        path_to_server(DEFAULT_XORG_SERVER), display(DEFAULT_DISPLAY) {
> >+  Private() : display(-1) {
> >    }
> >-
> >    std::string path_to_conf;
> >    std::string path_to_log_file;
> >    std::string path_to_server;
> >@@ -103,16 +99,16 @@ void xorg::testing::Environment::SetDisplayNumber(int display_num)
> >  }
> >
> >  void xorg::testing::Environment::SetUp() {
> >-  static char display_string[6];
> >-  snprintf(display_string, 6, ":%d", d_->display);
> >-
> >-  d_->server.SetDisplayNumber(d_->display);
> >-  d_->server.SetOption("-logfile", d_->path_to_log_file);
> >-  d_->server.SetOption("-config", d_->path_to_conf);
> >+  if (d_->display >= 0)
> >+    d_->server.SetDisplayNumber(d_->display);
> >+  if (d_->path_to_log_file.length())
> >+    d_->server.SetOption("-logfile", d_->path_to_log_file);
> >+  if (d_->path_to_conf.length())
> >+    d_->server.SetOption("-config", d_->path_to_log_file);
> >    d_->server.Start(d_->path_to_server);
> >    d_->server.WaitForConnections();
> >
> >-  Process::SetEnv("DISPLAY", display_string, true);
> >+  Process::SetEnv("DISPLAY", d_->server.GetDisplayString(), true);
> >  }
> >
> >  void xorg::testing::Environment::TearDown() {
> >
> 


More information about the xorg-devel mailing list