[1/2] helpers: Save a copy to the trapped error, not pointer

Submitted by Daniel Stone on Nov. 5, 2012, 4:58 a.m.

Details

Message ID 1352091523-2678-1-git-send-email-daniel@fooishbar.org
State New
Headers show

Not browsing as part of any series.

Commit Message

Daniel Stone Nov. 5, 2012, 4:58 a.m.
SetErrorTrap/ReleaseErrorTrap were storing a pointer to the error,
rather than a copy of it.  It was pretty liable to getting trashed, so
instead store a copy of our error.

Signed-off-by: Daniel Stone <daniel@fooishbar.org>
---
 tests/common/helpers.cpp |   10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

Patch hide | download patch | download mbox

diff --git a/tests/common/helpers.cpp b/tests/common/helpers.cpp
index f48a134..3803d26 100644
--- a/tests/common/helpers.cpp
+++ b/tests/common/helpers.cpp
@@ -132,9 +132,9 @@  void DeviceSetEnabled(Display *dpy, int deviceid, bool enabled)
 /* Basic error trapping */
 static struct {
     bool is_trapping;
-    XErrorEvent *error;
+    XErrorEvent error;
     XErrorHandler prev_error_handler;
-} trap_state = { False, NULL, NULL };
+} trap_state;
 
 static int
 ErrorHandler(Display *dpy,
@@ -143,7 +143,7 @@  ErrorHandler(Display *dpy,
     if (!trap_state.is_trapping)
         ADD_FAILURE() << "Error trap received error while not trapping. WTF?";
 
-    trap_state.error = error;
+    trap_state.error = *error;
     return 0;
 }
 
@@ -154,6 +154,7 @@  void SetErrorTrap(Display *dpy) {
     XSync(dpy, False);
     trap_state.prev_error_handler = XSetErrorHandler(ErrorHandler);
     trap_state.is_trapping = True;
+    memset(&trap_state.error, 0, sizeof(trap_state.error));
 }
 
 XErrorEvent * ReleaseErrorTrap(Display *dpy) {
@@ -164,8 +165,7 @@  XErrorEvent * ReleaseErrorTrap(Display *dpy) {
     XSetErrorHandler(trap_state.prev_error_handler);
     trap_state.prev_error_handler = NULL;
 
-    XErrorEvent *error = trap_state.error;
-    trap_state.error = NULL;
+    XErrorEvent *error = &trap_state.error;
     trap_state.is_trapping = False;
     return error;
 }

Comments

On Mon, Nov 05, 2012 at 03:58:42PM +1100, Daniel Stone wrote:
> SetErrorTrap/ReleaseErrorTrap were storing a pointer to the error,
> rather than a copy of it.  It was pretty liable to getting trashed, so
> instead store a copy of our error.
> 
> Signed-off-by: Daniel Stone <daniel@fooishbar.org>
> ---
>  tests/common/helpers.cpp |   10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/tests/common/helpers.cpp b/tests/common/helpers.cpp
> index f48a134..3803d26 100644
> --- a/tests/common/helpers.cpp
> +++ b/tests/common/helpers.cpp
> @@ -132,9 +132,9 @@ void DeviceSetEnabled(Display *dpy, int deviceid, bool enabled)
>  /* Basic error trapping */
>  static struct {
>      bool is_trapping;
> -    XErrorEvent *error;
> +    XErrorEvent error;
>      XErrorHandler prev_error_handler;
> -} trap_state = { False, NULL, NULL };
> +} trap_state;
>  
>  static int
>  ErrorHandler(Display *dpy,
> @@ -143,7 +143,7 @@ ErrorHandler(Display *dpy,
>      if (!trap_state.is_trapping)
>          ADD_FAILURE() << "Error trap received error while not trapping. WTF?";
>  
> -    trap_state.error = error;
> +    trap_state.error = *error;
>      return 0;
>  }
>  
> @@ -154,6 +154,7 @@ void SetErrorTrap(Display *dpy) {
>      XSync(dpy, False);
>      trap_state.prev_error_handler = XSetErrorHandler(ErrorHandler);
>      trap_state.is_trapping = True;
> +    memset(&trap_state.error, 0, sizeof(trap_state.error));
>  }
>  
>  XErrorEvent * ReleaseErrorTrap(Display *dpy) {
> @@ -164,8 +165,7 @@ XErrorEvent * ReleaseErrorTrap(Display *dpy) {
>      XSetErrorHandler(trap_state.prev_error_handler);
>      trap_state.prev_error_handler = NULL;
>  
> -    XErrorEvent *error = trap_state.error;
> -    trap_state.error = NULL;
> +    XErrorEvent *error = &trap_state.error;

unless I'm missing something, this will always return an address instead of
NULL if there is no error. I've got a similar patch, coming up in a sec.

Cheers,
   Peter

>      trap_state.is_trapping = False;
>      return error;
>  }
> -- 
> 1.7.10.4