[PATCH] xnest: Fix cursor handling with multiple screens (v1.1)

Adam Jackson ajax at redhat.com
Tue Dec 8 09:47:35 PST 2015


Starting Xnest with (say) -scrns 2 and running xdpyinfo against it
results in a crash on cursor cleanup:

    (EE) Backtrace:
    (EE) 0: ./hw/xnest/Xnest (OsSigHandler+0x29) [0x468ba9]
    (EE) 1: /lib64/libc.so.6 (__restore_rt+0x0) [0x30d7034a4f]
    (EE) 2: ./hw/xnest/Xnest (xnestUnrealizeCursor+0x31) [0x419a81]
    (EE) 3: ./hw/xnest/Xnest (FreeCursor+0x71) [0x425b21]
    (EE) 4: ./hw/xnest/Xnest (doFreeResource+0x62) [0x453c22]
    (EE) 5: ./hw/xnest/Xnest (FreeClientResources+0x97) [0x454cc7]
    (EE) 6: ./hw/xnest/Xnest (FreeAllResources+0x47) [0x454d77]
    (EE) 7: ./hw/xnest/Xnest (dix_main+0x3fe) [0x434e7e]
    (EE) 8: /lib64/libc.so.6 (__libc_start_main+0xf0) [0x30d7020700]
    (EE) 9: ./hw/xnest/Xnest (_start+0x29) [0x417de9]
    (EE) 10: ? (?+0x29) [0x29]
    (EE)
    (EE) Segmentation fault at address 0x0

The cursor private handling here was fairly stupid anyway, mallocing a
struct just to hold a single XID is wasteful.  Fix this by storing the
XID directly in the private slot.

v1.1: Better commit message explanation

Bugzilla: https://bugs.freedesktop.org/27845
Signed-off-by: Adam Jackson <ajax at redhat.com>
---
 hw/xnest/Cursor.c   | 30 ++++++++++++++++++++++++------
 hw/xnest/XNCursor.h | 13 -------------
 2 files changed, 24 insertions(+), 19 deletions(-)

diff --git a/hw/xnest/Cursor.c b/hw/xnest/Cursor.c
index 285e10e..3ee0472 100644
--- a/hw/xnest/Cursor.c
+++ b/hw/xnest/Cursor.c
@@ -36,6 +36,19 @@ is" without express or implied warranty.
 #include "Keyboard.h"
 #include "Args.h"
 
+static Cursor *
+xnestCursorAddr(CursorPtr pCursor, ScreenPtr pScreen)
+{
+    return (Cursor *)dixLookupScreenPrivateAddr(&pCursor->devPrivates,
+                                                CursorScreenKey, pScreen);
+}
+
+static Cursor
+xnestCursor(CursorPtr pCursor, ScreenPtr pScreen)
+{
+    return *xnestCursorAddr(pCursor, pScreen);
+}
+
 xnestCursorFuncRec xnestCursorFuncs = { NULL };
 
 Bool
@@ -46,6 +59,7 @@ xnestRealizeCursor(DeviceIntPtr pDev, ScreenPtr pScreen, CursorPtr pCursor)
     XColor fg_color, bg_color;
     unsigned long valuemask;
     XGCValues values;
+    Cursor *cursor;
 
     valuemask = GCFunction |
         GCPlaneMask | GCForeground | GCBackground | GCClipMask;
@@ -98,10 +112,10 @@ xnestRealizeCursor(DeviceIntPtr pDev, ScreenPtr pScreen, CursorPtr pCursor)
     bg_color.green = pCursor->backGreen;
     bg_color.blue = pCursor->backBlue;
 
-    xnestSetCursorPriv(pCursor, pScreen, malloc(sizeof(xnestPrivCursor)));
-    xnestCursor(pCursor, pScreen) =
-        XCreatePixmapCursor(xnestDisplay, source, mask, &fg_color, &bg_color,
-                            pCursor->bits->xhot, pCursor->bits->yhot);
+    cursor = xnestCursorAddr(pCursor, pScreen);
+    *cursor = XCreatePixmapCursor(xnestDisplay, source, mask,
+                                  &fg_color, &bg_color,
+                                  pCursor->bits->xhot, pCursor->bits->yhot);
 
     XFreePixmap(xnestDisplay, source);
     XFreePixmap(xnestDisplay, mask);
@@ -112,8 +126,12 @@ xnestRealizeCursor(DeviceIntPtr pDev, ScreenPtr pScreen, CursorPtr pCursor)
 Bool
 xnestUnrealizeCursor(DeviceIntPtr pDev, ScreenPtr pScreen, CursorPtr pCursor)
 {
-    XFreeCursor(xnestDisplay, xnestCursor(pCursor, pScreen));
-    free(xnestGetCursorPriv(pCursor, pScreen));
+    Cursor cursor = xnestCursor(pCursor, pScreen);
+
+    if (cursor != None)
+        XFreeCursor(xnestDisplay, cursor);
+    dixSetScreenPrivate(&pCursor->devPrivates, CursorScreenKey, pScreen, NULL);
+
     return True;
 }
 
diff --git a/hw/xnest/XNCursor.h b/hw/xnest/XNCursor.h
index 1a3c6f4..efb2b43 100644
--- a/hw/xnest/XNCursor.h
+++ b/hw/xnest/XNCursor.h
@@ -26,19 +26,6 @@ extern DevPrivateKeyRec xnestCursorScreenKeyRec;
 #define xnestCursorScreenKey (&xnestCursorScreenKeyRec)
 extern xnestCursorFuncRec xnestCursorFuncs;
 
-typedef struct {
-    Cursor cursor;
-} xnestPrivCursor;
-
-#define xnestGetCursorPriv(pCursor, pScreen) ((xnestPrivCursor *) \
-    dixLookupScreenPrivate(&(pCursor)->devPrivates, CursorScreenKey, pScreen))
-
-#define xnestSetCursorPriv(pCursor, pScreen, v) \
-    dixSetScreenPrivate(&(pCursor)->devPrivates, CursorScreenKey, pScreen, v)
-
-#define xnestCursor(pCursor, pScreen) \
-  (xnestGetCursorPriv(pCursor, pScreen)->cursor)
-
 Bool xnestRealizeCursor(DeviceIntPtr pDev,
                         ScreenPtr pScreen, CursorPtr pCursor);
 Bool xnestUnrealizeCursor(DeviceIntPtr pDev,
-- 
2.5.0



More information about the xorg-devel mailing list