Factor out duplicated WindowOptPtr initialization.

By using the same code for both MakeWindowOptional and CreateRootWindow,
we ensure that new WindowOptPtrs are always fully initialized.

Previously, CreateRootWindow did not initialize optional->cursor. It
relied on InitRootWindow to overwrite it later, which seems like a wild
pointer dereference just waiting to happen.

Signed-off-by: Jamey Sharp <jamey@minilop.net>

1 files changed, 35 insertions, 39 deletions

diff --git a/dix/window.c b/dix/window.c
index 44bfa182a..c87020dff 100644
--- a/dix/window.c
+++ b/dix/window.c
@@ -152,6 +152,12 @@ static unsigned char _back_msb[4] = {0x11, 0x44, 0x22, 0x88};
 static Bool WindowParentHasDeviceCursor(WindowPtr pWin, 
                                         DeviceIntPtr pDev, 
                                         CursorPtr pCurs);
+static WindowOptPtr
+AllocateWindowOptional(CursorPtr cursor,
+                       VisualID visual,
+                       Colormap colormap,
+                       Mask dontPropagateMask);
+
 static Bool 
 WindowSeekDeviceCursor(WindowPtr pWin, 
                        DeviceIntPtr pDev, 
@@ -481,25 +487,11 @@ CreateRootWindow(ScreenPtr pScreen)
     pWin->parent = NullWindow;
     SetWindowToDefaults(pWin);
 
-    pWin->optional = malloc(sizeof (WindowOptRec));
+    pWin->optional = AllocateWindowOptional(None,
+            pScreen->rootVisual, pScreen->defColormap, 0);
     if (!pWin->optional)
         return FALSE;
 
-    pWin->optional->dontPropagateMask = 0;
-    pWin->optional->otherEventMasks = 0;
-    pWin->optional->otherClients = NULL;
-    pWin->optional->passiveGrabs = NULL;
-    pWin->optional->userProps = NULL;
-    pWin->optional->backingBitPlanes = ~0L;
-    pWin->optional->backingPixel = 0;
-    pWin->optional->boundingShape = NULL;
-    pWin->optional->clipShape = NULL;
-    pWin->optional->inputShape = NULL;
-    pWin->optional->inputMasks = NULL;
-    pWin->optional->deviceCursors = NULL;
-    pWin->optional->colormap = pScreen->defColormap;
-    pWin->optional->visual = pScreen->rootVisual;
-
     pWin->nextSib = NullWindow;
 
     pWin->drawable.id = FakeClientID(0);
@@ -3500,18 +3492,21 @@ CheckWindowOptionalNeed (WindowPtr w)
  * values.
  */
 
-Bool
-MakeWindowOptional (WindowPtr pWin)
+static WindowOptPtr
+AllocateWindowOptional (CursorPtr cursor,
+                        VisualID visual,
+                        Colormap colormap,
+                        Mask dontPropagateMask)
 {
-    WindowOptPtr optional;
-    WindowOptPtr parentOptional;
-
-    if (pWin->optional)
-	return TRUE;
-    optional = malloc(sizeof (WindowOptRec));
+    WindowOptPtr optional = malloc(sizeof (WindowOptRec));
     if (!optional)
-	return FALSE;
-    optional->dontPropagateMask = DontPropagateMasks[pWin->dontPropagate];
+	return NULL;
+    optional->cursor = cursor;
+    if (cursor)
+	cursor->refcnt++;
+    optional->visual = visual;
+    optional->colormap = colormap;
+    optional->dontPropagateMask = dontPropagateMask;
     optional->otherEventMasks = 0;
     optional->otherClients = NULL;
     optional->passiveGrabs = NULL;
@@ -3523,21 +3518,22 @@ MakeWindowOptional (WindowPtr pWin)
     optional->inputShape = NULL;
     optional->inputMasks = NULL;
     optional->deviceCursors = NULL;
+    return optional;
+}
 
+Bool
+MakeWindowOptional (WindowPtr pWin)
+{
+    WindowOptPtr parentOptional;
+
+    if (pWin->optional)
+	return TRUE;
     parentOptional = FindWindowWithOptional(pWin)->optional;
-    optional->visual = parentOptional->visual;
-    if (!pWin->cursorIsNone)
-    {
-	optional->cursor = parentOptional->cursor;
-	optional->cursor->refcnt++;
-    }
-    else
-    {
-	optional->cursor = None;
-    }
-    optional->colormap = parentOptional->colormap;
-    pWin->optional = optional;
-    return TRUE;
+    pWin->optional = AllocateWindowOptional(
+	pWin->cursorIsNone ? None : parentOptional->cursor,
+	parentOptional->visual, parentOptional->colormap,
+	DontPropagateMasks[pWin->dontPropagate]);
+    return pWin->optional != NULL;
 }
 
 /*