Clears 24 warnings from clang of the form:
TextExt16.c:63:34: warning: empty expression statement has no effect;
remove unnecessary ';' to silence this warning [-Wextra-semi-stmt]
CI_GET_DEFAULT_INFO_1D (fs, def);
^
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Part-of: <https://gitlab.freedesktop.org/xorg/lib/libx11/-/merge_requests/272>
Makes them match the style of the other macros here and clears
up 5 clang warnings of the form:
Context.c:194:27: warning: empty expression statement has no effect;
remove unnecessary ';' to silence this warning [-Wextra-semi-stmt]
_XCreateMutex(&db->linfo);
^
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Part-of: <https://gitlab.freedesktop.org/xorg/lib/libx11/-/merge_requests/272>
Clears 13 warnings from clang of the form:
Backgnd.c:44:72: warning: empty expression statement has no effect;
remove unnecessary ';' to silence this warning [-Wextra-semi-stmt]
OneDataCard32 (dpy, NEXTPTR(req,xChangeWindowAttributesReq), pixel);
^
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Part-of: <https://gitlab.freedesktop.org/xorg/lib/libx11/-/merge_requests/272>
Clears 38 warnings from clang of the form:
cmsLkCol.c:155:35: warning: empty expression statement has no effect;
remove unnecessary ';' to silence this warning [-Wextra-semi-stmt]
Data (dpy, colorname, (long)n);
^
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Part-of: <https://gitlab.freedesktop.org/xorg/lib/libx11/-/merge_requests/272>
Clears 12 warnings from clang of the form:
cmsCmap.c:194:34: warning: empty expression statement has no effect;
remove unnecessary ';' to silence this warning [-Wextra-semi-stmt]
DeqAsyncHandler(dpy, &async);
^
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Part-of: <https://gitlab.freedesktop.org/xorg/lib/libx11/-/merge_requests/272>
- the activation logic is reversed
- there is also _XInternalLockDisplay() that needs protection
- I've found cases (in fvwm2) where the callback calls XCheckIfEvent()
recursively. So the flag needs to be a counter.
Reviewed-by: Adam Jackson <ajax@redhat.com>
The documentation for this family of functions very clearly says not to
call into xlib in your predicate function, but historically single
threaded apps could get away with it just fine, and now that we've
forced thread-safety on the world such apps will now deadlock instead.
That's not an acceptable regression even if the app is technically
broken. This has been reported with XFCE and FVWM, and Motif's
cut-and-paste code has the same bug by inspection, so this does need to
be addressed.
This change nerfs LockDisplay/UnlockDisplay while inside the critical
bit of an IfEvent function. This is still safe in the sense that the
display remains locked and no other thread should be able to change it
from under us, but the loop that scans the event queue might not be
robust against it being modified as a side effect of protocol emitted by
the predicate callback. But that's not new, non-XInitThreads'd xlib
would have the same caveat.
Closes: xorg/lib/libx11#157
The Xserver made its own copy of this file in 2008, and the API's are
no longer the same between the server and client forks.
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
When _XGetRequest() detects that requested length exceeds remaining display
output buffer capacity it would return NULL. GetResReq() macro obtains "req"
pointer from a call to _XGetRequest() and then proceeds to assign request id
through "req" pointer which leads to NULL pointer dereference in this case.
Fix this by checking if "req" is valid before assigning request id.
Signed-off-by: Igor V. Kovalenko <igor.v.kovalenko@gmail.com>
This patch is based on research done by Dmitry Osipenko to uncover the
cause of a large class of Xlib lockups.
_XError must unlock and re-lock the display around the call to the
user error handler function. When re-locking the display, two
functions are called to ensure that the display is ready to generate a request:
_XIDHandler(dpy);
_XSeqSyncFunction(dpy);
The first ensures that there is at least one XID available to use
(possibly calling _xcb_generate_id to do so). The second makes sure a
reply is received at least every 65535 requests to keep sequence
numbers in sync (possibly generating a GetInputFocus request and
synchronously awaiting the reply).
If the second of these does generate a GetInputFocus request and wait
for the reply, then a pending error will cause recursion into _XError,
which deadlocks the display.
One seemingly easy fix is to have _XError avoid those calls by
invoking InternalLockDisplay instead of LockDisplay. That function
does everything that LockDisplay does *except* call those final two
functions which may end up receiving an error.
However, that doesn't protect the system from applications which call
some legal Xlib function from within their error handler. Any Xlib
function which cannot generate protocol or wait for events is valid,
including many which invoke LockDisplay.
What we need to do is make LockDisplay skip these two function calls
precisely when it is called from within the _XError context for the
same display.
This patch accomplishes this by creating a list of threads in the
display which are in _XError, and then having LockDisplay check the
current thread against those list elements.
Inspired-by: Dmitry Osipenko <digetx@gmail.com>
Signed-off-by: Keith Packard <keithp@keithp.com>
Tested-by: Dmitry Osipenko <digetx@gmail.com>
Reviewed-by: Dmitry Osipenko <digetx@gmail.com>
This function complements XSetIOErrorHandler(), allowing to override
the default behavior that trusts on I/O errors never coming back
(i.e. exit()ing the process).
This is meant as a mechanism for Wayland compositors (that are too
a X11 client + compositing manager) to unfasten seatbelts and jump
through the car window. It might get lucky and land on a stack of
pillows.
In consequence, some functions labeled as _X_NORETURN can as a
matter of fact return. So those hints were removed.
Signed-off-by: Carlos Garnacho <carlosg@gnome.org>
Reviewed-by: Michel Dänzer <mdaenzer@redhat.com>
Allows us to depend on _X_COLD directly instead of having to check for it.
(Since we also use _X_UNUSED, 7.0.22 or later was implicitly required
already but not checked for.)
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
This causes issues when compiling code for C++17.
While here, make function prototype match the header with regards
to removal of another register keyword.
This patch is based on a suggestion made by Uli Schlachter in a comment
to the bug report https://gitlab.freedesktop.org/xorg/lib/libx11/issues/93.
Explanation of the bug (given by Uli Schlachter as well):
An error was received and handled. Since there was an error callback set,
Xlib unlocks the display, runs the error callback, and then locks the display
again. This goes through _XLockDisplay and then calls _XSeqSyncFunction.
On this "lock the thing"-path, Xlib notices that sequence numbers are close to
wrap-around and tries to send a GetInputFocus request. However, the earlier
calls already registered themselves as "we are handling replies/errors, do
not interfere!" and so the code here waits for "that other thread" to be done
before it continues. Only that there is no other thread, but it is this thread
itself and thus a deadlock follows.
The bug is relatively easy to reproduce on any desktop environment by
using actively a touchscreen input that supports multitouch, i.e. practically
all mobile devices are affected.
Fixes: https://gitlab.freedesktop.org/xorg/lib/libx11/issues/93
Suggested-by: Uli Schlachter <psychon@znc.in>
Tested-by: Dmitry Osipenko <digetx@gmail.com>
Reported-by: Dmitry Osipenko <digetx@gmail.com>
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
The basic rule "put parantheses around macro parameters" should be
observed where possible. Otherwise code like
ConnectionNumber(foo = bar);
fails to compile. (It obviously passes if ConnectionNumber is a C
function.) There are several other macros amended for the same reason.
This bug appeared while building http://ioccc.org/1993/cmills.c, so
historically it was not present.
Signed-off-by: Dominik Muth <muth@nxdomain.no-ip.biz>
Reviewed-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
The only use of XCONN_CHECK_FREQ was removed in commit 15e5eaf628
when we dropped the old Xlib connection handling in favor of xcb's.
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Reviewed-by: Mark Kettenis <kettenis@openbsd.org>
Make use of the new 64-bit sequence number API in XCB 1.11.1 to avoid
the 32-bit sequence number wrap in libX11.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=71338
Signed-off-by: Christian Linhart <chris@demorecorder.com>
Signed-off-by: Olivier Fourdan <ofourdan@redhat.com>
Reviewed-by: Adam Jackson <ajax@redhat.com>
C89 or bust! This was documented as being needed for "only Lynx,
Linux-libc5, OS/2" and has never been enabled in modular builds,
since none of those platforms have had anyone step up to add support
since the X11R7 conversion to autotools.
Mostly performed with unifdef -UX_LOCALE, followed by removal of files
left without any purpose, and manual cleanup of remaining references.
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Reviewed-by: Adam Jackson <ajax@redhat.com>
Try to offset the cost of all the recent checks we've added by giving
the compiler a hint that the branches that involve us eating data
are less likely to be used than the ones that process it.
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Matches the units of the length field in X protocol replies, and provides
a single implementation of overflow checking to avoid having to replicate
those checks in every caller.
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Reviewed-by: Matthieu Herrb <matthieu.herrb@laas.fr>
MakeBigReq inserts a length field after the first 4 bytes of the request
(after req->length), pushing everything else back by 4 bytes.
The current memmove moves everything but the first 4 bytes back.
If a request aligns to the end of the buffer pointer when MakeBigReq is
invoked for that request, this runs over the buffer.
Instead, we need to memmove minus the first 4 bytes (which aren't moved),
minus the last 4 bytes (so we still align to the previous tail).
The 4 bytes that fell out are already handled with Data32, which will
handle the buffermax correctly.
The case where req->length = 1 was already not functional.
Reported by Abhishek Arya <inferno@chromium.org>.
https://bugzilla.mozilla.org/show_bug.cgi?id=803762
Reviewed-by: Jeff Muizelaar <jmuizelaar@mozilla.com>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
(mostly performed with unifdef, followed by some manual cleanup of
the remaining code)
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
WORD64 seems to have only been defined in <X11/Xmd.h> when building for
CRAY, to handle int being a 64-bit value (ILP64, not LP64) and having
64-bit alignment requirements.
It hadn't been fully supported even before autotooling, as can be
seen by removed code such as:
#ifdef WORD64
_XkbWriteCopyData32 Not Implemented Yet for sizeof(int)==8
#endif
(mostly performed with unifdef, followed by some manual cleanup of
the remaining code)
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
MUSTCOPY seems to have only been defined in <X11/Xmd.h> when building for
CRAY, to handle missing some sizes of integer type.
(mostly performed with unifdef, followed by some manual cleanup of
spacing/indenting in the remaining code)
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
Casts were annoying gcc by dropping constness when changing types,
when routines simply either copy data into the request buffer or
send it directly to the X server, and never modify the input.
Fixes gcc warnings including:
ChProp.c: In function 'XChangeProperty':
ChProp.c:65:6: warning: cast discards '__attribute__((const))' qualifier from pointer target type [-Wcast-qual]
ChProp.c:65:6: warning: cast discards '__attribute__((const))' qualifier from pointer target type [-Wcast-qual]
ChProp.c:74:6: warning: cast discards '__attribute__((const))' qualifier from pointer target type [-Wcast-qual]
ChProp.c:74:6: warning: cast discards '__attribute__((const))' qualifier from pointer target type [-Wcast-qual]
ChProp.c:83:6: warning: cast discards '__attribute__((const))' qualifier from pointer target type [-Wcast-qual]
SetHints.c: In function 'XSetStandardProperties':
SetHints.c:262:20: warning: cast discards '__attribute__((const))' qualifier from pointer target type [-Wcast-qual]
SetPntMap.c: In function 'XSetPointerMapping':
SetPntMap.c:46:5: warning: cast discards '__attribute__((const))' qualifier from pointer target type [-Wcast-qual]
SetPntMap.c:46:5: warning: cast discards '__attribute__((const))' qualifier from pointer target type [-Wcast-qual]
StBytes.c: In function 'XStoreBuffer':
StBytes.c:97:33: warning: cast discards '__attribute__((const))' qualifier from pointer target type [-Wcast-qual]
StName.c: In function 'XStoreName':
StName.c:40:27: warning: cast discards '__attribute__((const))' qualifier from pointer target type [-Wcast-qual]
StName.c: In function 'XSetIconName':
StName.c:51:27: warning: cast discards '__attribute__((const))' qualifier from pointer target type [-Wcast-qual]
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
The final } matches the one on the #define line, not one that doesn't
appear after the else statement it was lined up with
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
GetEmptyReq and GetResReq cannot do this due to the final typecast -
typically requests that need either of those do not have their own typedef
in the protocol headers.
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Reviewed-by: Jamey Sharp <jamey@minilop.net>
Some XI2 requests change in size over different versions and libXi would
need to hack around GetReq and GetReqExtra. Add a new GetReqSized so the
library can explicitly specify the size of the request in 4-byte units.
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Reviewed-by: Jamey Sharp <jamey@minilop.net>
