Now that the brackets are always the nearest change points (regardless of
transition) we need to update the counters whenever we check for any updates.
Otherwise we end up with a situation where counter->value is out of date and
an alarm doesn't trigger because we're still using the value from last time
something actually triggered.
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Reviewed-by: Adam Jackson <ajax@redhat.com>
Reviewed-by: Keith Packard <keithp@keithp.com>
(cherry picked from commit 2523a445a0)
The current code sets bracket_greater to the first trigger after the current
value, and bracket_less to the last trigger before the current value.
For example, the idle timer with three negative and three positive transitions
would set this:
nt1 nt2 nt3
|--------|------|--|------- idle --|---|--|-----> t
pt1 pt2 pt3
bracket_less == nt2
bracket_greater == pt2
This is an optimization so we can skip code paths in the block/wakeup handlers
if the current value doesn't meet any of the trigger requirements. Those
handlers largely do a
if (bracket_less is less than current value &&
bracket_greater is greater than current value)
return, nothing to do
However, unless the bracket values are updated at the correct time, the
following may happen:
nt
|--------------|---------- idle ------|--------> t
pt
In this case, neither bracket is set, we're past the pos transition and not
yet at the neg transition. idle may now go past nt, but the brackets are not
updated. If idle is then reset to 0, no alarm is triggered for nt. Likewise,
idle may now go past pt and no alarm is triggered.
Changing an alarm or triggering an alarm will re-calculate the brackets, so
this bug is somewhat random. If any other client triggers an alarm when the
brackets are wrongly NULL, the recalculation will set them this bug may not
appear.
This patch changes the behavior, so that the brackets are always the nearest
positive or negative transitions to the current counter value. In the example
above, nt will trigger a wakeup and a re-calculation of the brackets, so that
going past it in the negative direction will then cause the proper alarm
triggers.
Or, in Keith's words:
Timer currently past a positive trigger
No bracket values, because no trigger in range
Timer moves backwards before the positive trigger
Brackets not reset, even though there is now a trigger in range
Timer moves forward past the positive trigger
Trigger doesn't fire because brackets not set
Setting the LT bracket in this case will cause everything to get
re-evaluated when the sync value moves backwards before the trigger
value.
X.Org Bug 59644 <http://bugs.freedesktop.org/show_bug.cgi?id=59644>
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Reviewed-by: Adam Jackson <ajax@redhat.com>
Reviewed-by: Keith Packard <keithp@keithp.com>
(cherry picked from commit e57ec99b03)
The time between the idle reset and the IdleTimeWakeupHandler to be called is
indeterminate. Clients with an PositiveTransition or NegativeTransition alarm
on a low threshold may miss an alarm.
Work around this by keeping a reset flag for each device. When the
WakeupHandler triggers and the reset flag is set, we force a re-calculation of
everything and pretend the current idle time is zero. Immediately after is the
next calculation with the real idle time.
Relatively reproducible test case: Set up a XSyncNegativeTransition alarm for
a threshold of 1 ms. May trigger, may not.
X.Org Bug 70476 <http://bugs.freedesktop.org/show_bug.cgi?id=70476>
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Reviewed-by: Adam Jackson <ajax@redhat.com>
Reviewed-by: Keith Packard <keithp@keithp.com>
(cherry picked from commit 06b87aa528)
And now that we have the accessors, localize it. No functional changes, just
preparing for a future change.
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Reviewed-by: Adam Jackson <ajax@redhat.com>
Reviewed-by: Keith Packard <keithp@keithp.com>
(cherry picked from commit efc1035ca9)
The main idletime counter has an initialized deviceid, might as well be
supplying it properly. Without this, we'd only ever check the XIAllDevices
counter, so the wait would never be adjusted for the device-specific triggers.
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Reviewed-by: Adam Jackson <ajax@redhat.com>
Reviewed-by: Keith Packard <keithp@keithp.com>
(cherry picked from commit b7c9bd9cf2)
Both ServertimeBracketValues and IdleTimeBracketValues copy the value into
there SysCounter privates. Call it for a NULL set as well, so we don't end up
with stale pointers and we can remove the block/wakeup handlers.
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Reviewed-by: Adam Jackson <ajax@redhat.com>
Reviewed-by: Keith Packard <keithp@keithp.com>
(cherry picked from commit 2efe49c102)
No functional changes, just merges a > and == condition into a >= condition.
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Reviewed-by: Adam Jackson <ajax@redhat.com>
Reviewed-by: Keith Packard <keithp@keithp.com>
(cherry picked from commit 5c7cfd4c69)
Use the grabtype to determine which type of event to send - all other events
are pointless and may result in erroneous events being delivered.
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
(cherry picked from commit 7cf1b595c8)
For an active grab, grab->eventMask can be either the core or the XI1 mask.
With the overlap of event filters, calling DeliverOneGrabbedEvent(XI1) for a
ProximityOut event will trigger if the client has selected for enter events -
the filter is the same for both.
Thus, we end up delivering a proximity event to a client not expecting one.
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
(cherry picked from commit 78944d62ff)
If a client calls XAllowEvents(SyncPointer) it expects events as normal until
the next button press or release event - that freezes the device. An e.g.
proximity event must thus not freeze the pointer.
As per the spec, only button and key events may do so, so narrow it to these
cases.
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
(cherry picked from commit be6ea80b79)
If we have a client which has registered for a DeviceButton grab
be sure to pass this to CheckDeviceGrabAndHintWindow(). Since the
order of clients is arbitrary there is no guarantee that the last
client in the list is the one that belongs to this class.
Signed-off-by: Egbert Eich <eich@freedesktop.org>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
(cherry picked from commit 05ea6307fa)
XIAllowEvents changed length in XI 2.2 (for the touchid). A bug in libXi
causes libXi to always use the new request length if the server supports
2.2, regardless of the client's XIQueryVersion request.
The server takes the client's XIQueryVersion request into account though,
resulting in a BadLength error if a 2.[0,1] client calls XIAllowEvents on a
XI 2.2+ server.
Can't fix this in libXi, so work around this in the server.
X.Org Bug 68554 <http://bugs.freedesktop.org/show_bug.cgi?id=68554>
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Reviewed-by: Adam Jackson <ajax@redhat.com>
(cherry picked from commit 94d4e29aed)
if the grab type isn't XI2, grab->xi2mask is random. That random data may
have the enter/leave mask set, leading to events sent to the client that the
client can't handler.
Source of these errors:
_xgeWireToEvent: Unknown extension 131, this should never happen.
Simplest reproducer:
Start Xephyr, press button inside window, move out. As the pointer leaves
the Xephyr window, the errors appear.
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Reviewed-by: Adam Jackson <ajax@redhat.com>
(cherry picked from commit 82939e0239)
pop without push restores the commandline options. The proper way is to
push, then ignore, then pop.
And while we're at it, change the pop argument to a comment - pop ignores
the argument, but be proper about it.
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
(cherry picked from commit d803f296c6)
Code to recognize these in extension enable/disable options was wrapped
in #ifdef XorgLoader, but that's not defined when building miinitext.c
since the great module merge of 1.13. Change to an #ifdef that is defined.
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Reviewed-by: Robert Hooker <robert.hooker@canonical.com>
Reviewed-by: Daniel Stone <daniel@fooishbar.org>
Signed-off-by: Keith Packard <keithp@keithp.com>
Save a pointer to the passed in closure structure before copying it
and overwriting the *c pointer to point to our copy instead of the
original. If we hit an error, once we free(c), reset c to point to
the original structure before jumping to the cleanup code that
references *c.
Since one of the errors being checked for is whether the server was
able to malloc(c->nChars * itemSize), the client can potentially pass
a number of characters chosen to cause the malloc to fail and the
error path to be taken, resulting in the read from freed memory.
Since the memory is accessed almost immediately afterwards, and the
X server is mostly single threaded, the odds of the free memory having
invalid contents are low with most malloc implementations when not using
memory debugging features, but some allocators will definitely overwrite
the memory there, leading to a likely crash.
Reported-by: Pedro Ribeiro <pedrib@gmail.com>
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Reviewed-by: Julien Cristau <jcristau@debian.org>
Replace hardcoded SVR4 || linux || CSRG_BASED with an autoconf check and
the _POSIX_SAVED_IDS macro.
Suggested-by: Mark Kettenis <mark.kettenis@xs4all.nl>
Signed-off-by: Julien Cristau <jcristau@debian.org>
Reviewed-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Reviewed-by: Mark Kettenis <kettenis@openbsd.org>.
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
(cherry picked from commit c218ba8423)
Missing _XSERVER64 define caused inconsistent sizeof(XID) between the
test and hashtable code, leading to test failures on 64bit big endian
archs like s390x or ppc64.
Signed-off-by: Julien Cristau <jcristau@debian.org>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
(cherry picked from commit 930c6ff15d)
I'm getting a error building xorg-server-1.14.1.902 with thelatest snapshot
of gcc-4.8:
input.c:225:43: error: array subscript is above array bounds
[-Werror=array-bounds]
This is because kdNumInputFds can become equal to KD_MAX_INPUT_FDS in
KdRegisterFd(). This means that in KdUnregisterFd(), kdInputFds[j + 1] can
be beyond the end of the array.
Signed-off-by: Chris Clayton <chris2553@googlemail.com>
Reviewed-by: Keith Packard <keithp@keithp.com>
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
The old code was broken and allowed setting client version >= XIVersion,
this was fixed in the previous patch, but updating the value for XIVersion
broke the tests, so fix the tests too.
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@canonical.com>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
(cherry picked from commit b6e5c4669e)
Do not allow setting client version to an arbitrary value >= XIVersion.
Fixes a test error with test/xi2/protocol-xiqueryversion.c, introduced by
commit 4360514d1c "Xi: Allow clients to ask for 2.3 and then 2.2 without failing"
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@canonical.com>
Reviewed-by: Keith Packard <keithp@keithp.com>
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
(cherry picked from commit 500e844a24)
As of 4360514d1c, XIQueryVersion supports
requesting versions 2.2+ in random order, only 2.0 and 2.1 are restricted.
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
(cherry picked from commit a5abf790183798ad8aa2c29c056df3647777cfbd)
This allows different sub-systems within the same application to
request different Xi versions without either getting old behaviour
everywhere or simply failing with a BadValue.
Signed-off-by: Keith Packard <keithp@keithp.com>
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
(cherry picked from commit 4360514d1c)
XI 1.x only allows for first + num valuators, so if a device sends data for
valuators 0 and 2+ only (i.e. valuator 1 is missing) we still need to get
the data for that from somewhere.
XI 1.x uses the hack of an unset valuator mask to get the right coordinates,
i.e. we set the value but don't set the mask for it so XI2 events have the
right mask.
For an absolute device in relative mode, this broke in b28a1af55c, the
value was now always 0. This wasn't visible on the cursor, only in an XI 1.x
client. The GIMP e.g. sees jumps to x/0 every few events.
Drop the condition introduced in b28a1af55c, data in valuators is always
absolute, regardless of the mode.
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
(cherry picked from commit 3d87566310)
grab->type is only non-zero for passive grabs. We're checking an active grab
here, so we need to check if the touch mask is set on the grab.
Test case: grab the device, then start two simultaneous touches. The
grabbing client won't see the second touchpoints because grab->type is 0
and the second touch is not an emulating pointer.
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
(cherry picked from commit 4fb686d6a6)
All callers currently ignore the new value, so this patch has no effect.
Inverse call graph:
DeliverTouchEmulatedEvent
DeliverEmulatedMotionEvent Ignores value
DeliverTouchBeginEvent
DeliverTouchEvent
DeliverTouchEvents Ignores value
DeliverTouchEndEvent
DeliverTouchEvent
DeliverTouchEvents Ignores value
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Reviewed-by: Keith Packard <keithp@keithp.com>
(cherry picked from commit 9978b57b8d)
Ungrabbing a device during an active touch grab rejects the grab. Ungrabbing
a device during an active pointer grab accepts the grab.
Rejection is not really an option for a pointer-emulated grab, if a client
has a button mask on the window it would get a ButtonPress emulated after
UngrabDevice. That is against the core grab behaviour.
X.Org Bug 66720 <http://bugs.freedesktop.org/show_bug.cgi?id=66720>
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Reviewed-by: Jasper St. Pierre <jstpierre@mecheye.net>
(cherry picked from commit 8eeaa74bc2)
This shouldn't have been in the patch
Reported-by: Colin Harrison <colin.harrison@virgin.net>
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Reviewed-by: Keith Packard <keithp@keithp.com>
Signed-off-by: Keith Packard <keithp@keithp.com>
(cherry picked from commit c21344add2)
Too many callers relied on the refcnt being handled correctly. Use a simple
wrapper to handle that case.
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
(cherry picked from commit 9a5ad65330)
ProcessTouchEvents() calls UDS for all touch events, but if the event type
was switched to TouchUpdate(pending end) UDS is a noop.
Daniel Drake found this can cause stuck buttons if a touch grab is
activated, rejected and the touch event is passed to a regular listener.
This sequence causes the TouchEnd to be changed to TouchUpdate(pending end).
The actual TouchEnd event is later generated by the server once it is passed
to the next listener. UDS is never called for this event, thus the button
remains logically down.
A previous patch suggested for UDS to handle TouchUpdate events [1], however
this would release the button when the first TouchEvent is processed, not
when the last grab has been released (as is the case for sync pointer
grabs). A client may thus have the grab on the device, receive a ButtonPress
but see the button logically up in an XQueryPointer request.
This patch adds a call to UDS to TouchEmitTouchEnd(). The device state must
be updated once a TouchEnd event was sent to the last grabbing listener and
the number of grabs on the touchpoint is 0.
[1] http://patchwork.freedesktop.org/patch/13464/
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
(cherry picked from commit 35c2e263db)
The cursor is referenced during CopyGrab(), thus doesn't need to be handled
manually anymore. It does need to be refcounted for temp grabs though.
The oldGrab handling in ProcGrabPointer is a leftover from the cursor in the
grab being refcounted, but the grab itself being a static struct in the
DeviceIntRec. Now that all grabs are copied, this lead to a double-free of
the cursor (Reproduced in Thunderbird, dragging an email twice (or more
often) causes a crash).
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
(cherry picked from commit 481702101b)
A client may call XIGrabDevice twice, overwriting the existing grab. Thus,
make sure we free the old copy after we copied it. Free it last, to make
sure our refcounts don't run to 0 and inadvertantly free something on the
way.
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
(cherry picked from commit 3093f78d17)
If we have one listener left but it's not a grab, it cannot be in
LISTENER_HAS_ACCEPTED state.
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
(cherry picked from commit 2566bdd8bc)
TouchListenerGone cleans up if a client disappears. Having this in
FreeGrab() triggers cyclic removal of grabs, emitting wrong events. In
particular, it would clean up a passive grab record while that grab is
active.
Move it to CloseDownClient() instead, cleaning up before we go.
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
(cherry picked from commit 5b00fc5227)
Introduced in xorg-server-1.13.99.901-2-g9ad0fdb. Storing the grab pointer
in the listener turns out to be a bad idea. If the grab is not an active
grab or an implicit grab, the pointer stored is the one to the grab attached
on the window. This grab may be removed if the client calls UngrabButton or
similar while the touch is still active, leaving a dangling pointer.
To avoid this, copy the grab wherever we need to reference it later. This
is also what we do for pointer/keyboard grabs, where we copy the grab as
soon as it becomes active.
Reported-by: Maarten Lankhorst <maarten.lankhorst@canonical.com>
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
(cherry picked from commit 395124bd27)
