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

[Chase Douglas]

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);
}

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.

>
> 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() {
>