If a driver calls AttendClient() from within a timer callback we
need to re-compute the local 'are_ready' to prevent the attended
client from waiting until WaitForSomething() times out.
This is a fix similar to commit 9ed5b263.
Signed-off-by: Damien Leone <dleone@nvidia.com>
Reviewed-by: Keith Packard <keithp@keithp.com>
As we are not freeing elements while iterating the list of timers, we
can forgo using the safe variant, and reduce the number of pointer
dances required for the insertion sort.
Reviewed-by: Adam Jackson <ajax@redhat.com>
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Currently, we use xorg_list_add(new, head->prev) which is functionaly
equivalent to xorg_list_append(), but with more pointer chasing, so
reduce the strain on the reader and compiler by using the simpler
append().
Reviewed-by: Adam Jackson <ajax@redhat.com>
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Currently we only check timer expiry if there are no client fd (or
other input) waiting to be serviced. This makes it very easy to starve
the timers with long request queues, and so miss critical timestamps.
The timer subsystem is just another input waiting to be serviced, so
evaluate it on every loop like all the others, at the cost of calling
GetTimeInMillis() slightly more frequently. (A more invasive and likely
OS specific alternative would be to move the timer wheel to the local
equivalent of timerfd, and treat it as an input fd to the event loop
exactly equivalent to all the others, and so also serviced on every
pass. The trade-off being that the kernel timer wheel is likely more
efficiently integrated with epoll, but individual updates to each timer
would then require syscalls.)
Reviewed-by: Peter Harris <pharris@opentext.com>
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Querying a pkg-config variable using the --variable option produces the
value of the given variable as stored in the pkg-config file and should
not be used to add directories to the include search path.
The reason for this is that it breaks cross-compilation, because header
files are installed relative to the host sysroot. pkg-config supports a
PKG_CONFIG_SYSROOT_DIR environment variable that points to this sysroot
and will prepend that to the path of directories in -I or -L options in
pkg-config's Cflags, Libs or Libs.private keywords. However, because no
context can be inferred from variable names, as opposed to the keywords
with fixed meaning, the sysroot path will not be prepended to them. The
build system is responsible for doing so if necessary since it is aware
of the context in which the variable is used.
Adding the include directory returned by pkg-config to the include path
leaks build system information into the cross-build and break with very
confusing errors such as this:
In file included from include/misc.h:82:0,
from dix/atom.c:55:
/usr/include/pthread.h:682:6: warning: '__regparm__' attribute directive ignored [-Wattributes]
__cleanup_fct_attribute;
^~~~~~~~~~~~~~~~~~~~~~~
or this:
In file included from include/misc.h:139:0,
from dix/atom.c:55:
/usr/include/stdlib.h:133:8: error: '_Float128' is not supported on this target
extern _Float128 strtof128 (const char *__restrict __nptr,
^~~~~~~~~
Fix this by replacing the include directory with the appropriate xproto
dependency required to add the correct include directory to the compile
command for subdirectories that are missing the dependency. As detailed
above, this gives pkg-config the opportunity to prepend the sysroot for
all paths in -I compiler options.
Reviewed-by: Adam Jackson <ajax@redhat.com>
Signed-off-by: Thierry Reding <treding@nvidia.com>
Trivial way to reproduce the bug:
$ Xorg -logfile /tmp/mylog -config /etc/xpra/xorg.conf -displayfd 2
The server then moans:
Failed to rename log file "/tmp/mylog" to "/tmp/mylog": No such file or directory
And the log file is created but immediately renamed to "/tmp/mylog.old".
This is caused by the changes to the log file handling introduced by
this commit:
https://cgit.freedesktop.org/xorg/xserver/commit/?id=edcb6426f20c3be5dd5f50b76a686754aef2f64e
To fix this, only rename the logfile if the log filename contains the
magic substitution string "%s".
Signed-off-by: Antoine Martin <antoine@nagafix.co.uk>
Reviewed-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Signed-off-by: Nicolai Hähnle <nicolai.haehnle@amd.com>
Reviewed-by: Ben Crocker <bcrocker@redhat.com>
Reviewed-by: Antoine Martin <antoine@nagafix.co.uk>
Tested-by: Ben Crocker <bcrocker@redhat.com>
Having different types of code all trying to check for elevated privileges
is a bad idea. This implementation is the most thorough one.
Signed-off-by: Nicolai Hähnle <nicolai.haehnle@amd.com>
Reviewed-by: Ben Crocker <bcrocker@redhat.com>
Reviewed-by: Antoine Martin <antoine@nagafix.co.uk>
Tested-by: Ben Crocker <bcrocker@redhat.com>
Reviewed-by: Emil Velikov <emil.velikov@collabora.com>
Otherwise this is broken on cygwin:
rrlease.c: In function ‘ProcRRCreateLease’:
rrlease.c:305:9: error: implicit declaration of function ‘WriteFdToClient’ [-Werror=implicit-function-declaration]
if (WriteFdToClient(client, fd, TRUE) < 0) {
Signed-off-by: Adam Jackson <ajax@redhat.com>
When xorg_backtrace calls unw_get_proc_name and an error occurs, offset
might not be set for the current frame.
Initialize offset for each frame so that the offset from another frame
cannot be used inadvertently.
Signed-off-by: Jeff Smith <whydoubt@gmail.com>
Reviewed-by: Adam Jackson <ajax@redhat.com>
When a monotonic clock is not available, GetTimeInMicros() returns the
time in nanoseconds. Instead, return the time in microseconds, as the
name indicates.
Signed-off-by: Jeff Smith <whydoubt@gmail.com>
Reviewed-by: Adam Jackson <ajax@redhat.com>
The inputthread is kept locked all the time while X server's VT is not active.
If the X server is terminated while not active, it will be stuck forever in
InputThreadFini waiting for the thread to join, but it wouldn't because it is
locked.
Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=103782
Signed-off-by: Michal Srb <msrb@suse.com>
Reviewed-by: Adam Jackson <ajax@redhat.com>
Building with strict-aliasing rightly chirps here:
../os/xdmcp.c: In function ‘XdmcpRegisterConnection’:
../os/xdmcp.c:489:31: warning: dereferencing type-punned pointer will break strict-aliasing rules [-Wstrict-aliasing]
&((struct sockaddr_in6 *) &address)->sin6_addr.s6_addr[12];
^~~~~~~~~~~~
We have "const char *address", so &address here is a char ** (i.e., it
points to the slot on the stack containing the pointer to the character
array passed in as an argument). Casting that to a struct sockaddr_in6 *
is wrong, because it means that area of the stack will be reinterpreted
as a struct sockaddr_in6.
Instead, cast address, not &address.
Signed-off-by: Adam Jackson <ajax@redhat.com>
x11perf -noop with 200 xlogos connected is slightly faster with ports:
before after Operation
---------- ----------------- --------------------
18400000.0 19200000.0 (1.04) X protocol NoOperation
Acked-by: Keith Packard <keithp@keithp.com>
Signed-off-by: Peter Harris <pharris@opentext.com>
AIX's poll only allows FD_SETSIZE entries in the fd list, which is
insufficient for expanded MaxClients.
As a bonus, x11perf -noop with ~250 xlogos connected is slightly faster
with pollset:
before after Operation
--------- ---------------- --------------------
5750000.0 5990000.0 (1.04) X protocol NoOperation
Signed-off-by: Peter Harris <pharris@opentext.com>
Acked-by: Keith Packard <keithp@keithp.com>
SIGQUIT is a normal termination request, but any other signal we handle
here wants a core. This has the effect of making FatalError's call to
AbortServer trigger the
if (CoreDump)
OsAbort();
path. This will allow us to remove some DDX code that has the same net
effect.
Signed-off-by: Adam Jackson <ajax@redhat.com>
Reviewed-by: Olivier Fourdan <ofourdan@redhat.com>
Reviewed-by: Dave Airlie <airlied@redhat.com>
Don't reuse cmd for strtok output to ensure the proper pointer is
freed afterwards.
The code incorrectly assumed the pointer returned by strtok(cmd, ":")
would always point to cmd. However, strtok(str, sep) != str if str
begins with sep. This caused an invalid-free crash when running
a program under X with a name beginning with a colon.
Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=104123
Signed-off-by: Tomasz Śniatowski <kailoran@gmail.com>
Reviewed-by: Michel Dänzer <michel.daenzer@amd.com>
Terminate a dead session when -once was passed. Don't restart it.
Signed-off-by: Daniel Martin <consume.noise@gmail.com>
Reviewed-by: Walter Harms <wharms@bfs.de>
This was always wide enough to work on an fd_mask ("mask" ffs
presumably). We don't operate on fd_masks anymore, so this can go.
Signed-off-by: Adam Jackson <ajax@redhat.com>
Reviewed-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Roundhouse kick replacing the various (sizeof(foo)/sizeof(foo[0])) with
the ARRAY_SIZE macro from dix.h when possible. A semantic patch for
coccinelle has been used first. Additionally, a few macros have been
inlined as they had only one or two users.
Signed-off-by: Daniel Martin <consume.noise@gmail.com>
Reviewed-by: Adam Jackson <ajax@redhat.com>
A client can send a big request where the 32B "length" field has value
0. When the big request header is removed and the length corrected,
the value will underflow to 0xFFFFFFFF. Functions processing the
request later will think that the client sent much more data and may
touch memory beyond the receive buffer.
Signed-off-by: Eric Anholt <eric@anholt.net>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
The meson build gives me:
../os/utils.c: In function ‘LockServer’:
../os/utils.c:310:40: warning: ‘snprintf’ output may be truncated before the last format character [-Wformat-truncation=]
snprintf(pid_str, sizeof(pid_str), "%10ld\n", (long) getpid());
^~~~~~~~~
../os/utils.c:310:5: note: ‘snprintf’ output between 12 and 13 bytes into a destination of size 12
snprintf(pid_str, sizeof(pid_str), "%10ld\n", (long) getpid());
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Which seems to be due to the %d part meaning that a negative number's -
sign would be one wider than we're expecting. Fine, just coerce it to
unsigned.
Signed-off-by: Adam Jackson <ajax@redhat.com>
Reviewed-by: Alan Coopersmith <alan.coopersmith@oracle.com>
The function itself does not depend on the macro. Move it outside
of the ifdef guard and remove the identical copy in XWIN.
This is step 1 towards removing the duplication in winauth.c and moving
the OS specifics to os/
Signed-off-by: Emil Velikov <emil.l.velikov@gmail.com>
Reviewed-by: Jon Turney <jon.turney@dronecode.org.uk>
The epoll code depends on epoll_create1, not epoll_create.
Signed-off-by: Peter Harris <pharris@opentext.com>
Signed-off-by: Eric Anholt <eric@anholt.net>
Reviewed-by: Eric Anholt <eric@anholt.net>
This ensures that we don't use the now-closed file descriptor in the
future.
Reviewed-by: Adam Jackson <ajax@redhat.com>
Signed-off-by: Keith Packard <keithp@keithp.com>
oc->trans_conn is set to NULL when the connection is closed. At this
point, oc->fd is no longer valid and shouldn't be used. Move
dereference of oc->fd up into YieldControlNoInput where the state of
oc->trans_conn can be checked in a single place.
Reviewed-by: Adam Jackson <ajax@redhat.com>
Signed-off-by: Keith Packard <keithp@keithp.com>
In set_poll_client, check oc->trans_conn to make sure the connection
is still running before changing the ospoll configuration of the file
descriptor in case some other bit of the server is now using this file
descriptor.
Reviewed-by: Adam Jackson <ajax@redhat.com>
Signed-off-by: Keith Packard <keithp@keithp.com>
AbortClient performs most of the same operations as
CloseDownFileDescriptor except that it doesn't call ospoll_remove,
leaving that unaware that the file descriptor has been closed.
If the file descriptor is re-used before the server comes back around
to clean up, and that new file descriptor is passed to SetNotifyFd,
then that function will mistakenly re-interpret the stale ClientPtr
returned by ospoll_data as a struct notify * instead and mangle data
badly.
To fix this, the patch does:
1) Change CloseDownFileDescriptor so that it can be called multiple
times on the same OsCommPtr. The calls related to the file
descriptor are moved inside the check for trans_conn and
oc->trans_conn is set to NULL after cleaning up.
2) Move the XdmcpCloseDisplay call into CloseDownFileDescriptor. I
don't think the actually matters as we just need to know at some
point that the session client has exited. Moving it avoids the
possibility of having this accidentally trigger from another client
with the same fd which closes down at around the same time.
3) Change AbortClient to call CloseDownFileDescriptor. This makes sure
that all of the fd-related clean up happens in the same way
everywhere, in particular ensures that ospoll is notified about the
closed file descriptor at the time it is closed and not some time later.
Debian-bug: https://bugs.debian.org/862824
Reviewed-by: Adam Jackson <ajax@redhat.com>
Signed-off-by: Keith Packard <keithp@keithp.com>
This infrastructure is no longer read, only written; the mapping
from fd to client is now handled by ospoll.
Reviewed-by: Adam Jackson <ajax@redhat.com>
Signed-off-by: Keith Packard <keithp@keithp.com>
Whatever problem this is trying to fix, we don't care. Just include the
thing and stop worrying about whether _POSIX_SOURCE is defined.
Signed-off-by: Adam Jackson <ajax@redhat.com>
Reviewed-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Reviewed-by: Julien Cristau <jcristau@debian.org>
Tested-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Use conf_data outside of include/ to avoid re-running detection of the
same functions.
Reviewed-by: Eric Anholt <eric@anholt.net>
Signed-off-by: Peter Harris <pharris@opentext.com>
Include dix-config.h first to pick up _GNU_SOURCE so we get the
definition for sigset_t.
Reported-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Signed-off-by: Adam Jackson <ajax@redhat.com>
Test to build xserver_poll.c was inverted compared to autoconf. Build
xserver_poll.c if poll is missing.
Reviewed-by: Adam Jackson <ajax@redhat.com>
Signed-off-by: Jon Turney <jon.turney@dronecode.org.uk>
This makes sure the server will go look at the client again, notice
that the FD is no longer valid and close the client down.
Bugzilla: https://bugs.freedesktop.org/100863
Signed-off-by: Keith Packard <keithp@keithp.com>
Reviewed-and-Tested-by: Michel Dänzer <michel.daenzer@amd.com>
There are three copies of the same short sequence of operations to
close down a client when a write error occurs. Create a new function,
AbortClient, which performs these operations and then call it from the
three places.
Signed-off-by: Keith Packard <keithp@keithp.com>
Reviewed-and-Tested-by: Michel Dänzer <michel.daenzer@amd.com>
This is all just stuff we wish we had in libc, and some of this gets
used in eg. the dmx utilities build, so split it to its own library to
avoid pulling in xserver stuff.
Reviewed-by: Eric Anholt <eric@anholt.net>
Signed-off-by: Adam Jackson <ajax@redhat.com>
This is a work in progress that builds Xvfb, Xephyr, Xwayland, Xnest,
and Xdmx so far. The outline of Xquartz/Xwin support is in tree, but
hasn't been built yet. The unit tests are also not done.
The intent is to build this as a complete replacement for the
autotools system, then eventually replace autotools. meson is faster
to generate the build, faster to run the bulid, shorter to write the
build files in, and less error-prone than autotools.
v2: Fix indentation nits, move version declaration to project(), use
existing meson_options for version-config.h's vendor name/web.
Signed-off-by: Eric Anholt <eric@anholt.net>
Acked-by: Keith Packard <keithp@keithp.com>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
We mostly use #ifdef throughout the tree, and this lets the generated
config.h files just be #define TOKEN instead of #define TOKEN 1.
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
Reviewed-by: Keith Packard <keithp@keithp.com>
Reviewed-by: Adam Jackson <ajax@redhat.com>
Signed-off-by: Eric Anholt <eric@anholt.net>
Without this, assertion failures can make life hard for users and those
trying to help them.
v2:
* Change commit log wording slightly to "can make life hard", since
apparently e.g. logind can alleviate that somewhat.
* Set default handler for SIGABRT in
hw/xfree86/common/xf86Init.c:InstallSignalHandlers() and
hw/xquartz/quartz.c:QuartzInitOutput() (Eric Anholt)
Reviewed-by: Eric Anholt <eric@anholt.net>
Signed-off-by: Michel Dänzer <michel.daenzer@amd.com>
The clever pointer tricks were actually not working, and we were doing
the byte-by-byte moves in general. By just doing the memcpy and
obvious byte swap code, we end up generating actual byte swap
instructions, thanks to optimizing compilers.
text data bss dec hex filename
before: 2240807 51552 132016 2424375 24fe37 hw/xfree86/Xorg
after: 2215167 51552 132016 2398735 249a0f hw/xfree86/Xorg
Signed-off-by: Eric Anholt <eric@anholt.net>
Reviewed-by: Keith Packard <keithp@keithp.com>
Most of this is a legacy of the old "extmod" design where you could load
_some_ extensions dynamically but only if the server had been built with
support for them in the first place.
Note that since we now only initialize the DPMS extension if at least
one screen supports it, we no longer need DPMSCapableFlag: if it would
be false, we would never read its value.
Signed-off-by: Adam Jackson <ajax@redhat.com>
Reviewed-by: Eric Anholt <eric@anholt.net>
timingsafe_memcmp.c:21:1: warning: no previous prototype for ‘timingsafe_memcmp’ [-Wmissing-prototypes]
timingsafe_memcmp(const void *b1, const void *b2, size_t len)
Signed-off-by: Adam Jackson <ajax@redhat.com>
- typo in #ifdef check
- also need to add AC_CHECK_FUNCS([arc4random_buf])
Reported-by Eric Engestrom. Thanks
Reviewed-by: Adam Jackson <ajax@redhat.com>
Signed-off-by: Matthieu Herrb <matthieu@herrb.eu>
And the current code for MitToId has a use-after-free() issue.
[Also remove the actual implementations - ajax]
Signed-off-by: Matthieu Herrb <matthieu@herrb.eu>
Reviewed-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Provide the function definition for systems that don't have it.
Signed-off-by: Matthieu Herrb <matthieu@herrb.eu>
Reviewed-by: Alan Coopersmith <alan.coopersmith@oracle.com>
The input thread should generate events, not send them. Make it easier to
find the instances where it's doing so.
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Tested-by: Olivier Fourdan <ofourdan@redhat.com>
Reviewed-by: Olivier Fourdan <ofourdan@redhat.com>
