On Tue, Nov 6, 2012 at 7:57 PM, Peter Hutterer <span dir="ltr"><<a href="mailto:peter.hutterer@who-t.net" target="_blank">peter.hutterer@who-t.net</a>></span> wrote:<br><div class="gmail_extra"><div class="gmail_quote">
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Xlib's default error handler prints the error and calls exit(1). Tests that<br>
accidentally trigger an error thus quit without cleaning up properly.<br>
<br>
Install a default error handler that prints the basic info and continue with<br>
the test. Clients that expect to trigger errors should set a custom error<br>
handler.<br>
<br>
Signed-off-by: Peter Hutterer <<a href="mailto:peter.hutterer@who-t.net">peter.hutterer@who-t.net</a>><br>
---<br>
 include/xorg/gtest/xorg-gtest-xserver.h | 13 ++++++<br>
 src/xserver.cpp                         | 54 +++++++++++++++++++++++-<br>
 test/xserver-test.cpp                   | 73 +++++++++++++++++++++++++++++++++<br>
 3 files changed, 139 insertions(+), 1 deletion(-)<br>
<br>
diff --git a/include/xorg/gtest/xorg-gtest-xserver.h b/include/xorg/gtest/xorg-gtest-xserver.h<br>
index 8bf7996..11fc93d 100644<br>
--- a/include/xorg/gtest/xorg-gtest-xserver.h<br>
+++ b/include/xorg/gtest/xorg-gtest-xserver.h<br>
@@ -265,6 +265,19 @@ class XServer : public xorg::testing::Process {<br>
      */<br>
     static void RegisterXIOErrorHandler();<br>
<br>
+    /**<br>
+     * Install a default XErrorHandler. That error handler will cause a test<br>
+     * failure if called.<br>
+     *<br>
+     * This function is called automatically by XServer::Start(). Usually,<br>
+     * you will not need to call this function unless your test does not<br>
+     * instantiate and Start() an XServer object.<br>
+     *<br>
+     * This function will only install a new error handler if the currently<br>
+     * installed XErrorHandler is not the default handler used by Xlib.<br>
+     */<br>
+    static void RegisterXErrorHandler();<br>
+<br>
   private:<br>
     struct Private;<br>
     std::auto_ptr<Private> d_;<br>
diff --git a/src/xserver.cpp b/src/xserver.cpp<br>
index ad018a1..4faa8e9 100644<br>
--- a/src/xserver.cpp<br>
+++ b/src/xserver.cpp<br>
@@ -394,6 +394,40 @@ const std::string& xorg::testing::XServer::GetVersion(void) {<br>
   return d_->version;<br>
 }<br>
<br>
+static int _x_error_handler(Display *dpy, XErrorEvent *err)<br>
+{<br>
+  std::stringstream error;<br>
+  switch(err->error_code) {<br>
+    case BadRequest: error << "BadRequest"; break;<br>
+    case BadValue: error << "BadValue"; break;<br>
+    case BadWindow: error << "BadWindow"; break;<br>
+    case BadPixmap: error << "BadPixmap"; break;<br>
+    case BadAtom: error << "BadAtom"; break;<br>
+    case BadCursor: error << "BadCursor"; break;<br>
+    case BadFont: error << "BadFont"; break;<br>
+    case BadMatch: error << "BadMatch"; break;<br>
+    case BadDrawable: error << "BadDrawable"; break;<br>
+    case BadAccess: error << "BadAccess"; break;<br>
+    case BadAlloc: error << "BadAlloc"; break;<br>
+    case BadColor: error << "BadColor"; break;<br>
+    case BadGC: error << "BadGC"; break;<br>
+    case BadIDChoice: error << "BadIDChoice"; break;<br>
+    case BadName: error << "BadName"; break;<br>
+    case BadLength: error << "BadLength"; break;<br>
+    case BadImplementation: error << "BadImplementation"; break;<br>
+    default:<br>
+      error << err->error_code;<br>
+      break;<br>
+  }<br>
+<br>
+  ADD_FAILURE() << "XError received: " << error.str() << ", request " <<<br>
+    (int)err->request_code << "(" << (int)err->minor_code << "), detail: "<br>
+    << err->resourceid << "\nThis error handler is likely to be triggered "<br>
+    "more than once.\nCheck the first error for the real error";<br>
+  return 0;<br>
+}<br>
+<br>
+<br>
 static int _x_io_error_handler(Display *dpy) _X_NORETURN;<br>
 static int _x_io_error_handler(Display *dpy)<br>
 {<br>
@@ -409,6 +443,15 @@ void xorg::testing::XServer::RegisterXIOErrorHandler()<br>
     XSetIOErrorHandler(old_handler);<br>
 }<br>
<br>
+void xorg::testing::XServer::RegisterXErrorHandler()<br>
+{<br>
+  XErrorHandler old_handler;<br>
+  old_handler = XSetErrorHandler(_x_error_handler);<br>
+<br>
+  if (old_handler != _XDefaultError)<br>
+    XSetErrorHandler(old_handler);<br>
+}<br>
+<br>
 void xorg::testing::XServer::Start(const std::string &program) {<br>
   TestStartup();<br>
<br>
@@ -464,7 +507,15 @@ void xorg::testing::XServer::Start(const std::string &program) {<br>
         args.push_back(it->second);<br>
     }<br>
<br>
-    Process::Start(program.empty() ? d_->path_to_server : program, args);<br>
+    std::string server_binary = program.empty() ? d_->path_to_server : program;<br>
+<br>
+    if (getenv("XORG_GTEST_XSERVER_USE_VALGRIND")) {<br>
+      args.insert(args.begin(), server_binary);<br>
+      server_binary = "valgrind";<br>
+      args.insert(args.begin(), "--leak-check=full");<br>
+    }<br></blockquote><div><br></div><div>While this looks totally cool, it probably belongs in a separate commit :).</div><div><br></div><div>Without this hunk:</div><div><br></div><div>Reviewed-by: Chase Douglas <<a href="mailto:chase.douglas@ubuntu.com">chase.douglas@ubuntu.com</a>></div>
<div><br></div><div>You can also add my Reviewed-by tag to a separate commit for just this hunk. You might want to consider allowing for some standard options like --show-reachable</div><div><br></div><div>-- Chase</div></div>
</div>