Xi: check all handlers before applying property changes.

The current code exposes to inconsistent updates, i.e. if handler N succeeds but handler N+1 fails in setting the property, an error is returned to the client although parts of the server now behave as if the property change succeeded. This patch adds a "checkonly" parameter to the SetProperty handler. The handlers are then called twice, once with checkonly set to TRUE. On the checkonly run, handlers _MUST_ return error codes if the property cannot be applied. Handlers are not permitted to actually apply the changes. On the second run, handlers are permitted to apply property changes. Errors codes returned on the second run are ignored.
juergbi · Oct 13, 2008 · f3f6ea8 · f3f6ea8
1 parent ad67e3f
commit f3f6ea8
Show file tree

Hide file tree

Showing 4 changed files with 35 additions and 19 deletions.
diff --git a/Xi/xiproperty.c b/Xi/xiproperty.c
@@ -97,7 +97,8 @@ long
 XIRegisterPropertyHandler(DeviceIntPtr         dev,
                           int (*SetProperty) (DeviceIntPtr dev,
                                               Atom property,
-                                              XIPropertyValuePtr prop),
+                                              XIPropertyValuePtr prop,
+                                              BOOL checkonly),
                           int (*GetProperty) (DeviceIntPtr dev,
                                               Atom property),
                           int (*DeleteProperty) (DeviceIntPtr dev,
@@ -346,22 +347,31 @@ XIChangeDeviceProperty (DeviceIntPtr dev, Atom property, Atom type,
 
         if (dev->properties.handlers)
         {
-            XIPropertyHandlerPtr handler = dev->properties.handlers;
-            while(handler)
+            XIPropertyHandlerPtr handler;
+            BOOL checkonly = TRUE;
+            /* run through all handlers with checkonly TRUE, then again with
+             * checkonly FALSE. Handlers MUST return error codes on the
+             * checkonly run, errors on the second run are ignored */
+            do
             {
-                if (handler->SetProperty)
+                handler = dev->properties.handlers;
+                while(handler)
                 {
-                    rc = handler->SetProperty(dev, prop->propertyName,
-                                              &new_value);
-                    if (rc != Success)
+                    if (handler->SetProperty)
                     {
-                        if (new_value.data)
-                            xfree (new_value.data);
-                        return (rc);
+                        rc = handler->SetProperty(dev, prop->propertyName,
+                                &new_value, checkonly);
+                        if (checkonly && rc != Success)
+                        {
+                            if (new_value.data)
+                                xfree (new_value.data);
+                            return (rc);
+                        }
                     }
+                    handler = handler->next;
                 }
-                handler = handler->next;
-            }
+                checkonly = !checkonly;
+            } while (!checkonly);
         }
         if (prop_value->data)
             xfree (prop_value->data);

diff --git a/dix/devices.c b/dix/devices.c
@@ -103,17 +103,21 @@ DevPrivateKey UnusedClassesPrivateKey = &UnusedClassesPrivateKeyIndex;
  * DIX property handler.
  */
 static int
-DeviceSetProperty(DeviceIntPtr dev, Atom property, XIPropertyValuePtr prop)
+DeviceSetProperty(DeviceIntPtr dev, Atom property, XIPropertyValuePtr prop,
+                  BOOL checkonly)
 {
     if (property == XIGetKnownProperty(XI_PROP_ENABLED))
     {
         if (prop->format != 8 || prop->type != XA_INTEGER || prop->size != 1)
             return BadValue;
 
-        if ((*((CARD8*)prop->data)) && !dev->enabled)
-            EnableDevice(dev);
-        else if (!(*((CARD8*)prop->data)) && dev->enabled)
-            DisableDevice(dev);
+        if (!checkonly)
+        {
+            if ((*((CARD8*)prop->data)) && !dev->enabled)
+                EnableDevice(dev);
+            else if (!(*((CARD8*)prop->data)) && dev->enabled)
+                DisableDevice(dev);
+        }
     }
 
     return Success;

diff --git a/include/exevents.h b/include/exevents.h
@@ -232,7 +232,8 @@ extern long XIRegisterPropertyHandler(
         DeviceIntPtr         dev,
         int (*SetProperty) (DeviceIntPtr dev,
                             Atom property,
-                            XIPropertyValuePtr prop),
+                            XIPropertyValuePtr prop,
+                            BOOL checkonly),
         int (*GetProperty) (DeviceIntPtr dev,
                             Atom property),
         int (*DeleteProperty) (DeviceIntPtr dev,

diff --git a/include/inputstr.h b/include/inputstr.h
@@ -367,7 +367,8 @@ typedef struct _XIPropertyHandler
     long id;
     int (*SetProperty) (DeviceIntPtr dev,
                         Atom property,
-                        XIPropertyValuePtr prop);
+                        XIPropertyValuePtr prop,
+                        BOOL checkonly);
     int (*GetProperty) (DeviceIntPtr dev,
                         Atom property);
     int (*DeleteProperty) (DeviceIntPtr dev,