[PATCH] Xi: check all handlers before applying property changes.

[Peter Hutterer]

On Wed, Oct 08, 2008 at 11:05:07PM +0200, Simon Thum wrote:
> > Not necessarily. Once the server has support for X, we can just remove it from
> > the driver and say that you need at least server Y with this driver version.
> > Or, once the server has a "XYZ" property, you can just make e.g. "Evdev XYZ"
> > an alias for "XYZ", keeping backwards compat with clients.
> We're arguing in different directions here. My point is, that this all
> is superfluous if you avoid double-defined props in the first place.
> Enforcing that somewhat helps to resist the temptation :)

So your point is to never have properties in the driver that could be in the
server? This may be tough, as drivers have a quicker turnover time, and
features can get added easier.

> The question is whether one needs it. Your last major change was all
> about reducing complexity. You could go further in that direction by
> adopting only-one-non-ignoring-handler - at the expense of not
> supporting some (IMO questionable) useages.

I agree with the example being of questionable use. However, as we just
started with the whole thing I don't think we can quite grasp what the next
few months may bring in terms of property support. And I wouldn't be surprised
if a use-case pops up where we need that.

One other example I came up with while writing the email: some property
that needs handling in the DIX and one of the DDXes. This would require two
separate handlers, the code can't be merged into one.

> > if a property does not reflect the state as it is in the server, I'd consider
> > this a bug. Regarding the example: the "enabled" prop actually calls
> > Enable/DisableDevice, so it should always be the server state.
> Sure, we can reasonably assume that. The mere point is the the state is
> not determined by the prop, just reflected (e.g. someone else could call
> EnableDevice, like VTSwitch or power mgmt). This small difference is
> what I'd say makes for a difference in return codes. (Plus, pretty
> unrelated: your only chance of knowing is having a getter).

So the theory is that whatever changes state in the server _MUST_ also update
the respective property. In the given example, not updating the property is
akin to not setting device->enabled = FALSE when disabling the device.
If the state is changed through changing the property - easy. Otherwise, the
code must ensure that XIChangeDeviceProperty() is called.

> Overkill was mainly referring to the possibilities it opens (n-times
> handled props) and the associated complexity, which would inevitably
> affect property users (like you and me).
 
There's two types of users: clients and server/driver developers. For clients
the handlers are invisible anyway, and either it succeeds or it doesn't - in
which case we get an error value. And I hope that any property is reasonably
documented so that one knows what values are legit :)

Developers, well, I still think that there is little complexity. 
Register a handler, check only on the first call, change state on the second
call. We don't have to coordinate with other handlers, the second run simply
never starts if another handler disagrees.

Sure, it may get complicated if we have 10 handlers for each property. But at
that point we should got back to the drawing board and see what we have done
wrong. Because I don't think that merging the code from 10 different handlers
into one handler would be any more straightforward.

> So, I pushed that far enough - it is your decision, and I can live with
> whatever you end up with.

My current decision is to use the checkonly flag. It comes at the cost of a
single parameter and an if condition for each property. Not using it comes at
the cost of maybe needing to break the ABI later, when it may already be too
late (ABI freeze, and whatnot).

Cheers,
  Peter