xfree86: modesetting: use Xace instead of highjacking request handler vector

Highjacking the ChangeProperty and DeleteProperty vectors is a pretty fragile
and clumpsy way to receive window property updates. Consider eg. multiple
drivers and hotplugging: when they're unplugged in different order than they've
been plugged (and both drivers doing that hack), who knows what happens.

Instead use the hook infrastructure we already have since 1997: Xace.

Listening on property changes that way is pretty simple, doesn't suffer from
the mentioned problems and only takes 1/4 of the code.

Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>

Xext/xace.c

@@ -280,3 +280,15 @@ XaceIsLocal(ClientPtr client)
 {
     return ClientIsLocal(client);
 }
+
+Bool
+XaceRegisterCallback(int hook, CallbackProcPtr callback, void *data)
+{
+    return AddCallback(XaceHooks+(hook), callback, data);
+}
+
+Bool
+XaceDeleteCallback(int hook, CallbackProcPtr callback, void *data)
+{
+    return DeleteCallback(XaceHooks+(hook), callback, data);
+}

Xext/xace.h

@@ -58,11 +58,11 @@ CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
 extern CallbackListPtr XaceHooks[XACE_NUM_HOOKS];
 
 /* Entry point for hook functions.  Called by Xserver.
- * Required by libdbe and libextmod
+ * Required by several modules
  */
-/* needs to be exported for in-tree modsetting driver, but not part 
-   of public API for external modules */
 _X_EXPORT int XaceHook(int hook, ... /* appropriate args for hook */);
+_X_EXPORT Bool XaceRegisterCallback(int hook, CallbackProcPtr callback, void *data);
+_X_EXPORT Bool XaceDeleteCallback(int hook, CallbackProcPtr callback, void *data);
 
 /* determine whether any callbacks are present for the XACE hook */
 int XaceHookIsSet(int hook);
@@ -96,16 +96,7 @@ int XaceHookScreensaverAccess(ClientPtr client, ScreenPtr screen, Mask access_mo
 int XaceHookAuthAvail(ClientPtr client, XID authId);
 int XaceHookKeyAvail(xEventPtr ev, DeviceIntPtr dev, int count);
 
-
+/* Register / unregister a callback for a given hook. */
-/* Register a callback for a given hook.
- */
-#define XaceRegisterCallback(hook,callback,data) \
-    AddCallback(XaceHooks+(hook), callback, data)
-
-/* Unregister an existing callback for a given hook.
- */
-#define XaceDeleteCallback(hook,callback,data) \
-    DeleteCallback(XaceHooks+(hook), callback, data)
 
 /* XTrans wrappers for use by security modules
  */

hw/xfree86/drivers/modesetting/driver.c

@@ -43,6 +43,7 @@
 #include <X11/extensions/Xv.h>
 
 #include "dix/dix_priv.h"
+#include "Xext/xacestr.h"
 
 #include "xf86.h"
 #include "xf86Priv.h"
@@ -86,14 +87,10 @@ static Bool ms_pci_probe(DriverPtr driver,
                          int entity_num, struct pci_device *device,
                          intptr_t match_data);
 static Bool ms_driver_func(ScrnInfoPtr scrn, xorgDriverFuncOp op, void *data);
+static void ms_property_access(CallbackListPtr *pcbl, void* unused, void *calldata);
 
-/* window wrapper functions used to get the notification when
- * the window property changes */
 static Atom vrr_atom;
-static Bool property_vectors_wrapped;
+static int activeScreens = 0;
-static Bool restore_property_vector;
-static int (*saved_change_property) (ClientPtr client);
-static int (*saved_delete_property) (ClientPtr client);
 
 #ifdef XSERVER_LIBPCIACCESS
 static const struct pci_id_match ms_device_match[] = {
@@ -918,110 +915,22 @@ ms_vrr_property_update(WindowPtr window, Bool variable_refresh)
         ms_present_set_screen_vrr(scrn, variable_refresh);
 }
 
-/* Wrapper for xserver/dix/property.c:ProcChangeProperty */
-static int
-ms_change_property(ClientPtr client)
-{
-    WindowPtr window = NULL;
-    int ret = 0;
-
-    REQUEST(xChangePropertyReq);
-
-    client->requestVector[X_ChangeProperty] = saved_change_property;
-    ret = saved_change_property(client);
-
-    if (restore_property_vector)
-        return ret;
-
-    client->requestVector[X_ChangeProperty] = ms_change_property;
-
-    if (ret != Success)
-        return ret;
-
-    ret = dixLookupWindow(&window, stuff->window, client, DixSetPropAccess);
-    if (ret != Success)
-        return ret;
-
-    // Checking for the VRR property change on the window
-    if (stuff->property == vrr_atom &&
-        xf86ScreenToScrn(window->drawable.pScreen)->PreInit == PreInit &&
-        stuff->format == 32 && stuff->nUnits == 1) {
-        uint32_t *value = (uint32_t *)(stuff + 1);
-        ms_vrr_property_update(window, *value != 0);
-    }
-
-    return ret;
-}
-
-/* Wrapper for xserver/dix/property.c:ProcDeleteProperty */
-static int
-ms_delete_property(ClientPtr client)
-{
-    WindowPtr window;
-    int ret;
-
-    REQUEST(xDeletePropertyReq);
-
-    client->requestVector[X_DeleteProperty] = saved_delete_property;
-    ret = saved_delete_property(client);
-
-    if (restore_property_vector)
-        return ret;
-
-    client->requestVector[X_DeleteProperty] = ms_delete_property;
-
-    if (ret != Success)
-        return ret;
-
-    ret = dixLookupWindow(&window, stuff->window, client, DixSetPropAccess);
-    if (ret != Success)
-        return ret;
-
-    if (stuff->property == vrr_atom &&
-        xf86ScreenToScrn(window->drawable.pScreen)->PreInit == PreInit)
-        ms_vrr_property_update(window, FALSE);
-
-    return ret;
-}
-
 static void
-ms_unwrap_property_requests(ScrnInfoPtr scrn)
+ms_property_access(CallbackListPtr *pcbl, void* unused, void *calldata)
 {
-    int i;
+    XacePropertyAccessRec *rec = calldata;
+    PropertyPtr pProp = *(rec->ppProp);
 
-    if (!property_vectors_wrapped)
+    // our property ? window on screen handled by us ?
+    if (!(pProp->propertyName == vrr_atom) &&
+         (xf86ScreenToScrn(rec->pWin->drawable.pScreen)->PreInit == PreInit))
         return;
 
-    if (ProcVector[X_ChangeProperty] == ms_change_property)
+    if (rec->access_mode & DixDestroyAccess)
-        ProcVector[X_ChangeProperty] = saved_change_property;
+        ms_vrr_property_update(rec->pWin, FALSE);
-    else
-        restore_property_vector = TRUE;
 
-    if (ProcVector[X_DeleteProperty] == ms_delete_property)
+    if ((rec->access_mode & DixWriteAccess) && (pProp->format == 32) && (pProp->size == 1))
-        ProcVector[X_DeleteProperty] = saved_delete_property;
+        ms_vrr_property_update(rec->pWin, (*(uint32_t *)(pProp->data)) != 0);
-    else
-        restore_property_vector = TRUE;
-
-    for (i = 0; i < currentMaxClients; i++) {
-        if (clients[i]->requestVector[X_ChangeProperty] == ms_change_property) {
-            clients[i]->requestVector[X_ChangeProperty] = saved_change_property;
-        } else {
-            restore_property_vector = TRUE;
-        }
-
-        if (clients[i]->requestVector[X_DeleteProperty] == ms_delete_property) {
-            clients[i]->requestVector[X_DeleteProperty] = saved_delete_property;
-        } else {
-            restore_property_vector = TRUE;
-        }
-    }
-
-    if (restore_property_vector) {
-        xf86DrvMsg(scrn->scrnIndex, X_WARNING,
-                   "Couldn't unwrap some window property request vectors\n");
-    }
-
-    property_vectors_wrapped = FALSE;
 }
 
 static void
@@ -1043,7 +952,6 @@ FreeRec(ScrnInfoPtr pScrn)
         ms_ent = ms_ent_priv(pScrn);
         ms_ent->fd_ref--;
         if (!ms_ent->fd_ref) {
-            ms_unwrap_property_requests(pScrn);
             if (ms->pEnt->location.type == BUS_PCI)
                 ret = drmClose(ms->fd);
             else
@@ -2194,17 +2102,15 @@ ScreenInit(ScreenPtr pScreen, int argc, char **argv)
     pScrn->vtSema = TRUE;
 
     if (ms->vrr_support) {
-        if (!property_vectors_wrapped) {
-            saved_change_property = ProcVector[X_ChangeProperty];
-            ProcVector[X_ChangeProperty] = ms_change_property;
-            saved_delete_property = ProcVector[X_DeleteProperty];
-            ProcVector[X_DeleteProperty] = ms_delete_property;
-            property_vectors_wrapped = TRUE;
-        }
         vrr_atom = MakeAtom("_VARIABLE_REFRESH",
                              strlen("_VARIABLE_REFRESH"), TRUE);
     }
 
+    /* first one: register Xace hook for Window property updates */
+    if (!activeScreens)
+        XaceRegisterCallback(XACE_PROPERTY_ACCESS, ms_property_access, NULL);
+    activeScreens++;
+
     return TRUE;
 }
 
@@ -2289,6 +2195,12 @@ CloseScreen(ScreenPtr pScreen)
     modesettingPtr ms = modesettingPTR(pScrn);
     modesettingEntPtr ms_ent = ms_ent_priv(pScrn);
 
+    activeScreens--;
+    if (activeScreens < 1) {
+        XaceDeleteCallback(XACE_PROPERTY_ACCESS, ms_property_access, NULL);
+        activeScreens = 0;
+    }
+
     /* Clear mask of assigned crtc's in this generation */
     ms_ent->assigned_crtcs = 0;