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

[Peter Hutterer]

On Wed, Oct 08, 2008 at 09:12:44AM +0200, Simon Thum wrote:
> Peter Hutterer wrote:
> > Axis range changes are one example I can think of. The driver needs to update
> > it for axis inversion, the server for scaling.
> Hmm - that essentially forbids moving inversion into the server (which
> could be done), because we'd never know if the driver wouldn't also do
> it.

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.
 
> > No, this doesn't help if handler 1 reports "properly changed" and handler 2
> > reports a BadValue (for whatever reason).
> Yep, that's why it would need to be complemented with a
> only-one-non-ignoring-handler policy. This would foster clear separation
> and shouldn't be problematic to introduce in the early stages.

right. but then you'd need a method to figure out whether your handler is the
one that is allowed to return an error code. Alternatively, the server could
just stop processing handlers after the first "active" handler. In which case
you can have a driver that needs to access a property but is never called.
Both cases are not quite optimal.

> > Since the property handlers don't change the actual property value (that's
> > done in the server on Success), "Don't care" is essentially the same as
> > "properly changed" to the server.
> Yes, but the actual propery value may not be what reflects the state in
> the server, e.g. as in the 'Enabled' prop. So there is a difference.

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.

> Not that your design is bad - I just think we should keep complexity out
> of handlers. They're what's going to multiply. 2-phase is good in
> general, but IMO overkill.

I don't quite see the overkill there. All the evdev code looks basically

if (values_are_bananas)
   return BadSomething;

if (!checkonly) { // this line got added
  foo();
}

I can't imagine it being much simpler than that.

> PS: Thanks for attributing the scaling stuff to me, but that really
> hadn't been necessary.

This way git-blame points at you if it breaks :)
I think I just ammended from the rounding patch you sent, never bothered to
change the author.

Cheers,
  Peter