On Wed, Jul 11, 2012 at 11:45:59AM -0700, Chase Douglas wrote:
> On 07/10/2012 08:28 PM, Peter Hutterer wrote:
> >And provide a SetOption call for the various commandline options that the
> >server may take.
> >
> >Signed-off-by: Peter Hutterer <peter.hutte...@who-t.net>
> >---
> >Changes to v1:
> >- drop separate SetConfigPath(), SetLogfilePath()
> >
> > include/xorg/gtest/xorg-gtest-xserver.h | 16 ++++++++++++++++
> > src/environment.cpp | 2 ++
> > src/xserver.cpp | 27 +++++++++++++++++++++++++++
> > 3 files changed, 45 insertions(+)
> >
> >diff --git a/include/xorg/gtest/xorg-gtest-xserver.h
> >b/include/xorg/gtest/xorg-gtest-xserver.h
> >index 56dacf6..58fdc53 100644
> >--- a/include/xorg/gtest/xorg-gtest-xserver.h
> >+++ b/include/xorg/gtest/xorg-gtest-xserver.h
> >@@ -56,6 +56,11 @@ class XServer : public xorg::testing::Process {
> > void SetDisplayNumber(unsigned int display_number);
> >
> > /**
> >+ * @param [in] path_to_server The path to the binary
> >+ */
>
> Missing method documentation.
>
> >+ void SetServerPath(std::string &path_to_server);
>
> The argument should be const.
>
> >+
> >+ /**
> > * Get the display number from this server. If the server was not
> > * started yet, this function returns the display number the server
> > will
> > * be started on.
> >@@ -73,6 +78,17 @@ class XServer : public xorg::testing::Process {
> > const std::string& GetDisplayString(void);
> >
> > /**
> >+ * Set startup options for the server.
> >+ *
> >+ * For arguments that do not take/need a value, use the empty string as
> >+ * value.
> >+ *
> >+ * @param [in] key Commandline option
> >+ * @param [in] value Option value (if any)
> >+ */
> >+ void SetOption(std::string key, std::string value);
>
> Pass in a const reference in order to make sure we aren't
> unnecessarily copying a string just to get it into the method:
>
> void SetOption(const std::string& key, const std::string& value);
hah. the const did the trick, thanks. I had tried std::string& before but
then you can't just pass in a compile-time string, so the call to
option["-logfile"] = "foo" didn't work.
Cheers,
Peter
> >+
> >+ /**
> > * Wait for a specific device to be added to the server.
> > *
> > * @param [in] display The X display connection
> >diff --git a/src/environment.cpp b/src/environment.cpp
> >index 9a524ab..2f1c4e2 100644
> >--- a/src/environment.cpp
> >+++ b/src/environment.cpp
> >@@ -142,6 +142,8 @@ void xorg::testing::Environment::SetUp() {
> > }
> >
> > d_->server.SetDisplayNumber(d_->display);
> >+ d_->server.SetOption("-logfile", d_->path_to_log_file);
> >+ d_->server.SetOption("-config", d_->path_to_conf);
> > d_->server.Start(d_->path_to_server, d_->path_to_server.c_str(),
> > display_string,
> > "-logverbose", "10",
> >diff --git a/src/xserver.cpp b/src/xserver.cpp
> >index c24cee0..f36bd10 100644
> >--- a/src/xserver.cpp
> >+++ b/src/xserver.cpp
> >@@ -27,6 +27,7 @@
> >
> > ******************************************************************************/
> >
> > #include "xorg/gtest/xorg-gtest-xserver.h"
> >+#include "defines.h"
> >
> > #include <sys/types.h>
> > #include <sys/wait.h>
> >@@ -40,12 +41,30 @@
> > #include <cstring>
> > #include <stdexcept>
> > #include <vector>
> >+#include <map>
> >
> > #include <X11/extensions/XInput2.h>
> >
> > struct xorg::testing::XServer::Private {
> >+ Private()
> >+ : display_number(DEFAULT_DISPLAY),
> >+ display_string(),
>
> This ^^ isn't needed. The default constructor will be called if
> unspecified, which initializes to an empty string.
>
> >+ path_to_server(DEFAULT_XORG_SERVER) {
> >+
> >+ std::string key, value;
> >+ key = std::string("-config");
> >+ value = std::string(DUMMY_CONF_PATH);
> >+ options[key] = value;
> >+
> >+ key = std::string("-logfile");
> >+ value = std::string(DEFAULT_XORG_LOGFILE);
> >+ options[key] = value;
>
> The above can be simplified down to:
>
> options["-config"] = DUMMY_CONF_PATH;
> options["-logfile"] = DEFAULT_XORG_LOGFILE;
>
> >+ }
> >+
> > unsigned int display_number;
> > std::string display_string;
> >+ std::string path_to_server;
> >+ std::map<std::string, std::string> options;
> > };
> >
> > xorg::testing::XServer::XServer() : d_(new Private) {
> >@@ -68,6 +87,10 @@ const std::string&
> >xorg::testing::XServer::GetDisplayString(void) {
> > return d_->display_string;
> > }
> >
> >+void xorg::testing::XServer::SetServerPath(std::string &path_to_server) {
> >+ d_->path_to_server = path_to_server;
> >+}
> >+
> > bool xorg::testing::XServer::WaitForEvent(::Display *display, time_t
> > timeout)
> > {
> > fd_set fds;
> >@@ -194,3 +217,7 @@ bool xorg::testing::XServer::WaitForDevice(::Display
> >*display, const std::string
> >
> > return false;
> > }
> >+
> >+void xorg::testing::XServer::SetOption(std::string key, std::string value) {
> >+ d_->options[key] = value;
> >+}
> >
>
> With the above fixes:
>
> Reviewed-by: Chase Douglas <chase.doug...@canonical.com>
_______________________________________________
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
