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

[Peter Hutterer]

On Fri, Oct 10, 2008 at 11:54:56AM +0200, Simon Thum wrote:
> > If so, a few questions:
> > - A handler that registers at the head of the queue, assumes it is before
> >   other handlers. Thus it has to be aware of other handlers in the first
> >   place?
> Yes, that would be the uncommon case, in which we should be well aware
> we're doing something special.
> 
> > - Two handlers that place themselves at the head of the queue now both assume
> >   they are called first?
> That's an assumption you shouldn't make. You can make the assumption
> that you're called before the others already in.
> 
> > - What if the handler order has to be different for each property?
> You need two handlers. Keep in mind handlers may well handle different
> property sets, so order can't depend on the prop alone.

So in light of the three answers here, I don't understand how this is an
improvement to the proposed patch that sparked this thread.
The patch I proposed does not make any guarantee over when a handler is
called. This automatically separates handlers so they cannot interfere with
each other, and dependency ordering is a nonissue, as you cannot rely on it to
begin with. Code that relies on other handlers should not end up in the
repository. 

> > - if myHandler allows pass-through after changing state, what if otherHandler
> >   returns an error code?
> That's indeed a problem. You shouldn't do it :)
> Either you call the rest first, if that's impossible, you should be
> prepared to undo stuff.
> 
> [...]
> 
> You also might want to consider this:
> http://www.cs.clemson.edu/~malloy/courses/patterns/chain.html

I think we're disagreeing over the underlying purpose of handlers.
In my view, a property handler is a simple piece of code that changes
some local settings. e.g. the emulateWheel boolean in the evdev driver.
Due to the local nature, you may need multiple handlers simultaneously.
e.g. a "debug" property, that triggers debugging spew in the driver, in the
dix, and in the ddx.

Right now, we pass into each handler and may end up with inconsistent state if
it fails halfway through.
With the proposed patch, we simply check first if _all_ handlers can deal with
the property change. If not, nothing happens and an error is returned. Only if
all handlers guarantee that the change will work, they are instructed to
perform the change. At this point, failure can only be caused by bugs.

The chain of command either limits yourself to one handler, or you require
undoability in the handlers, complicating code that should only trigger a few
flags to be set. I don't disagree with the design pattern, I just don't think
it is the right one for the purpose.

Cheers,
  Peter