RE: Xvfb and xkb

[Ashwin Chandra]

After more searching on the web, a lot of people say the Xvfb and keymaps
just don't work correctly (at least in older versions).

Can anyone tell me if they have actually succeeded in getting Xvfb and
setxkbmap working ?

-Original Message-
From: Peter Hutterer [mailto:peter.hutte...@who-t.net]
Sent: Wednesday, August 15, 2012 11:27 PM
To: Ashwin Chandra
Cc: xorg@lists.x.org
Subject: Re: Xvfb and xkb

On Wed, Aug 15, 2012 at 10:20:13PM -0700, Ashwin Chandra wrote:
 Thank you for responding Peter.

 I am not sure what you mean by calling setxkbmap first?

 Even if I embed the keymap to use in the InitKeyboardDeviceStruct() in
 the Xvfb initiation process, same problem.

oh, that answers the question then. servers regenerate when the last client
disconnects, so if you do the following, the keymap is reset before anyone
can use it:

Xvfb 
setxkbmap -layout de
/* last (only) client disconnects, server regens with defaults */ xterm /*
has the default keymap now */

 I wanted to know if there was a way to debug this and see exactly
 which keymap file in a certain directory is being used? It looks like
 to me just the keymap is somehow wrong.

tbh, shoft of strace or gdb, I don't know

Cheers,
   Peter

 -Original Message-
 From: Peter Hutterer [mailto:peter.hutte...@who-t.net]
 Sent: Wednesday, August 15, 2012 10:12 PM
 To: Ashwin Chandra
 Cc: xorg@lists.x.org
 Subject: Re: Xvfb and xkb

 On Fri, Aug 10, 2012 at 08:42:51AM -0700, Ashwin Chandra wrote:
  I am using Xvfb to render an X session in memory. I am trying to use
  setxkbmap to change the key layouts. I seem to have difficulty doing
  this other than US key layout. There are no errors when calling
  “setxkbmap de”
  (german language) however the keymap looks all wrong and certain
  modifier keys like alt-gr don’t seem to work. I thought maybe I
  don’t have the right keymaps compiled in but I did a diff of
  /usr/share/X11/xkb with a working machine and it is the same. So I
  need some help figuring out how to debug this problem. Setxkbmap
  doesn’t seem to have much info except the verbose options telling me
  what it is setting.

 stab in the dark: you're not by any chance calling setxkbmap first and
 then other clients. If so, this would cause server regeneration and
 re-set anything done before.


___
xorg@lists.x.org: X.Org support
Archives: http://lists.freedesktop.org/archives/xorg
Info: http://lists.x.org/mailman/listinfo/xorg
Your subscription address: arch...@mail-archive.com

Re: Xvfb and xkb

[Alan Coopersmith]

On 08/16/12 09:14 AM, Ashwin Chandra wrote:

After more searching on the web, a lot of people say the Xvfb and keymaps
just don't work correctly (at least in older versions).

Can anyone tell me if they have actually succeeded in getting Xvfb and
setxkbmap working ?


Since Xvfb has no keyboard devices to get input from, how would you tell
and why would you care?

--
-Alan Coopersmith-alan.coopersm...@oracle.com
 Oracle Solaris Platform Engineering: X Window System
___
xorg@lists.x.org: X.Org support
Archives: http://lists.freedesktop.org/archives/xorg
Info: http://lists.x.org/mailman/listinfo/xorg
Your subscription address: arch...@mail-archive.com

Re: Xvfb and xkb

[Peter Hutterer]

On Thu, Aug 16, 2012 at 09:18:55AM -0700, Alan Coopersmith wrote:
 On 08/16/12 09:14 AM, Ashwin Chandra wrote:
 After more searching on the web, a lot of people say the Xvfb and keymaps
 just don't work correctly (at least in older versions).
 
 Can anyone tell me if they have actually succeeded in getting Xvfb and
 setxkbmap working ?
 
 Since Xvfb has no keyboard devices to get input from, how would you tell
 and why would you care?

XTest should still work? could be a use-case, though I don't feel creative
enough right now to think of one :)

Cheers,
   Peter

___
xorg@lists.x.org: X.Org support
Archives: http://lists.freedesktop.org/archives/xorg
Info: http://lists.x.org/mailman/listinfo/xorg
Your subscription address: arch...@mail-archive.com

X.Org Book Sprint 2012 (guide to writing graphics drivers)

[Matt Dew]

(Cross post. Yes, I know this is short notice and a short window to 
reply.   My apologies on that.)


X.Org Book Sprint 2012
Monday Sept 17  Tuesday Sept 18.
Nürnberg (Nuremberg), Germany.

   The X.Org Consortium will hold a book sprint on the Monday and
Tuesday before the Developers Conference in Nürnberg Germany.
During these two days attendees will focus on a book for graphics
drivers development.  The book will be targeted towards new
contributors but will have enough details to be useful to all
levels of graphics driver development.  Anyone interested in
helping out and contributing to the book sprint is encouraged to
get involved.


From http://www.booksprints.net/about/

A Book Sprint brings together a group to produce a book in 3-5 days.
There is no pre-production and the group is guided by a facilitator
from zero to published book. The books produced are high quality
content and are made available immediately at the end of the sprint
via print-on-demand services and e-book formats.

Our book sprint will be slightly different than that.  Our book sprint
is only two days and we are not starting from complete scratch. We are
starting with Marcheu's existing guide, located at:
http://people.freedesktop.org/~marcheu/linuxgraphicsdrivers.pdf

For more information or if you would like to participate in the book
sprint, please contact:
board at foundation.x.org
marcoz at osource.org

Please contact us on or before Mon Aug 20, 2012.

More information can be found at:
http://www.x.org/wiki/Events/BookSprint2012
___
xorg@lists.x.org: X.Org support
Archives: http://lists.freedesktop.org/archives/xorg
Info: http://lists.x.org/mailman/listinfo/xorg
Your subscription address: arch...@mail-archive.com

[PATCH xorg-gtest 01/11] Add Red Hat copyright notices

[Peter Hutterer]

Signed-off-by: Peter Hutterer peter.hutte...@who-t.net
---
 COPYING  | 1 +
 include/xorg/gtest/evemu/xorg-gtest-device.h | 1 +
 include/xorg/gtest/xorg-gtest-environment.h  | 1 +
 include/xorg/gtest/xorg-gtest-process.h  | 1 +
 include/xorg/gtest/xorg-gtest-test.h | 1 +
 src/device.cpp   | 1 +
 src/environment.cpp  | 1 +
 src/test.cpp | 1 +
 8 files changed, 8 insertions(+)

diff --git a/COPYING b/COPYING
index 4da49fe..2e36349 100644
--- a/COPYING
+++ b/COPYING
@@ -1,4 +1,5 @@
 Copyright (C) 2011, 2012 Canonical, Ltd.
+Copyright © 2012 Red Hat, Inc.
 
 Permission is hereby granted, free of charge, to any person obtaining a copy of
 this software and associated documentation files (the Software), to deal in
diff --git a/include/xorg/gtest/evemu/xorg-gtest-device.h 
b/include/xorg/gtest/evemu/xorg-gtest-device.h
index 223738e..1a0e5ee 100644
--- a/include/xorg/gtest/evemu/xorg-gtest-device.h
+++ b/include/xorg/gtest/evemu/xorg-gtest-device.h
@@ -3,6 +3,7 @@
  * X testing environment - Google Test environment feat. dummy x server
  *
  * Copyright (C) 2012 Canonical Ltd.
+ * Copyright © 2012 Red Hat, Inc.
  *
  * Permission is hereby granted, free of charge, to any person obtaining a copy
  * of this software and associated documentation files (the Software), to 
deal
diff --git a/include/xorg/gtest/xorg-gtest-environment.h 
b/include/xorg/gtest/xorg-gtest-environment.h
index aee9cdd..b2c2be7 100644
--- a/include/xorg/gtest/xorg-gtest-environment.h
+++ b/include/xorg/gtest/xorg-gtest-environment.h
@@ -3,6 +3,7 @@
  * X testing environment - Google Test environment feat. dummy x server
  *
  * Copyright (C) 2011, 2012 Canonical Ltd.
+ * Copyright © 2012 Red Hat, Inc.
  *
  * Permission is hereby granted, free of charge, to any person obtaining a copy
  * of this software and associated documentation files (the Software), to 
deal
diff --git a/include/xorg/gtest/xorg-gtest-process.h 
b/include/xorg/gtest/xorg-gtest-process.h
index 7d6ab62..48f387b 100644
--- a/include/xorg/gtest/xorg-gtest-process.h
+++ b/include/xorg/gtest/xorg-gtest-process.h
@@ -3,6 +3,7 @@
  * X testing environment - Google Test environment feat. dummy x server
  *
  * Copyright (C) 2011, 2012 Canonical Ltd.
+ * Copyright © 2012 Red Hat, Inc.
  *
  * Permission is hereby granted, free of charge, to any person obtaining a copy
  * of this software and associated documentation files (the Software), to 
deal
diff --git a/include/xorg/gtest/xorg-gtest-test.h 
b/include/xorg/gtest/xorg-gtest-test.h
index a776693..133f21b 100644
--- a/include/xorg/gtest/xorg-gtest-test.h
+++ b/include/xorg/gtest/xorg-gtest-test.h
@@ -3,6 +3,7 @@
  * X testing environment - Google Test environment feat. dummy x server
  *
  * Copyright (C) 2011, 2012 Canonical Ltd.
+ * Copyright © 2012 Red Hat, Inc.
  *
  * Permission is hereby granted, free of charge, to any person obtaining a copy
  * of this software and associated documentation files (the Software), to 
deal
diff --git a/src/device.cpp b/src/device.cpp
index 1f1c94b..884ba3b 100644
--- a/src/device.cpp
+++ b/src/device.cpp
@@ -3,6 +3,7 @@
  * X testing environment - Google Test environment feat. dummy x server
  *
  * Copyright (C) 2012 Canonical Ltd.
+ * Copyright © 2012 Red Hat, Inc.
  *
  * Permission is hereby granted, free of charge, to any person obtaining a copy
  * of this software and associated documentation files (the Software), to 
deal
diff --git a/src/environment.cpp b/src/environment.cpp
index 9b3ab90..54eb6b6 100644
--- a/src/environment.cpp
+++ b/src/environment.cpp
@@ -3,6 +3,7 @@
  * X testing environment - Google Test environment feat. dummy x server
  *
  * Copyright (C) 2011, 2012 Canonical Ltd.
+ * Copyright © 2012 Red Hat, Inc.
  *
  * Permission is hereby granted, free of charge, to any person obtaining a copy
  * of this software and associated documentation files (the Software), to 
deal
diff --git a/src/test.cpp b/src/test.cpp
index 94adf13..c7b69bc 100644
--- a/src/test.cpp
+++ b/src/test.cpp
@@ -3,6 +3,7 @@
  * X testing environment - Google Test environment feat. dummy x server
  *
  * Copyright (C) 2011, 2012 Canonical Ltd.
+ * Copyright © 2012 Red Hat, Inc.
  *
  * Permission is hereby granted, free of charge, to any person obtaining a copy
  * of this software and associated documentation files (the Software), to 
deal
-- 
1.7.11.2

___
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

[PATCH xorg-gtest 02/11] process: add XORG_GTEST_CHILD_STDOUT environment variable

[Peter Hutterer]

If this variable is set, leave stdout and stderr open for the child process
to see potential error messages thrown by that child.

Signed-off-by: Peter Hutterer peter.hutte...@who-t.net
---
 README  | 3 +++
 src/process.cpp | 6 --
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/README b/README
index 33e1fe3..bda4b66 100644
--- a/README
+++ b/README
@@ -79,3 +79,6 @@ Environment variables
 -
 XORG_GTEST_XSERVER_SIGSTOP
   If set, an XServer object will raise a SIGSTOP signal after startup.
+XORG_GTEST_CHILD_STDOUT
+  If set to any value, Process::Start() will _not_ close stdout/stdin/stderr
+  for the forked child.
diff --git a/src/process.cpp b/src/process.cpp
index 7df2b84..03dbc42 100644
--- a/src/process.cpp
+++ b/src/process.cpp
@@ -58,8 +58,10 @@ void xorg::testing::Process::Start(const std::string 
program, const std::vector
 throw std::runtime_error(Failed to fork child process);
   } else if (d_-pid == 0) { /* Child */
 close(0);
-close(1);
-close(2);
+if (getenv(XORG_GTEST_CHILD_STDOUT) == NULL) {
+  close(1);
+  close(2);
+}
 
 std::vectorchar* args;
 std::vectorstd::string::const_iterator it;
-- 
1.7.11.2

___
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

[PATCH xorg-gtest 03/11] xserver: use temporary variable for log file too

[Peter Hutterer]

And use + for string concatination, the code as-is was a legacy from const
char* handling.

Signed-off-by: Peter Hutterer peter.hutte...@who-t.net
---
 src/xserver.cpp | 17 -
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/src/xserver.cpp b/src/xserver.cpp
index 86c8484..eb64d3b 100644
--- a/src/xserver.cpp
+++ b/src/xserver.cpp
@@ -266,28 +266,27 @@ void xorg::testing::XServer::TestStartup(void) {
 throw std::runtime_error(message);
   }
 
+  std::string log = d_-options[-logfile];
+
   /* 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_-options[-logfile].c_str(), std::ofstream::out);
+  log_test.open(log.c_str(), std::ofstream::out);
   log_test.close();
   if (log_test.fail()) {
 std::string message;
-message += X.org server log file ;
-message += d_-options[-logfile];
-message +=  is not writable.;
+message += X.org server log file  + log +  is not writable.;
 throw std::runtime_error(message);
   }
 
-  std::string old_log_file = d_-options[-logfile];
-  old_log_file += .old;
+  std::string old_log_file = log + .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.;
+message += X.org old server log file  + old_log_file +  is not 
writable.;
 throw std::runtime_error(message);
   }
 
-- 
1.7.11.2

___
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

[PATCH xorg-gtest 04/11] xserver: if the logfile was created by TestStartup, remove it again

[Peter Hutterer]

If the file didn't exist and we created it by checking whether it is
writeable, the file needs to be removed again. Otherwise we have empty
logfiles lying around if the server fails to start.

Signed-off-by: Peter Hutterer peter.hutte...@who-t.net
---
 src/xserver.cpp | 14 +++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/src/xserver.cpp b/src/xserver.cpp
index eb64d3b..0d2ace1 100644
--- a/src/xserver.cpp
+++ b/src/xserver.cpp
@@ -270,6 +270,11 @@ void xorg::testing::XServer::TestStartup(void) {
 
   /* The Xorg server won't start unless the log file and the old log file are
* writable. */
+  bool logfile_was_present;
+  std::ifstream file_test;
+  file_test.open(log.c_str());
+  logfile_was_present = file_test.good();
+
   std::ofstream log_test;
   log_test.open(log.c_str(), std::ofstream::out);
   log_test.close();
@@ -277,10 +282,13 @@ void xorg::testing::XServer::TestStartup(void) {
 std::string message;
 message += X.org server log file  + log +  is not writable.;
 throw std::runtime_error(message);
-  }
+  } else if (!logfile_was_present)
+unlink(log.c_str());
 
   std::string old_log_file = log + .old;
 
+  file_test.open(old_log_file.c_str());
+  logfile_was_present = file_test.good();
 
   log_test.open(old_log_file.c_str(), std::ofstream::out);
   log_test.close();
@@ -288,8 +296,8 @@ void xorg::testing::XServer::TestStartup(void) {
 std::string message;
 message += X.org old server log file  + old_log_file +  is not 
writable.;
 throw std::runtime_error(message);
-  }
-
+  } else if (!logfile_was_present)
+unlink(old_log_file.c_str());
 }
 
 const std::string xorg::testing::XServer::GetVersion(void) {
-- 
1.7.11.2

___
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

[PATCH xorg-gtest 05/11] xserver: don't push the program name into the argument list

[Peter Hutterer]

Process:Start() does this for us.

Signed-off-by: Peter Hutterer peter.hutte...@who-t.net
---
 src/xserver.cpp | 1 -
 1 file changed, 1 deletion(-)

diff --git a/src/xserver.cpp b/src/xserver.cpp
index 0d2ace1..8647443 100644
--- a/src/xserver.cpp
+++ b/src/xserver.cpp
@@ -336,7 +336,6 @@ void xorg::testing::XServer::Start(const std::string 
program) {
   std::vectorstd::string args;
   std::mapstd::string, std::string::iterator it;
 
-  args.push_back(program);
   args.push_back(std::string(GetDisplayString()));
 
   for (it = d_-options.begin(); it != d_-options.end(); it++) {
-- 
1.7.11.2

___
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

[PATCH xorg-gtest 06/11] process: require NULL as last argument to Start()

[Peter Hutterer]

And handle empty arguments and NULL_terminated arguments. This is a better
API than require the user to pass empty strings, especially since in some
cases an empty string may be a valid argument.

Signed-off-by: Peter Hutterer peter.hutte...@who-t.net
---
 include/xorg/gtest/xorg-gtest-process.h |  7 ---
 src/process.cpp | 13 -
 2 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/include/xorg/gtest/xorg-gtest-process.h 
b/include/xorg/gtest/xorg-gtest-process.h
index 48f387b..b4ac0d9 100644
--- a/include/xorg/gtest/xorg-gtest-process.h
+++ b/include/xorg/gtest/xorg-gtest-process.h
@@ -30,6 +30,7 @@
 #define XORG_GTEST_PROCESS_H
 
 #include stdarg.h
+#include X11/Xfuncproto.h /* for _X_SENTINEL */
 
 #include memory
 #include string
@@ -114,7 +115,7 @@ class Process {
*
* @param program The program to start.
* @param args Variadic list of arguments passed to the program. This list
-   * must end in a zero-length string (, not NULL).
+   * must end with NULL.
*
* @throws std::runtime_error on failure.
*
@@ -127,7 +128,7 @@ class Process {
* Starts a program as a child process.
*
* Takes a variadic list of arguments passed to the program. This list
-   * must end in a zero-length string (, not NULL).
+   * must end with NULL.
* See 'man execvp' for further information on the variadic argument list.
*
* @param program The program to start.
@@ -137,7 +138,7 @@ class Process {
* @post If successful: Child process forked and program started.
* @post If successful: Subsequent calls to Pid() return child process pid.
*/
-  void Start(const std::string program, ...);
+  void Start(const std::string program, ...) _X_SENTINEL(0);
 
   /**
* Terminates (SIGTERM) this child process and waits a given timeout for
diff --git a/src/process.cpp b/src/process.cpp
index 03dbc42..b85170b 100644
--- a/src/process.cpp
+++ b/src/process.cpp
@@ -69,7 +69,6 @@ void xorg::testing::Process::Start(const std::string 
program, const std::vector
 args.push_back(strdup(program.c_str()));
 
 for (it = argv.begin(); it != argv.end(); it++)
-  if (!it-empty())
 args.push_back(strdup(it-c_str()));
 args.push_back(NULL);
 
@@ -82,10 +81,14 @@ void xorg::testing::Process::Start(const std::string 
program, const std::vector
 void xorg::testing::Process::Start(const std::string program, va_list args) {
   std::vectorstd::string argv;
 
-  do {
-std::string arg(va_arg(args, char*));
-argv.push_back(arg);
-  } while (!argv.back().empty());
+  if (args) {
+char *arg;
+do {
+  arg = va_arg(args, char*);
+  if (arg)
+argv.push_back(std::string(arg));
+} while (arg);
+  }
 
   Start(program, argv);
 }
-- 
1.7.11.2

___
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

[PATCH xorg-gtest v3 07/11] process: add state enum and GetState()

[Peter Hutterer]

Add a set of basic states to Process to allow callers to keep track of which
state a process is in (as seen from the library). This simplifies code that
needs to happen on certain conditions only, e.g. log file cleanup is only
needed if the process was previously started.

Signed-off-by: Peter Hutterer peter.hutte...@who-t.net
---
Changes to v2:
- add two more starts for when the server has finsied on its own

 include/xorg/gtest/xorg-gtest-process.h | 23 +
 src/process.cpp | 36 +
 2 files changed, 59 insertions(+)

diff --git a/include/xorg/gtest/xorg-gtest-process.h 
b/include/xorg/gtest/xorg-gtest-process.h
index b4ac0d9..f1fc0ec 100644
--- a/include/xorg/gtest/xorg-gtest-process.h
+++ b/include/xorg/gtest/xorg-gtest-process.h
@@ -64,6 +64,22 @@ namespace testing {
  */
 class Process {
  public:
+   /**
+* Describes the state of a process as seen by this library. This state
+* changes some behaviors inside the library, most notably:
+* * A process in state ERROR or NONE will fail to Kill() or Terminate()
+* * A process in state FINISHED_SUCCESS or FINISHED_FAILURE will always
+* succeed to Kill() or Terminate()
+*/
+   enum State {
+ ERROR, /** An error has occured, state is now unknown */
+ NONE,  /** The process has not been started yet */
+ RUNNING,   /** The process has been started */
+ FINISHED_SUCCESS,  /** The process finished with an exit code of 0 */
+ FINISHED_FAILURE,  /** The process finished with a non-zero exit code */
+ TERMINATED,/** The process was successfully terminated by this 
library */
+   };
+
   /**
* Helper function to adjust the environment of the current process.
*
@@ -183,6 +199,13 @@ class Process {
*/
   pid_t Pid() const;
 
+  /**
+   * Return the state of the process.
+   *
+   * @return The current state of the process
+   */
+  enum Process::State GetState();
+
  private:
   struct Private;
   std::auto_ptrPrivate d_;
diff --git a/src/process.cpp b/src/process.cpp
index b85170b..4e903d6 100644
--- a/src/process.cpp
+++ b/src/process.cpp
@@ -42,10 +42,27 @@
 
 struct xorg::testing::Process::Private {
   pid_t pid;
+  enum State state;
 };
 
 xorg::testing::Process::Process() : d_(new Private) {
   d_-pid = -1;
+  d_-state = NONE;
+}
+
+enum xorg::testing::Process::State xorg::testing::Process::GetState() {
+  if (d_-state == RUNNING) {
+int status;
+int pid = waitpid(Pid(), status, WNOHANG);
+if (pid == Pid()) {
+  if (WIFEXITED(status)) {
+d_-pid = -1;
+d_-state = WEXITSTATUS(status) ? FINISHED_FAILURE : FINISHED_SUCCESS;
+  }
+}
+  }
+
+  return d_-state;
 }
 
 void xorg::testing::Process::Start(const std::string program, const 
std::vectorstd::string argv) {
@@ -55,6 +72,7 @@ void xorg::testing::Process::Start(const std::string 
program, const std::vector
   d_-pid = fork();
 
   if (d_-pid == -1) {
+d_-state = ERROR;
 throw std::runtime_error(Failed to fork child process);
   } else if (d_-pid == 0) { /* Child */
 close(0);
@@ -74,8 +92,11 @@ void xorg::testing::Process::Start(const std::string 
program, const std::vector
 
 execvp(program.c_str(), args[0]);
 
+d_-state = ERROR;
 throw std::runtime_error(Failed to start process);
   }
+
+  d_-state = RUNNING;
 }
 
 void xorg::testing::Process::Start(const std::string program, va_list args) {
@@ -117,6 +138,19 @@ bool xorg::testing::Process::WaitForExit(unsigned int 
timeout) {
 bool xorg::testing::Process::KillSelf(int signal, unsigned int timeout) {
   bool wait_success = true;
 
+  enum State state = GetState();
+  switch (state) {
+case FINISHED_SUCCESS:
+case FINISHED_FAILURE:
+case TERMINATED:
+  return true;
+case ERROR:
+case NONE:
+  return false;
+default:
+  break;
+  }
+
   if (d_-pid == -1) {
 return false;
   } else if (d_-pid == 0) {
@@ -125,12 +159,14 @@ bool xorg::testing::Process::KillSelf(int signal, 
unsigned int timeout) {
   } else { /* Parent */
 if (kill(d_-pid, signal)  0) {
   d_-pid = -1;
+  d_-state = ERROR;
   return false;
 }
 if (timeout  0)
   wait_success = WaitForExit(timeout);
 d_-pid = -1;
   }
+  d_-state = TERMINATED;
   return wait_success;
 }
 
-- 
1.7.11.2

___
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

[PATCH xorg-gtest v2 08/11] xserver: add RemoveLogFile()

[Peter Hutterer]

Simple unlink() call to the logfile in use. The log file is only removed if
the server was started and terminated or finished successfully. If it was
never started or the startup failed for some reason, this function does
nothing. Behaviour can be overridden by forcing the removal.

Signed-off-by: Peter Hutterer peter.hutte...@who-t.net
---
Changes to v1:
- add force parameter, update for new state

 include/xorg/gtest/xorg-gtest-xserver.h | 12 
 src/xserver.cpp |  6 ++
 2 files changed, 18 insertions(+)

diff --git a/include/xorg/gtest/xorg-gtest-xserver.h 
b/include/xorg/gtest/xorg-gtest-xserver.h
index f3bda9b..2e7ef13 100644
--- a/include/xorg/gtest/xorg-gtest-xserver.h
+++ b/include/xorg/gtest/xorg-gtest-xserver.h
@@ -95,6 +95,18 @@ class XServer : public xorg::testing::Process {
 virtual bool Kill(unsigned int timeout = 0);
 
 /**
+ * Remove the log file used by this server. By default, this function
+ * only removes the log file if the server was terminated or finished
+ * with an exit code of 0.
+ *
+ * If force is true, the log file is removed regardless of the state of
+ * the server.
+ *
+ * @param force Force removal of the log file
+ */
+void RemoveLogFile(bool force = false);
+
+/**
  * Waits until this server is ready to take connections.
  */
 void WaitForConnections(void);
diff --git a/src/xserver.cpp b/src/xserver.cpp
index 8647443..ab17e20 100644
--- a/src/xserver.cpp
+++ b/src/xserver.cpp
@@ -371,6 +371,12 @@ bool xorg::testing::XServer::Kill(unsigned int timeout) {
 return true;
 }
 
+void xorg::testing::XServer::RemoveLogFile(bool force) {
+  enum Process::State state = GetState();
+  if (force || state == Process::TERMINATED || state == 
Process::FINISHED_SUCCESS)
+unlink(d_-options[-logfile].c_str());
+}
+
 void xorg::testing::XServer::SetOption(const std::string key, const 
std::string value) {
   d_-options[key] = value;
 }
-- 
1.7.11.2

___
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

[PATCH xorg-gtest 09/11] test: add a self-check directory

[Peter Hutterer]

Signed-off-by: Peter Hutterer peter.hutte...@who-t.net
---
 Makefile.am   |  2 +-
 configure.ac  |  1 +
 test/.gitignore   |  1 +
 test/Makefile.am  | 62 +
 test/process-test.cpp | 70 +++
 5 files changed, 135 insertions(+), 1 deletion(-)
 create mode 100644 test/.gitignore
 create mode 100644 test/Makefile.am
 create mode 100644 test/process-test.cpp

diff --git a/Makefile.am b/Makefile.am
index 8e0a0c6..b984c8e 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -23,7 +23,7 @@
 # SOFTWARE.
 #
 
-SUBDIRS = aclocal data doc include src examples
+SUBDIRS = aclocal data doc include src examples test
 
 pkgconfigdir = $(libdir)/pkgconfig
 pkgconfig_DATA = xorg-gtest.pc
diff --git a/configure.ac b/configure.ac
index e113dee..9245d99 100644
--- a/configure.ac
+++ b/configure.ac
@@ -83,6 +83,7 @@ AC_CONFIG_FILES([Makefile
  examples/Makefile
  include/Makefile
  src/Makefile
+ test/Makefile
  xorg-gtest.pc])
 
 AC_OUTPUT
diff --git a/test/.gitignore b/test/.gitignore
new file mode 100644
index 000..096072f
--- /dev/null
+++ b/test/.gitignore
@@ -0,0 +1 @@
+process-test
diff --git a/test/Makefile.am b/test/Makefile.am
new file mode 100644
index 000..44c1027
--- /dev/null
+++ b/test/Makefile.am
@@ -0,0 +1,62 @@
+#
+# @file examples/Makefile.am
+# @brief automake recipe for the xorg-gtest self-tests
+#
+# Copyright (C) 2011, 2012 Canonical, Ltd.
+# Copyright © 2012 Red Hat, Inc.
+#
+# Permission is hereby granted, free of charge, to any person obtaining a copy
+# of this software and associated documentation files (the Software), to deal
+# in the Software without restriction, including without limitation the rights
+# to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+# copies of the Software, and to permit persons to whom the Software is
+# furnished to do so, subject to the following conditions:
+#
+# The above copyright notice and this permission notice (including the next
+# paragraph) shall be included in all copies or substantial portions of the
+# Software.
+#
+# THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+# AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+# SOFTWARE.
+#
+
+noinst_PROGRAMS = process-test
+
+AM_CPPFLAGS = $(GTEST_CPPFLAGS)
+AM_CXXFLAGS = $(BASE_CXXFLAGS)
+
+tests_libraries = \
+   libgtest.a \
+   libxorg-gtest.a \
+   -lpthread \
+   $(X11_LIBS) \
+   $(EVEMU_LIBS)
+
+process_test_SOURCES = process-test.cpp
+process_test_CPPFLAGS = $(AM_CPPFLAGS) -I$(top_srcdir)/include
+process_test_LDADD =  $(tests_libraries)
+
+check_LIBRARIES = libgtest.a libxorg-gtest.a
+
+# build googletest as static lib
+nodist_libgtest_a_SOURCES = $(GTEST_SOURCE)/src/gtest-all.cc
+libgtest_a_CPPFLAGS = $(AM_CPPFLAGS) -w
+libgtest_a_CXXFLAGS = $(GTEST_CXXFLAGS) $(AM_CXXFLAGS)
+
+# build xorg-gtest as static lib
+libxorg_gtest_a_SOURCES = $(top_srcdir)/src/xorg-gtest-all.cpp
+libxorg_gtest_a_CPPFLAGS = \
+   $(AM_CPPFLAGS) \
+   -I$(top_srcdir)/include \
+   -I$(top_srcdir) \
+   -DDUMMY_CONF_PATH=\$(top_srcdir)/data/xorg/gtest/dummy.conf\
+libxorg_gtest_a_CXXFLAGS = $(GTEST_CXXFLAGS) $(AM_CXXFLAGS)
+
+if ENABLE_XORG_GTEST_TESTS
+TESTS = $(noinst_PROGRAMS)
+endif
diff --git a/test/process-test.cpp b/test/process-test.cpp
new file mode 100644
index 000..dc5402c
--- /dev/null
+++ b/test/process-test.cpp
@@ -0,0 +1,70 @@
+#include sys/types.h
+#include sys/wait.h
+
+#include gtest/gtest.h
+#include xorg/gtest/xorg-gtest.h
+
+using namespace xorg::testing;
+
+TEST(Process, StartWithNULLArg)
+{
+  SCOPED_TRACE(TESTCASE: invocation of 'ls' with no arguments);
+  Process p;
+  p.Start(ls, NULL);
+  ASSERT_GT(p.Pid(), 0);
+}
+
+TEST(Process, StartWithNULLTerminatedArg)
+{
+  SCOPED_TRACE(TESTCASE: invocation of 'ls' with NULL-terminated argument 
list);
+
+  Process p;
+  p.Start(ls, -l, NULL);
+  ASSERT_GT(p.Pid(), 0);
+}
+
+TEST(Process, TerminationSuccess)
+{
+  SCOPED_TRACE(TESTCASE: invocation of 'echo -n', check for success exit 
status);
+
+  Process p;
+  ASSERT_EQ(p.GetState(), Process::NONE);
+
+  /* Process:Start closes stdout, so we need something that doesn't print */
+  p.Start(echo, -n, NULL);
+  ASSERT_GT(p.Pid(), 0);
+  ASSERT_EQ(p.GetState(), Process::RUNNING);
+
+  /* ls shouldn't take longer terminate */
+  for (int i = 0; i  100; i++) {
+if (p.GetState() == Process::RUNNING)
+  usleep(5000);
+  }
+  ASSERT_EQ(p.GetState(), Process::FINISHED_SUCCESS);
+}
+
+TEST(Process, 

[PATCH xorg-gtest 10/11] process: terminate the child if the parent dies

[Peter Hutterer]

It's annoying to have the forked X server linger around when the test
segfaults, so make sure we take it down with us.

Signed-off-by: Peter Hutterer peter.hutte...@who-t.net
---
 src/process.cpp   |  5 +
 test/process-test.cpp | 45 +
 2 files changed, 50 insertions(+)

diff --git a/src/process.cpp b/src/process.cpp
index 4e903d6..d962a4c 100644
--- a/src/process.cpp
+++ b/src/process.cpp
@@ -27,6 +27,7 @@
 
 #include xorg/gtest/xorg-gtest-process.h
 
+#include sys/prctl.h
 #include sys/types.h
 #include sys/wait.h
 #include unistd.h
@@ -81,6 +82,10 @@ void xorg::testing::Process::Start(const std::string 
program, const std::vector
   close(2);
 }
 
+#ifdef __linux
+prctl(PR_SET_PDEATHSIG, SIGTERM);
+#endif
+
 std::vectorchar* args;
 std::vectorstd::string::const_iterator it;
 
diff --git a/test/process-test.cpp b/test/process-test.cpp
index dc5402c..183b9f8 100644
--- a/test/process-test.cpp
+++ b/test/process-test.cpp
@@ -1,3 +1,5 @@
+#include errno.h
+#include unistd.h
 #include sys/types.h
 #include sys/wait.h
 
@@ -63,6 +65,49 @@ TEST(Process, TerminationFailure)
   ASSERT_EQ(p.GetState(), Process::FINISHED_FAILURE);
 }
 
+TEST(Process, ChildTearDown)
+{
+  SCOPED_TRACE(TESTCASE: ensure child process dies when parent does);
+
+  int pipefd[2];
+  ASSERT_NE(pipe(pipefd), -1);
+
+  /* Fork, the child will spawn a Process, write that Process's PID to a
+ pipe and then kill itself. The parent checks that the child was
+ terminated when the parent was killed.
+   */
+  pid_t pid = fork();
+  if (pid == 0) { /* child */
+close(pipefd[0]);
+
+Process p;
+p.Start(sleep, 1000, NULL); /* forks another child */
+ASSERT_GT(p.Pid(), 0);
+
+char *buffer;
+ASSERT_GT(asprintf(buffer, %d, p.Pid()), 0);
+ASSERT_EQ(write(pipefd[1], buffer, strlen(buffer)), (int)strlen(buffer));
+close(pipefd[1]);
+
+raise(SIGKILL);
+  } else { /* parent */
+close(pipefd[1]);
+
+char buffer[20] = {0};
+ASSERT_GT(read(pipefd[0], buffer, sizeof(buffer)), 0);
+close(pipefd[0]);
+
+pid_t child_pid = atoi(buffer);
+for (int i = 0; i  10; i++) {
+  if (kill(child_pid, 0) != -1)
+usleep(100);
+
+}
+ASSERT_EQ(kill(child_pid, 0), -1);
+ASSERT_EQ(errno, ESRCH);
+  }
+}
+
 
 int main(int argc, char *argv[]) {
   testing::InitGoogleTest(argc, argv);
-- 
1.7.11.2

___
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

[PATCH xorg-gtest 11/11] test: add xserver test for logfile removal

[Peter Hutterer]

Signed-off-by: Peter Hutterer peter.hutte...@who-t.net
---
 test/Makefile.am  |  6 -
 test/xserver-test.cpp | 62 +++
 2 files changed, 67 insertions(+), 1 deletion(-)
 create mode 100644 test/xserver-test.cpp

diff --git a/test/Makefile.am b/test/Makefile.am
index 44c1027..ed6416a 100644
--- a/test/Makefile.am
+++ b/test/Makefile.am
@@ -25,7 +25,7 @@
 # SOFTWARE.
 #
 
-noinst_PROGRAMS = process-test
+noinst_PROGRAMS = process-test xserver-test
 
 AM_CPPFLAGS = $(GTEST_CPPFLAGS)
 AM_CXXFLAGS = $(BASE_CXXFLAGS)
@@ -41,6 +41,10 @@ process_test_SOURCES = process-test.cpp
 process_test_CPPFLAGS = $(AM_CPPFLAGS) -I$(top_srcdir)/include
 process_test_LDADD =  $(tests_libraries)
 
+xserver_test_SOURCES = xserver-test.cpp
+xserver_test_CPPFLAGS = $(AM_CPPFLAGS) -I$(top_srcdir)/include
+xserver_test_LDADD =  $(tests_libraries)
+
 check_LIBRARIES = libgtest.a libxorg-gtest.a
 
 # build googletest as static lib
diff --git a/test/xserver-test.cpp b/test/xserver-test.cpp
new file mode 100644
index 000..ba6c462
--- /dev/null
+++ b/test/xserver-test.cpp
@@ -0,0 +1,62 @@
+#include errno.h
+#include unistd.h
+#include sys/types.h
+#include sys/wait.h
+#include fstream
+
+#include gtest/gtest.h
+#include xorg/gtest/xorg-gtest.h
+
+using namespace xorg::testing;
+
+TEST(XServer, LogRemoval)
+{
+  SCOPED_TRACE(TESTCASE: X server startup and log file removal on success and 
error);
+  std::string logfile = /tmp/xorg-testing-xserver_.log;
+
+  /* make sure a previous failed test didn't leave it around */
+  unlink(logfile.c_str());
+
+  XServer server;
+  server.SetOption(-logfile, logfile);
+  server.Start();
+  server.Terminate(3000);
+  server.RemoveLogFile();
+
+  std::ifstream file(logfile.c_str());
+  ASSERT_FALSE(file.good());
+  file.close();
+
+  server.SetOption(-doesnotexist, );
+  server.Start();
+  while (server.GetState() == Process::RUNNING)
+usleep(5000);
+
+  ASSERT_EQ(server.GetState(), Process::FINISHED_FAILURE);
+  file.open(logfile.c_str());
+  ASSERT_FALSE(file.good()); /* server didn't leave the file behind */
+
+  /* now create it */
+  std::ofstream f(logfile.c_str());
+  file.open(logfile.c_str());
+  ASSERT_TRUE(file.good());
+  file.close();
+
+  /* must not remove it now */
+  server.RemoveLogFile();
+
+  file.open(logfile.c_str());
+  ASSERT_TRUE(file.good()); /* server didn't remove it */
+  file.close();
+
+  server.RemoveLogFile(true);
+  file.open(logfile.c_str());
+  ASSERT_FALSE(file.good()); /* server did remove it */
+  file.close();
+}
+
+
+int main(int argc, char *argv[]) {
+  testing::InitGoogleTest(argc, argv);
+  return RUN_ALL_TESTS();
+}
-- 
1.7.11.2

___
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

Re: [PATCH xorg-gtest 01/11] Add Red Hat copyright notices

[Mark Kettenis]

 From: Peter Hutterer peter.hutte...@who-t.net
 Date: Thu, 16 Aug 2012 16:36:34 +1000
 
 Signed-off-by: Peter Hutterer peter.hutte...@who-t.net
 ---
  COPYING  | 1 +
  include/xorg/gtest/evemu/xorg-gtest-device.h | 1 +
  include/xorg/gtest/xorg-gtest-environment.h  | 1 +
  include/xorg/gtest/xorg-gtest-process.h  | 1 +
  include/xorg/gtest/xorg-gtest-test.h | 1 +
  src/device.cpp   | 1 +
  src/environment.cpp  | 1 +
  src/test.cpp | 1 +
  8 files changed, 8 insertions(+)
 
 diff --git a/COPYING b/COPYING
 index 4da49fe..2e36349 100644
 --- a/COPYING
 +++ b/COPYING
 @@ -1,4 +1,5 @@
  Copyright (C) 2011, 2012 Canonical, Ltd.
 +Copyright © 2012 Red Hat, Inc.

Do you really have to pollute more source files with non-ASCII characters?
___
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

Re: [PATCH xorg-gtest 01/11] Add Red Hat copyright notices

[Daniel Stone]

Hi,

On 16 August 2012 09:13, Mark Kettenis mark.kette...@xs4all.nl wrote:
 index 4da49fe..2e36349 100644
 --- a/COPYING
 +++ b/COPYING
 @@ -1,4 +1,5 @@
  Copyright (C) 2011, 2012 Canonical, Ltd.
 +Copyright © 2012 Red Hat, Inc.

 Do you really have to pollute more source files with non-ASCII characters?

Unfortunately for you, I think you've lost the battle against UTF-8:
swamp81:~/x/xorg/xserver(master)% g -rl © **/*.[ch] | wc -l
264

Welcome to 2012!

Cheers,
Daniel
___
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

Re: [PATCH] vbe: Fix caching of DDC support status

[Adam Jackson]

On Mon, 2012-08-06 at 16:51 -0700, Keith Packard wrote:
 Adam Jackson a...@redhat.com writes:
 
  There are multiple kinds of not supported here.  One of them is where
  the video BIOS doesn't support the entrypoint.  Another is where the
  BIOS returns nothing because no monitor happens to be connected.  In the
  latter case we should just pretend we haven't checked yet, otherwise
  plugging in a monitor later and retrying DDC would fail.
 
 This all makes sense, but I'd love to know if someone else has tried
 this or has a better understanding of this code before I merge it into
 master.

Well, it's shipping in RHEL 6.3, because this needs it (because I
haven't properly fixed DDC to be native yet, as it didn't work on all
the GPUs I had available to test):

http://cgit.freedesktop.org/xorg/driver/xf86-video-mga/log/?h=g200se-randr-1.2

I've been trying to make my commit messages lucid enough that
non-domain-experts can follow them, but it doesn't seem to be having the
desired effect.

- ajax


signature.asc
Description: This is a digitally signed message part
___
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

Re: [PATCH 5/5] os: don't block signal-unsafe logging, merely warn about it.

[Chase Douglas]

On 08/15/2012 09:52 PM, Peter Hutterer wrote:

Throw an error into the log file, but continue anyway. And after three
warnings, stop complaining. Not all input drivers will be fixed in time (or
ever) and our printf implementation is vastly inferior, so there is still a
use-case for non-sigsafe logging.

This also adds more linebreaks to the message.

CC: Chase Douglas chase.doug...@canonical.com
Signed-off-by: Peter Hutterer peter.hutte...@who-t.net
---
  os/log.c | 39 +--
  1 file changed, 25 insertions(+), 14 deletions(-)

diff --git a/os/log.c b/os/log.c
index 4348739..7fca317 100644
--- a/os/log.c
+++ b/os/log.c
@@ -467,6 +467,7 @@ LogMessageTypeVerbString(MessageType type, int verb)
  void
  LogVMessageVerb(MessageType type, int verb, const char *format, va_list args)
  {
+static unsigned int warned;
  const char *type_str;
  char buf[1024];
  const size_t size = sizeof(buf);
@@ -474,13 +475,17 @@ LogVMessageVerb(MessageType type, int verb, const char 
*format, va_list args)
  size_t len = 0;

  if (inSignalContext) {
-BUG_WARN_MSG(inSignalContext,
- Warning: attempting to log data in a signal unsafe 
- manner while in signal context. Please update to check 
- inSignalContext and/or use LogMessageVerbSigSafe() or 
- ErrorFSigSafe(). The offending log format message is:\n
- %s\n, format);
-return;
+if (warned  3) {
+BUG_WARN_MSG(inSignalContext,
+ Warning: attempting to log data in a signal unsafe 
+ manner while in signal context.\nPlease update to check 

+ inSignalContext and/or use LogMessageVerbSigSafe() or 

+ ErrorFSigSafe().\nThe offending log format message 
is:\n
+ %s\n, format);
+warned++;
+if (warned == 3)
+LogMessageVerbSigSafe(X_WARNING, -1, Warned %u times about sigsafe 
logging. Will be quiet now.\n, warned);
+}
  }

  type_str = LogMessageTypeVerbString(type, verb);
@@ -566,6 +571,7 @@ void
  LogVHdrMessageVerb(MessageType type, int verb, const char *msg_format,
 va_list msg_args, const char *hdr_format, va_list hdr_args)
  {
+static unsigned int warned;
  const char *type_str;
  char buf[1024];
  const size_t size = sizeof(buf);
@@ -573,15 +579,20 @@ LogVHdrMessageVerb(MessageType type, int verb, const char 
*msg_format,
  size_t len = 0;

  if (inSignalContext) {
-BUG_WARN_MSG(inSignalContext,
- Warning: attempting to log data in a signal unsafe 
- manner while in signal context. Please update to check 
- inSignalContext and/or use LogMessageVerbSigSafe(). The 
- offending header and log message formats are:\n%s %s\n,
- hdr_format, msg_format);
-return;
+if (warned  3) {
+BUG_WARN_MSG(inSignalContext,
+ Warning: attempting to log data in a signal unsafe 
+ manner while in signal context.\nPlease update to check 

+ inSignalContext and/or use LogMessageVerbSigSafe().\nThe 

+ offending header and log message formats are:\n%s 
%s\n,
+ hdr_format, msg_format);
+warned++;
+if (warned == 3)
+LogMessageVerbSigSafe(X_WARNING, -1, Warned %u times about sigsafe 
logging. Will be quiet now.\n, warned);
+}
  }

+


Stray newline here ^^


  type_str = LogMessageTypeVerbString(type, verb);
  if (!type_str)
  return;



It's too bad everything won't be fixed in time, but we've gotten along 
this far without too much issue. Hopefully the warnings will be big 
enough to force the issue to a close.


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

Re: [PATCH xorg-gtest] process: document that va_args for Start() must end with zero-length string

[Chase Douglas]

On 08/14/2012 04:20 PM, Peter Hutterer wrote:

On Tue, Aug 14, 2012 at 10:19:00AM -0700, Chase Douglas wrote:

On 08/13/2012 06:16 PM, Peter Hutterer wrote:

Signed-off-by: Peter Hutterer peter.hutte...@who-t.net
---
  include/xorg/gtest/xorg-gtest-process.h | 6 --
  1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/include/xorg/gtest/xorg-gtest-process.h 
b/include/xorg/gtest/xorg-gtest-process.h
index 69b3b34..8cf60e3 100644
--- a/include/xorg/gtest/xorg-gtest-process.h
+++ b/include/xorg/gtest/xorg-gtest-process.h
@@ -123,7 +123,8 @@ class Process {
 * See 'man execvp' for further information on the variadic argument list.
 *
 * @param program The program to start.
-   * @param args Variadic list of arguments passed to the program.
+   * @param args Variadic list of arguments passed to the program. This list
+   * must end in a zero-length string (, not NULL).
 *
 * @throws std::runtime_error on failure.
 *
@@ -135,7 +136,8 @@ class Process {
/**
 * Starts a program as a child process.
 *
-   * Takes a variadic list of arguments passed to the program.
+   * Takes a variadic list of arguments passed to the program. This list
+   * must end in a zero-length string (, not NULL).
 * See 'man execvp' for further information on the variadic argument list.
 *
 * @param program The program to start.



Hmmm... I wish we had though to use a NULL sentinel for the varargs
version. We could have used the gcc sentinel function attribute to
help warn people of bad usage. I suppose it's too late to change now
that we've released it.


I disagree. We're only up to 0.4 and have few users only. Pretending we have
a good and stable API already is optimistic at best, it's better to fix the
things that are obviously (or at least reasonably :) broken. This isn't hard
thing to fix either (I'll send patches out in a bit), so there really isn't
an excuse for having a bad API, it'll just come and haunt us later.


I'll confess that I was attempting a bait a little with my response :). 
I agree that there are few users and the change would be small, so I'm 
fine with fixing this up.


-- Chase
___
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

Re: [PATCH xorg-gtest 06/11] process: require NULL as last argument to Start()

[Chase Douglas]

On 08/15/2012 11:36 PM, Peter Hutterer wrote:

And handle empty arguments and NULL_terminated arguments. This is a better
API than require the user to pass empty strings, especially since in some
cases an empty string may be a valid argument.

Signed-off-by: Peter Hutterer peter.hutte...@who-t.net
---
  include/xorg/gtest/xorg-gtest-process.h |  7 ---
  src/process.cpp | 13 -
  2 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/include/xorg/gtest/xorg-gtest-process.h 
b/include/xorg/gtest/xorg-gtest-process.h
index 48f387b..b4ac0d9 100644
--- a/include/xorg/gtest/xorg-gtest-process.h
+++ b/include/xorg/gtest/xorg-gtest-process.h
@@ -30,6 +30,7 @@
  #define XORG_GTEST_PROCESS_H

  #include stdarg.h
+#include X11/Xfuncproto.h /* for _X_SENTINEL */

  #include memory
  #include string
@@ -114,7 +115,7 @@ class Process {
 *
 * @param program The program to start.
 * @param args Variadic list of arguments passed to the program. This list
-   * must end in a zero-length string (, not NULL).
+   * must end with NULL.
 *
 * @throws std::runtime_error on failure.
 *
@@ -127,7 +128,7 @@ class Process {
 * Starts a program as a child process.
 *
 * Takes a variadic list of arguments passed to the program. This list
-   * must end in a zero-length string (, not NULL).
+   * must end with NULL.
 * See 'man execvp' for further information on the variadic argument list.
 *
 * @param program The program to start.
@@ -137,7 +138,7 @@ class Process {
 * @post If successful: Child process forked and program started.
 * @post If successful: Subsequent calls to Pid() return child process pid.
 */
-  void Start(const std::string program, ...);
+  void Start(const std::string program, ...) _X_SENTINEL(0);

/**
 * Terminates (SIGTERM) this child process and waits a given timeout for
diff --git a/src/process.cpp b/src/process.cpp
index 03dbc42..b85170b 100644
--- a/src/process.cpp
+++ b/src/process.cpp
@@ -69,7 +69,6 @@ void xorg::testing::Process::Start(const std::string 
program, const std::vector
  args.push_back(strdup(program.c_str()));

  for (it = argv.begin(); it != argv.end(); it++)
-  if (!it-empty())
  args.push_back(strdup(it-c_str()));


This should be de-indented now ^^


  args.push_back(NULL);

@@ -82,10 +81,14 @@ void xorg::testing::Process::Start(const std::string 
program, const std::vector
  void xorg::testing::Process::Start(const std::string program, va_list args) {
std::vectorstd::string argv;

-  do {
-std::string arg(va_arg(args, char*));
-argv.push_back(arg);
-  } while (!argv.back().empty());
+  if (args) {
+char *arg;
+do {
+  arg = va_arg(args, char*);
+  if (arg)
+argv.push_back(std::string(arg));
+} while (arg);
+  }

Start(program, argv);
  }



Thanks for fixing this up :).

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

Re: [PATCH xorg-gtest 10/11] process: terminate the child if the parent dies

[Chase Douglas]

On 08/15/2012 11:36 PM, Peter Hutterer wrote:

It's annoying to have the forked X server linger around when the test
segfaults, so make sure we take it down with us.

Signed-off-by: Peter Hutterer peter.hutte...@who-t.net
---
  src/process.cpp   |  5 +
  test/process-test.cpp | 45 +
  2 files changed, 50 insertions(+)

diff --git a/src/process.cpp b/src/process.cpp
index 4e903d6..d962a4c 100644
--- a/src/process.cpp
+++ b/src/process.cpp
@@ -27,6 +27,7 @@

  #include xorg/gtest/xorg-gtest-process.h

+#include sys/prctl.h
  #include sys/types.h
  #include sys/wait.h
  #include unistd.h
@@ -81,6 +82,10 @@ void xorg::testing::Process::Start(const std::string 
program, const std::vector
close(2);
  }

+#ifdef __linux
+prctl(PR_SET_PDEATHSIG, SIGTERM);
+#endif


I had been looking around for this ^^ for a while! It's just what we 
need to fully clean things up. Good find :).



+
  std::vectorchar* args;
  std::vectorstd::string::const_iterator it;

diff --git a/test/process-test.cpp b/test/process-test.cpp
index dc5402c..183b9f8 100644
--- a/test/process-test.cpp
+++ b/test/process-test.cpp
@@ -1,3 +1,5 @@
+#include errno.h
+#include unistd.h
  #include sys/types.h
  #include sys/wait.h

@@ -63,6 +65,49 @@ TEST(Process, TerminationFailure)
ASSERT_EQ(p.GetState(), Process::FINISHED_FAILURE);
  }

+TEST(Process, ChildTearDown)
+{
+  SCOPED_TRACE(TESTCASE: ensure child process dies when parent does);
+
+  int pipefd[2];
+  ASSERT_NE(pipe(pipefd), -1);
+
+  /* Fork, the child will spawn a Process, write that Process's PID to a
+ pipe and then kill itself. The parent checks that the child was
+ terminated when the parent was killed.
+   */
+  pid_t pid = fork();
+  if (pid == 0) { /* child */
+close(pipefd[0]);
+
+Process p;
+p.Start(sleep, 1000, NULL); /* forks another child */
+ASSERT_GT(p.Pid(), 0);
+
+char *buffer;
+ASSERT_GT(asprintf(buffer, %d, p.Pid()), 0);
+ASSERT_EQ(write(pipefd[1], buffer, strlen(buffer)), (int)strlen(buffer));
+close(pipefd[1]);
+
+raise(SIGKILL);
+  } else { /* parent */
+close(pipefd[1]);
+
+char buffer[20] = {0};
+ASSERT_GT(read(pipefd[0], buffer, sizeof(buffer)), 0);
+close(pipefd[0]);
+
+pid_t child_pid = atoi(buffer);
+for (int i = 0; i  10; i++) {
+  if (kill(child_pid, 0) != -1)
+usleep(100);
+
+}
+ASSERT_EQ(kill(child_pid, 0), -1);
+ASSERT_EQ(errno, ESRCH);
+  }
+}
+

  int main(int argc, char *argv[]) {
testing::InitGoogleTest(argc, argv);


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

Re: [PATCH xorg-gtest 11/11] test: add xserver test for logfile removal

[Chase Douglas]

On 08/15/2012 11:36 PM, Peter Hutterer wrote:

Signed-off-by: Peter Hutterer peter.hutte...@who-t.net
---
  test/Makefile.am  |  6 -
  test/xserver-test.cpp | 62 +++
  2 files changed, 67 insertions(+), 1 deletion(-)
  create mode 100644 test/xserver-test.cpp

diff --git a/test/Makefile.am b/test/Makefile.am
index 44c1027..ed6416a 100644
--- a/test/Makefile.am
+++ b/test/Makefile.am
@@ -25,7 +25,7 @@
  # SOFTWARE.
  #

-noinst_PROGRAMS = process-test
+noinst_PROGRAMS = process-test xserver-test

  AM_CPPFLAGS = $(GTEST_CPPFLAGS)
  AM_CXXFLAGS = $(BASE_CXXFLAGS)
@@ -41,6 +41,10 @@ process_test_SOURCES = process-test.cpp
  process_test_CPPFLAGS = $(AM_CPPFLAGS) -I$(top_srcdir)/include
  process_test_LDADD =  $(tests_libraries)

+xserver_test_SOURCES = xserver-test.cpp
+xserver_test_CPPFLAGS = $(AM_CPPFLAGS) -I$(top_srcdir)/include
+xserver_test_LDADD =  $(tests_libraries)
+
  check_LIBRARIES = libgtest.a libxorg-gtest.a

  # build googletest as static lib
diff --git a/test/xserver-test.cpp b/test/xserver-test.cpp
new file mode 100644
index 000..ba6c462
--- /dev/null
+++ b/test/xserver-test.cpp
@@ -0,0 +1,62 @@
+#include errno.h
+#include unistd.h
+#include sys/types.h
+#include sys/wait.h
+#include fstream
+
+#include gtest/gtest.h
+#include xorg/gtest/xorg-gtest.h
+
+using namespace xorg::testing;
+
+TEST(XServer, LogRemoval)
+{
+  SCOPED_TRACE(TESTCASE: X server startup and log file removal on success and 
error);
+  std::string logfile = /tmp/xorg-testing-xserver_.log;
+
+  /* make sure a previous failed test didn't leave it around */
+  unlink(logfile.c_str());
+
+  XServer server;
+  server.SetOption(-logfile, logfile);
+  server.Start();
+  server.Terminate(3000);
+  server.RemoveLogFile();
+
+  std::ifstream file(logfile.c_str());
+  ASSERT_FALSE(file.good());
+  file.close();
+
+  server.SetOption(-doesnotexist, );
+  server.Start();
+  while (server.GetState() == Process::RUNNING)
+usleep(5000);
+
+  ASSERT_EQ(server.GetState(), Process::FINISHED_FAILURE);
+  file.open(logfile.c_str());
+  ASSERT_FALSE(file.good()); /* server didn't leave the file behind */
+
+  /* now create it */
+  std::ofstream f(logfile.c_str());
+  file.open(logfile.c_str());
+  ASSERT_TRUE(file.good());
+  file.close();
+
+  /* must not remove it now */
+  server.RemoveLogFile();
+
+  file.open(logfile.c_str());
+  ASSERT_TRUE(file.good()); /* server didn't remove it */
+  file.close();
+
+  server.RemoveLogFile(true);
+  file.open(logfile.c_str());
+  ASSERT_FALSE(file.good()); /* server did remove it */
+  file.close();
+}
+
+
+int main(int argc, char *argv[]) {
+  testing::InitGoogleTest(argc, argv);
+  return RUN_ALL_TESTS();
+}


I don't see why these tests shouldn't be compiled and linked into one 
binary with the other tests in process-test.cpp. I think it will make 
the tests easier to maintain over time. The simple way to do this would be:


* Remove the main() functions from each file
* Compile gtest_main.cc
* Link each test object file and gtest_main together

There's nothing wrong with this approach, so I'll give my r-b, but the 
above approach would generate faster test runs and more easily analyzed 
test results in case you want to feed them into something like jenkins.


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

Re: [PATCH xorg-gtest 03/11] xserver: use temporary variable for log file too

[Chase Douglas]

On 08/15/2012 11:36 PM, Peter Hutterer wrote:

And use + for string concatination, the code as-is was a legacy from const
char* handling.

Signed-off-by: Peter Hutterer peter.hutte...@who-t.net
---
  src/xserver.cpp | 17 -
  1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/src/xserver.cpp b/src/xserver.cpp
index 86c8484..eb64d3b 100644
--- a/src/xserver.cpp
+++ b/src/xserver.cpp
@@ -266,28 +266,27 @@ void xorg::testing::XServer::TestStartup(void) {
  throw std::runtime_error(message);
}

+  std::string log = d_-options[-logfile];
+
/* 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_-options[-logfile].c_str(), std::ofstream::out);
+  log_test.open(log.c_str(), std::ofstream::out);
log_test.close();
if (log_test.fail()) {
  std::string message;
-message += X.org server log file ;
-message += d_-options[-logfile];
-message +=  is not writable.;
+message += X.org server log file  + log +  is not writable.;


I think this could be simplified to:

std::string message =
X.org server log file  + log +  is not writeable.;


  throw std::runtime_error(message);
}

-  std::string old_log_file = d_-options[-logfile];
-  old_log_file += .old;
+  std::string old_log_file = log + .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.;
+message += X.org old server log file  + old_log_file +  is not 
writable.;


Same simplification here


  throw std::runtime_error(message);
}




Either way,

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

Re: [PATCH xorg-gtest 01/11] Add Red Hat copyright notices

[Chase Douglas]

On 08/15/2012 11:36 PM, Peter Hutterer wrote:

Signed-off-by: Peter Hutterer peter.hutte...@who-t.net
---
  COPYING  | 1 +
  include/xorg/gtest/evemu/xorg-gtest-device.h | 1 +
  include/xorg/gtest/xorg-gtest-environment.h  | 1 +
  include/xorg/gtest/xorg-gtest-process.h  | 1 +
  include/xorg/gtest/xorg-gtest-test.h | 1 +
  src/device.cpp   | 1 +
  src/environment.cpp  | 1 +
  src/test.cpp | 1 +
  8 files changed, 8 insertions(+)

diff --git a/COPYING b/COPYING
index 4da49fe..2e36349 100644
--- a/COPYING
+++ b/COPYING
@@ -1,4 +1,5 @@
  Copyright (C) 2011, 2012 Canonical, Ltd.
+Copyright © 2012 Red Hat, Inc.

  Permission is hereby granted, free of charge, to any person obtaining a copy 
of
  this software and associated documentation files (the Software), to deal in
diff --git a/include/xorg/gtest/evemu/xorg-gtest-device.h 
b/include/xorg/gtest/evemu/xorg-gtest-device.h
index 223738e..1a0e5ee 100644
--- a/include/xorg/gtest/evemu/xorg-gtest-device.h
+++ b/include/xorg/gtest/evemu/xorg-gtest-device.h
@@ -3,6 +3,7 @@
   * X testing environment - Google Test environment feat. dummy x server
   *
   * Copyright (C) 2012 Canonical Ltd.
+ * Copyright © 2012 Red Hat, Inc.
   *
   * Permission is hereby granted, free of charge, to any person obtaining a 
copy
   * of this software and associated documentation files (the Software), to 
deal
diff --git a/include/xorg/gtest/xorg-gtest-environment.h 
b/include/xorg/gtest/xorg-gtest-environment.h
index aee9cdd..b2c2be7 100644
--- a/include/xorg/gtest/xorg-gtest-environment.h
+++ b/include/xorg/gtest/xorg-gtest-environment.h
@@ -3,6 +3,7 @@
   * X testing environment - Google Test environment feat. dummy x server
   *
   * Copyright (C) 2011, 2012 Canonical Ltd.
+ * Copyright © 2012 Red Hat, Inc.
   *
   * Permission is hereby granted, free of charge, to any person obtaining a 
copy
   * of this software and associated documentation files (the Software), to 
deal
diff --git a/include/xorg/gtest/xorg-gtest-process.h 
b/include/xorg/gtest/xorg-gtest-process.h
index 7d6ab62..48f387b 100644
--- a/include/xorg/gtest/xorg-gtest-process.h
+++ b/include/xorg/gtest/xorg-gtest-process.h
@@ -3,6 +3,7 @@
   * X testing environment - Google Test environment feat. dummy x server
   *
   * Copyright (C) 2011, 2012 Canonical Ltd.
+ * Copyright © 2012 Red Hat, Inc.
   *
   * Permission is hereby granted, free of charge, to any person obtaining a 
copy
   * of this software and associated documentation files (the Software), to 
deal
diff --git a/include/xorg/gtest/xorg-gtest-test.h 
b/include/xorg/gtest/xorg-gtest-test.h
index a776693..133f21b 100644
--- a/include/xorg/gtest/xorg-gtest-test.h
+++ b/include/xorg/gtest/xorg-gtest-test.h
@@ -3,6 +3,7 @@
   * X testing environment - Google Test environment feat. dummy x server
   *
   * Copyright (C) 2011, 2012 Canonical Ltd.
+ * Copyright © 2012 Red Hat, Inc.
   *
   * Permission is hereby granted, free of charge, to any person obtaining a 
copy
   * of this software and associated documentation files (the Software), to 
deal
diff --git a/src/device.cpp b/src/device.cpp
index 1f1c94b..884ba3b 100644
--- a/src/device.cpp
+++ b/src/device.cpp
@@ -3,6 +3,7 @@
   * X testing environment - Google Test environment feat. dummy x server
   *
   * Copyright (C) 2012 Canonical Ltd.
+ * Copyright © 2012 Red Hat, Inc.
   *
   * Permission is hereby granted, free of charge, to any person obtaining a 
copy
   * of this software and associated documentation files (the Software), to 
deal
diff --git a/src/environment.cpp b/src/environment.cpp
index 9b3ab90..54eb6b6 100644
--- a/src/environment.cpp
+++ b/src/environment.cpp
@@ -3,6 +3,7 @@
   * X testing environment - Google Test environment feat. dummy x server
   *
   * Copyright (C) 2011, 2012 Canonical Ltd.
+ * Copyright © 2012 Red Hat, Inc.
   *
   * Permission is hereby granted, free of charge, to any person obtaining a 
copy
   * of this software and associated documentation files (the Software), to 
deal
diff --git a/src/test.cpp b/src/test.cpp
index 94adf13..c7b69bc 100644
--- a/src/test.cpp
+++ b/src/test.cpp
@@ -3,6 +3,7 @@
   * X testing environment - Google Test environment feat. dummy x server
   *
   * Copyright (C) 2011, 2012 Canonical Ltd.
+ * Copyright © 2012 Red Hat, Inc.
   *
   * Permission is hereby granted, free of charge, to any person obtaining a 
copy
   * of this software and associated documentation files (the Software), to 
deal



For this and all the other patches I haven't replied specifically:

Reviewed-by: Chase Douglas chase.doug...@canonical.com

Thanks!
___
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

Re: [PATCH 0/5] signal safe logging fixes

[Chase Douglas]

On 08/15/2012 09:52 PM, Peter Hutterer wrote:


1/5 is just rearranging, the interesting bits are 3 and 4 to add signed
printing to sigsafe logging.

5/5 changes the current behaviour to allow signal-unsafe logging again. This
is largely done because I won't be able to fix all drivers by 1.13 and given
that we had signal unsafe logging for years without too much fallout, we'll
probably be fine for those drivers.


Everything looks reasonable and correct to me. For the series:

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

Re: [PATCH xorg-gtest 03/11] xserver: use temporary variable for log file too

[Peter Hutterer]

On Thu, Aug 16, 2012 at 08:49:24AM -0700, Chase Douglas wrote:
 On 08/15/2012 11:36 PM, Peter Hutterer wrote:
 And use + for string concatination, the code as-is was a legacy from const
 char* handling.
 
 Signed-off-by: Peter Hutterer peter.hutte...@who-t.net
 ---
   src/xserver.cpp | 17 -
   1 file changed, 8 insertions(+), 9 deletions(-)
 
 diff --git a/src/xserver.cpp b/src/xserver.cpp
 index 86c8484..eb64d3b 100644
 --- a/src/xserver.cpp
 +++ b/src/xserver.cpp
 @@ -266,28 +266,27 @@ void xorg::testing::XServer::TestStartup(void) {
   throw std::runtime_error(message);
 }
 
 +  std::string log = d_-options[-logfile];
 +
 /* 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_-options[-logfile].c_str(), std::ofstream::out);
 +  log_test.open(log.c_str(), std::ofstream::out);
 log_test.close();
 if (log_test.fail()) {
   std::string message;
 -message += X.org server log file ;
 -message += d_-options[-logfile];
 -message +=  is not writable.;
 +message += X.org server log file  + log +  is not writable.;
 
 I think this could be simplified to:
 
 std::string message =
 X.org server log file  + log +  is not writeable.;
 
   throw std::runtime_error(message);

whoops, must have had my blinkers on. you're right, it can even be
simplified to 
   throw std::runtime_error(Xorg server  + log +  blah blah )
so I've done just that.

Cheers,
   Peter

 }
 
 -  std::string old_log_file = d_-options[-logfile];
 -  old_log_file += .old;
 +  std::string old_log_file = log + .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.;
 +message += X.org old server log file  + old_log_file +  is not 
 writable.;
 
 Same simplification here
 
   throw std::runtime_error(message);
 }
 
 
 
 Either way,
 
 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

Re: [PATCH xorg-gtest 11/11] test: add xserver test for logfile removal

[Peter Hutterer]

On Thu, Aug 16, 2012 at 08:47:15AM -0700, Chase Douglas wrote:
 On 08/15/2012 11:36 PM, Peter Hutterer wrote:
 Signed-off-by: Peter Hutterer peter.hutte...@who-t.net
 ---
   test/Makefile.am  |  6 -
   test/xserver-test.cpp | 62 
  +++
   2 files changed, 67 insertions(+), 1 deletion(-)
   create mode 100644 test/xserver-test.cpp
 
 diff --git a/test/Makefile.am b/test/Makefile.am
 index 44c1027..ed6416a 100644
 --- a/test/Makefile.am
 +++ b/test/Makefile.am
 @@ -25,7 +25,7 @@
   # SOFTWARE.
   #
 
 -noinst_PROGRAMS = process-test
 +noinst_PROGRAMS = process-test xserver-test
 
   AM_CPPFLAGS = $(GTEST_CPPFLAGS)
   AM_CXXFLAGS = $(BASE_CXXFLAGS)
 @@ -41,6 +41,10 @@ process_test_SOURCES = process-test.cpp
   process_test_CPPFLAGS = $(AM_CPPFLAGS) -I$(top_srcdir)/include
   process_test_LDADD =  $(tests_libraries)
 
 +xserver_test_SOURCES = xserver-test.cpp
 +xserver_test_CPPFLAGS = $(AM_CPPFLAGS) -I$(top_srcdir)/include
 +xserver_test_LDADD =  $(tests_libraries)
 +
   check_LIBRARIES = libgtest.a libxorg-gtest.a
 
   # build googletest as static lib
 diff --git a/test/xserver-test.cpp b/test/xserver-test.cpp
 new file mode 100644
 index 000..ba6c462
 --- /dev/null
 +++ b/test/xserver-test.cpp
 @@ -0,0 +1,62 @@
 +#include errno.h
 +#include unistd.h
 +#include sys/types.h
 +#include sys/wait.h
 +#include fstream
 +
 +#include gtest/gtest.h
 +#include xorg/gtest/xorg-gtest.h
 +
 +using namespace xorg::testing;
 +
 +TEST(XServer, LogRemoval)
 +{
 +  SCOPED_TRACE(TESTCASE: X server startup and log file removal on success 
 and error);
 +  std::string logfile = /tmp/xorg-testing-xserver_.log;
 +
 +  /* make sure a previous failed test didn't leave it around */
 +  unlink(logfile.c_str());
 +
 +  XServer server;
 +  server.SetOption(-logfile, logfile);
 +  server.Start();
 +  server.Terminate(3000);
 +  server.RemoveLogFile();
 +
 +  std::ifstream file(logfile.c_str());
 +  ASSERT_FALSE(file.good());
 +  file.close();
 +
 +  server.SetOption(-doesnotexist, );
 +  server.Start();
 +  while (server.GetState() == Process::RUNNING)
 +usleep(5000);
 +
 +  ASSERT_EQ(server.GetState(), Process::FINISHED_FAILURE);
 +  file.open(logfile.c_str());
 +  ASSERT_FALSE(file.good()); /* server didn't leave the file behind */
 +
 +  /* now create it */
 +  std::ofstream f(logfile.c_str());
 +  file.open(logfile.c_str());
 +  ASSERT_TRUE(file.good());
 +  file.close();
 +
 +  /* must not remove it now */
 +  server.RemoveLogFile();
 +
 +  file.open(logfile.c_str());
 +  ASSERT_TRUE(file.good()); /* server didn't remove it */
 +  file.close();
 +
 +  server.RemoveLogFile(true);
 +  file.open(logfile.c_str());
 +  ASSERT_FALSE(file.good()); /* server did remove it */
 +  file.close();
 +}
 +
 +
 +int main(int argc, char *argv[]) {
 +  testing::InitGoogleTest(argc, argv);
 +  return RUN_ALL_TESTS();
 +}
 
 I don't see why these tests shouldn't be compiled and linked into
 one binary with the other tests in process-test.cpp. I think it will
 make the tests easier to maintain over time. The simple way to do
 this would be:

The XServer test requires a working installation of X, the process test
doesn't and ideally that stays that way. Playing with gtest_filter is fun
but not that nice as just running specific binaries. I'd prefer to leave it
as-is until we have more tests and actually see a direction, right now it's
more of a bucket to throw tests into. 

Cheers,
   Peter

 * Remove the main() functions from each file
 * Compile gtest_main.cc
 * Link each test object file and gtest_main together
 
 There's nothing wrong with this approach, so I'll give my r-b, but
 the above approach would generate faster test runs and more easily
 analyzed test results in case you want to feed them into something
 like jenkins.
 
 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

Re: [PATCH 5/5] os: don't block signal-unsafe logging, merely warn about it.

[Peter Hutterer]

On Thu, Aug 16, 2012 at 08:20:33AM -0700, Chase Douglas wrote:
 On 08/15/2012 09:52 PM, Peter Hutterer wrote:
 Throw an error into the log file, but continue anyway. And after three
 warnings, stop complaining. Not all input drivers will be fixed in time (or
 ever) and our printf implementation is vastly inferior, so there is still a
 use-case for non-sigsafe logging.
 
 This also adds more linebreaks to the message.
 
 CC: Chase Douglas chase.doug...@canonical.com
 Signed-off-by: Peter Hutterer peter.hutte...@who-t.net
 ---
   os/log.c | 39 +--
   1 file changed, 25 insertions(+), 14 deletions(-)
 
 diff --git a/os/log.c b/os/log.c
 index 4348739..7fca317 100644
 --- a/os/log.c
 +++ b/os/log.c
 @@ -467,6 +467,7 @@ LogMessageTypeVerbString(MessageType type, int verb)
   void
   LogVMessageVerb(MessageType type, int verb, const char *format, va_list 
  args)
   {
 +static unsigned int warned;
   const char *type_str;
   char buf[1024];
   const size_t size = sizeof(buf);
 @@ -474,13 +475,17 @@ LogVMessageVerb(MessageType type, int verb, const char 
 *format, va_list args)
   size_t len = 0;
 
   if (inSignalContext) {
 -BUG_WARN_MSG(inSignalContext,
 - Warning: attempting to log data in a signal unsafe 
 - manner while in signal context. Please update to 
 check 
 - inSignalContext and/or use LogMessageVerbSigSafe() or 
 
 - ErrorFSigSafe(). The offending log format message 
 is:\n
 - %s\n, format);
 -return;
 +if (warned  3) {
 +BUG_WARN_MSG(inSignalContext,
 + Warning: attempting to log data in a signal 
 unsafe 
 + manner while in signal context.\nPlease update to 
 check 
 + inSignalContext and/or use 
 LogMessageVerbSigSafe() or 
 + ErrorFSigSafe().\nThe offending log format 
 message is:\n
 + %s\n, format);
 +warned++;
 +if (warned == 3)
 +LogMessageVerbSigSafe(X_WARNING, -1, Warned %u times about 
 sigsafe logging. Will be quiet now.\n, warned);
 +}
   }
 
   type_str = LogMessageTypeVerbString(type, verb);
 @@ -566,6 +571,7 @@ void
   LogVHdrMessageVerb(MessageType type, int verb, const char *msg_format,
  va_list msg_args, const char *hdr_format, va_list 
  hdr_args)
   {
 +static unsigned int warned;
   const char *type_str;
   char buf[1024];
   const size_t size = sizeof(buf);
 @@ -573,15 +579,20 @@ LogVHdrMessageVerb(MessageType type, int verb, const 
 char *msg_format,
   size_t len = 0;
 
   if (inSignalContext) {
 -BUG_WARN_MSG(inSignalContext,
 - Warning: attempting to log data in a signal unsafe 
 - manner while in signal context. Please update to 
 check 
 - inSignalContext and/or use LogMessageVerbSigSafe(). 
 The 
 - offending header and log message formats are:\n%s 
 %s\n,
 - hdr_format, msg_format);
 -return;
 +if (warned  3) {
 +BUG_WARN_MSG(inSignalContext,
 + Warning: attempting to log data in a signal 
 unsafe 
 + manner while in signal context.\nPlease update to 
 check 
 + inSignalContext and/or use 
 LogMessageVerbSigSafe().\nThe 
 + offending header and log message formats are:\n%s 
 %s\n,
 + hdr_format, msg_format);
 +warned++;
 +if (warned == 3)
 +LogMessageVerbSigSafe(X_WARNING, -1, Warned %u times about 
 sigsafe logging. Will be quiet now.\n, warned);
 +}
   }
 
 +
 
 Stray newline here ^^
 
   type_str = LogMessageTypeVerbString(type, verb);
   if (!type_str)
   return;
 
 
 It's too bad everything won't be fixed in time, but we've gotten
 along this far without too much issue. Hopefully the warnings will
 be big enough to force the issue to a close.

there are _a lot_ of debugging messages in drivers, and some of those
drivers are unmaintained but continue to run anyway. the real issiue is that
the current implementation of pnprintf doesn't even come close to the real
one. no length modifiers, only few conversion specifiers, etc.

so we'd have to go through and audit every single ErrorF, DBG, xf86*Msg*,
etc. in the drivers. that's going to take a while.

 Reviewed-by: Chase Douglas chase.doug...@canonical.com

Thanks.

Cheers,
   Peter

___
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

[PULL] XQuartz logging fixes

[Jeremy Huddleston Sequoia]

The following changes since commit ad5fe2d9614959b68bf71e23abf7e5abac9c2734:

  Merge remote-tracking branch 'jeremyhu/master' (2012-08-15 13:29:17 -0700)

are available in the git repository at:


  git://people.freedesktop.org/~jeremyhu/xserver master

for you to fetch changes up to ac616d8ed5a634c6b32775eb7b071a13c575fd97:

  XQuartz: Use asl_log_descriptor for children as well (2012-08-16 19:44:00 
-0700)


Jeremy Huddleston Sequoia (4):
  XQuartz: console_redirect: Set the correct location for reading into the 
buffer
  XQuartz: console_redirect: Properly zero-out the tail of the array on 
realloc()
  XQuartz: Use asl_log_descriptor on Mountain Lion
  XQuartz: Use asl_log_descriptor for children as well

 hw/xquartz/X11Controller.m| 62 
--
 hw/xquartz/console_redirect.c | 28 +---
 hw/xquartz/mach-startup/bundle-main.c | 17 +
 3 files changed, 94 insertions(+), 13 deletions(-)

___
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

[PATCH keyboard] Use sigsafe logging for keyboard debug messages

[Peter Hutterer]

This changes the log format to simple hex display, the server's signal-safe
printf implementation doesn't handle %2.2x.

Signed-off-by: Peter Hutterer peter.hutte...@who-t.net
---
The second line in hunk 2 is a whitespace removal, I got annoyed by the red
blob (let c_space_errors=1 in vim)

Alan: please test this on Solaris.

 src/at_scancode.c |  4 ++--
 src/kbd.c |  4 ++--
 src/sun_kbd.c | 20 +++-
 src/xf86OSKbd.h   |  4 
 4 files changed, 19 insertions(+), 13 deletions(-)

diff --git a/src/at_scancode.c b/src/at_scancode.c
index 7209c20..a519a6c 100644
--- a/src/at_scancode.c
+++ b/src/at_scancode.c
@@ -112,8 +112,8 @@ ATScancode(InputInfoPtr pInfo, int *scanCode)
 case 0x36:
 return TRUE;
 default:
- xf86MsgVerb(X_INFO, 4, Unreported Prefix0 scancode: 
0x%02x\n,
-*scanCode);
+ LogMessageVerbSigSafe(X_INFO, 4, Unreported Prefix0 
scancode: 0x%x\n,
+   *scanCode);
  *scanCode += 0x78;
   }
break;
diff --git a/src/kbd.c b/src/kbd.c
index 94523aa..9a013b7 100644
--- a/src/kbd.c
+++ b/src/kbd.c
@@ -410,9 +410,9 @@ PostKbdEvent(InputInfoPtr pInfo, unsigned int scanCode, 
Bool down)
   int state;
 
 #ifdef DEBUG
-  ErrorF(kbd driver rec scancode: 0x02%x %s\n, scanCode, down?down:up);
+  LogMessageVerbSigSafe(X_INFO, -1, kbd driver rec scancode: 0x%x %s\n, 
scanCode, down ? down : up);
 #endif
- 
+
   /*
* First do some special scancode remapping ...
*/
diff --git a/src/sun_kbd.c b/src/sun_kbd.c
index f1e530e..2e7add6 100644
--- a/src/sun_kbd.c
+++ b/src/sun_kbd.c
@@ -459,20 +459,22 @@ ReadInput(InputInfoPtr pInfo)
case EINTR:  /* Interrupted, try again */
break;
case ENODEV: /* May happen when USB kbd is unplugged */
-   /* We use X_NONE here because it doesn't alloc since we
-  may be called from SIGIO handler */
-   xf86MsgVerb(X_NONE, 0,
-   %s: Device no longer present - removing.\n,
-   pInfo-name);
+   /* We use X_NONE here because it didn't alloc since we
+  may be called from SIGIO handler. No longer true for
+  sigsafe logging, but matters for older servers  */
+   LogMessageVerbSigSafe(X_NONE, 0,
+ %s: Device no longer present - 
removing.\n,
+ pInfo-name);
xf86RemoveEnabledDevice(pInfo);
priv-remove_timer = TimerSet(priv-remove_timer, 0, 1,
  RemoveKeyboard, pInfo);
return;
default: /* All other errors */
-   /* We use X_NONE here because it doesn't alloc since we
-  may be called from SIGIO handler */
-   xf86MsgVerb(X_NONE, 0, %s: Read error: %s\n, pInfo-name,
-   strerror(errno));
+   /* We use X_NONE here because it didn't alloc since we
+  may be called from SIGIO handler. No longer true for
+  sigsafe logging, but matters for older servers  */
+   LogMessageVerbSigSafe(X_NONE, 0, %s: Read error: %s\n, 
pInfo-name,
+ strerror(errno));
return;
}
} else { /* nBytes == 0, so nothing more to read */
diff --git a/src/xf86OSKbd.h b/src/xf86OSKbd.h
index cba049f..7349b1a 100644
--- a/src/xf86OSKbd.h
+++ b/src/xf86OSKbd.h
@@ -29,6 +29,10 @@
 
 #include xf86Xinput.h
 
+#if GET_ABI_MAJOR(ABI_XINPUT_VERSION)  18
+#define LogMessageVerbSigSafe xf86MsgVerb
+#endif
+
 Bool ATScancode(InputInfoPtr pInfo, int *scanCode);
 
 /* Public interface to OS-specific keyboard support. */
-- 
1.7.11.2

___
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

X.Org Book Sprint 2012 (guide to writing graphics drivers)

[Matt Dew]

(Cross post. Yes, I know this is short notice and a short window to 
reply.   My apologies on that.)


X.Org Book Sprint 2012
Monday Sept 17  Tuesday Sept 18.
Nürnberg (Nuremberg), Germany.

   The X.Org Consortium will hold a book sprint on the Monday and
Tuesday before the Developers Conference in Nürnberg Germany.
During these two days attendees will focus on a book for graphics
drivers development.  The book will be targeted towards new
contributors but will have enough details to be useful to all
levels of graphics driver development.  Anyone interested in
helping out and contributing to the book sprint is encouraged to
get involved.


From http://www.booksprints.net/about/

A Book Sprint brings together a group to produce a book in 3-5 days.
There is no pre-production and the group is guided by a facilitator
from zero to published book. The books produced are high quality
content and are made available immediately at the end of the sprint
via print-on-demand services and e-book formats.

Our book sprint will be slightly different than that.  Our book sprint
is only two days and we are not starting from complete scratch. We are
starting with Marcheu's existing guide, located at:
http://people.freedesktop.org/~marcheu/linuxgraphicsdrivers.pdf

For more information or if you would like to participate in the book
sprint, please contact:
board at foundation.x.org
marcoz at osource.org

Please contact us on or before Mon Aug 20, 2012.

More information can be found at:
http://www.x.org/wiki/Events/BookSprint2012
___
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