The code this comment was referring to was removed in
8b5086250a "Eliminate bogus event resizing."
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Reviewed-by: Daniel Stone <daniel@fooishbar.org>
Signed-off-by: Keith Packard <keithp@keithp.com>
This eliminates a poorly-named, poorly-documented field from the
ScreenRec, using a previously-unused flag bit in each GC instead.
Signed-off-by: Jamey Sharp <jamey@minilop.net>
Cc: Keith Packard <keithp@keithp.com>
Reviewed-by: Keith Packard <keithp@keithp.com>
Signed-off-by: Keith Packard <keithp@keithp.com>
Rather than always returning BadValue, associate an error status like
BadWindow with a resource type like RT_WINDOW, and return the
appropriate one for the requested type.
This patch only touches the core protocol resource types. Others still
return BadValue and need to be mapped appropriately.
dixLookupResourceByType can now return BadImplementation, if the caller
asked for a resource type that has not been allocated in the server.
Signed-off-by: Jamey Sharp <jamey@minilop.net>
Reviewed-by: Adam Jackson <ajax@redhat.com>
If filter is NoEventMask (aka CantBeFiltered), grab is null, and the
first event is not in the set of "critical events", then TryClientEvents
simply calls WriteEventsToClient. In that case, it returns 0 for fake or
dead clients, and 1 otherwise. Inline for this special case.
Signed-off-by: Jamey Sharp <jamey@minilop.net>
Reviewed-by: Julien Cristau <jcristau@debian.org>
Reviewed-by: Adam Jackson <ajax@redhat.com>
TryClientEvents already did this; this commit just moves the assignment
one level down so that no event source has to worry about sequence
numbers.
...No event source, that is, except XKB, which inexplicably calls
WriteToClient directly for several events.
Signed-off-by: Jamey Sharp <jamey@minilop.net>
Reviewed-by: Julien Cristau <jcristau@debian.org>
Reviewed-by: Adam Jackson <ajax@redhat.com>
This matches the test in TryClientEvents, and is a superset of tests
done by the callers of these functions. The consequence of forgetting
these tests is a server crash, so they're always desirable. In my
opinion, it's better to not require the callers to remember to do these
checks.
For callers that don't do very much work before calling WriteToClient or
WriteEventsToClient, I've removed the redundant checks.
hw/xquartz/xpr/appledri.c has an interesting case: While its check for
"client == NULL" appears redundant with the test in WriteEventsToClient,
it dereferences client to get the sequence number.
Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=27497
Signed-off-by: Jamey Sharp <jamey@minilop.net>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Julien Cristau <jcristau@debian.org>
Reviewed-by: Adam Jackson <ajax@redhat.com>
dix/colormap.c and dix/gc.c now dereference a ClientPtr, so they need to
include dixstruct.h. Regression introduced by commit
11c69880c7.
Reported-by: Robert Hooker <sarvatt@ubuntu.com>
Signed-off-by: Julien Cristau <jcristau@debian.org>
Reviewed-by: Jamey Sharp <jamey@minilop.net>
Signed-off-by: Keith Packard <keithp@keithp.com>
Just let Dispatch() check for a noClientException, rather than making
every single dispatch procedure take care of it.
Signed-off-by: Jamey Sharp <jamey@minilop.net>
Reviewed-by: Daniel Stone <daniel@fooishbar.org>
In order to execute a wire-level ChangeGC request, we need to look up
the resources named by any XIDs in the value-list. Various places in the
server already have pointers to the resources they want to set into the
GC, though, so over time the interface has evolved to accept either XIDs
or pointers, with several different function call signatures used in
different eras.
This patch makes the existing code require pointers to resources rather
than XIDs, and adds a simple wrapper that looks up any XIDs. The old
dixChangeGC API is preserved by delegating to whichever implementation
is appropriate.
This affects error-handling: If any of the XIDs are invalid, then the GC
is unchanged, and its ChangeGC callback is not invoked. This change is
allowed by the protocol spec, which says, "The order in which components
are verified and altered is server-dependent. If an error is generated,
a subset of the components may have been altered."
Signed-off-by: Jamey Sharp <jamey@minilop.net>
Reviewed-by: Keith Packard <keithp@keithp.com>
XSELinux was the only consumer of these interfaces and it no longer
needs them.
Signed-off-by: Keith Packard <keithp@keithp.com>
Reviewed-by: Jamey Sharp <jamey@minilop.net>
Otherwise we can't check that the XIDs this GC is being initialized with
are accessible to this client.
Signed-off-by: Jamey Sharp <jamey@minilop.net>
Cc: Eamon Walsh <ewalsh@tycho.nsa.gov>
Reviewed-by: Eamon Walsh <ewalsh@tycho.nsa.gov>
This doesn't change any behavior, but it isn't clear whether NullClient
is correct in all cases. As ajax says,
> For most of these changes, I think it's correct to use NullClient,
> since they are server-initiated changes and should not fail for (eg)
> xace reasons. ... At any rate, you're certainly not changing any
> semantics by leaving them all as NullClient, so this patch can't be
> more wrong than before.
The call in CreateGC is particularly questionable.
Signed-off-by: Jamey Sharp <jamey@minilop.net>
Reviewed-by: Adam Jackson <ajax@redhat.com>
This doesn't change any behavior, but it isn't clear whether NullClient
is correct in all cases. As ajax says,
> For most of these changes, I think it's correct to use NullClient,
> since they are server-initiated changes and should not fail for (eg)
> xace reasons. ... At any rate, you're certainly not changing any
> semantics by leaving them all as NullClient, so this patch can't be
> more wrong than before.
Signed-off-by: Jamey Sharp <jamey@minilop.net>
Reviewed-by: Keith Packard <keithp@keithp.com>
In commit 42d6112ec2, Eamon changed
dixChangeGC to require DixUseAccess on any GCFont XID. I think
doPolyText needs to require the same level of access. Otherwise
dixChangeGC could fail when it does the same lookup, which doPolyText
doesn't check for.
Signed-off-by: Jamey Sharp <jamey@minilop.net>
Cc: Eamon Walsh <ewalsh@tycho.nsa.gov>
Reviewed-by: Eamon Walsh <ewalsh@tycho.nsa.gov>
Previously the callers were only setting errorValue on Success, when
it's ignored, and leaving it alone on failure, when it's sent to the
client.
Since SetFontPath takes the ClientPtr, let it set client->errorValue
instead of letting the callers continue to get it wrong.
Signed-off-by: Jamey Sharp <jamey@minilop.net>
Reviewed-by: Julien Cristau <jcristau@debian.org>
Signed-off-by: Keith Packard <keithp@keithp.com>
Using one variant of function/macro makes it easier to fix the code
later.
Signed-off-by: Mikhail Gusarov <dottedmag@dottedmag.net>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
The only remaining X-functions used in server are XNF*, the rest is converted to
plain alloc/calloc/realloc/free/strdup.
X* functions are still exported from server and x* macros are still defined in
header file, so both ABI and API are not affected by this change.
Signed-off-by: Mikhail Gusarov <dottedmag@dottedmag.net>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
FreeResource() keeps clientTable[cid].elements up to date with the
number of resources allocated to the client. The other free
resource functions (FreeResourceByType(),
FreeClientNeverRetainResources() and FreeClientResources()) don't
maintain this invariant.
Typically, the only consequence is that the element count is too high
and we end up allocating the hash table bigger than necessary. However,
FreeResource() also relies on the element count to restart the search if
the list of resources has been changed during a resource destruction
callback. Since FreeResourceByType() doesn't update the count, if we call
that from a resource destruction callback from FreeResource(), the
loop isn't restarted and we end up following an invalid next pointer.
Furthermore, LookupClientResourceComplex() and
FreeClientNeverRetainResources() don't use the element count to detect
if a callback deleted a resource and may end up following an invalid
next pointer if the resource system is called into recursively.
Signed-off-by: Kristian Høgsberg <krh@bitplanet.net>
Reviewed-by: Keith Packard <keithp@keithp.com>
A few cursor value assignments weren't getting correctly ref counted,
causing leaks of cursor objects.
Reviewed-by: Adam Jackson <ajax@redhat.com>
Signed-off-by: Keith Packard <keithp@keithp.com>
this shut up some warnings.
Signed-off-by: Tiago Vignatti <tiago.vignatti@nokia.com>
Reviewed-by: Jamey Sharp <jamey@minilop.net>
Signed-off-by: Keith Packard <keithp@keithp.com>
In the process, fixes a memory leak in CloseDevice, and an unchecked
memory allocation in InitializePredictableAccelerationProperties.
Signed-off-by: Keith Packard <keithp@keithp.com>
Reviewed-by: Jamey Sharp <jamey@minilop.net>
These have the same default, but if you specify something different with
-s on the command line, only the screensaver time is changed. As DPMS
is usually what's desired, change it to match.
Signed-off-by: Adam Jackson <ajax@redhat.com>
Reviewed-by: Keith Packard <keithp@keithp.com>
Signed-off-by: Keith Packard <keithp@keithp.com>
If dixLookupResourceByType did not return Success, it will have set the
pointer to NULL, so the second if will always be true.
Signed-off-by: Jamey Sharp <jamey@minilop.net>
Reviewed-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Bizarre. This seems to never be used before. I left the field in ScreenInfo,
with another name. So, stop looking at it.
Signed-off-by: Tiago Vignatti <tiago.vignatti@nokia.com>
Reviewed-by: Aaron Plattner <aplattner@nvidia.com>
The de-duplication of CheckPhysLimits 942eae6868 added a
condition that is invalid for a Xinerama setup. pScreen is invalid for the
Xinerama case, so comparing it to anything is a bad idea.
Signed-off-by: Tim Yamin <plasm@roo.me.uk>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
If both devices are synchronously grabbed, first with a GrabPointer, then
with a GrabKeyboard (GrabModeSync on both), sync.other of each device points
to the grab of the respective other device.
If the keyboard is then thawed through a AllowSome request, the VCK's
sync.other is reset to NULL. Subsequently, an event on the VCP would crash
the server when dereferencing sync.other on the VCP.
The check's purpose is to compare if the other device is grabbed by the same
client, which should be checked by accessing (dev->deviceGrab->grab->resource).
A check of the server-1.3 sources confirms that.
XTS test case: Xlib13 XAllowEvents 20.
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>
If we run out of opcodes, nothing is print on the log, making the
problem hard to debug. In the current Xserver, if you enable some
extensions like multibuffer (+2 events) and use nvidia binary driver (+5
events) you can run out of opcode numbers.
Signed-off-by: Paulo Ricardo Zanoni <pzanoni@mandriva.com>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
Reviewed-by: Julien Cristau <jcristau@debian.org>
Signed-off-by: Keith Packard <keithp@keithp.com>
Xext/xf86bigfont.c contains three non-static functions which are called
elsewhere in the server. This creates a new header containing these
declarations in order to fix several warnings:
xf86bigfont.c:285: warning: no previous prototype for `XF86BigfontFreeFontShm'
dixfonts.c:502: warning: implicit declaration of function `XF86BigfontFreeFontS$
dixfonts.c:502: warning: nested extern declaration of `XF86BigfontFreeFontShm'
log.c:436: warning: implicit declaration of function `XF86BigfontCleanup'
log.c:436: warning: nested extern declaration of `XF86BigfontCleanup'
Signed-off-by: Yaakov Selkowitz <yselkowitz@users.sourceforge.net>
Reviewed-by: Julien Cristau <jcristau@debian.org>
A client requesting a GrabModeSync button grab, owner-events true, with only
the ButtonRelease mask set would never receive the press event even if the
grab window had the ButtonPress mask set.
The protocol requires that if owner-events is true, then the delivery mask
is the combination of the grab mask + the window event mask.
X.Org Bug 25400 <http://bugs.freedesktop.org/show_bug.cgi?id=25400>
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Tested-by: Jim Ramsay <i.am@jimramsay.com>
Signed-off-by: Keith Packard <keithp@keithp.com>
Tidy up some cosmetic issues in log strings:
- Add missing '\n'
- Fix some strings starting with '\n'
- Remove '\f' from some log strings
These all just look daft in a log with timestamps.
Also clarify log message about screen origin coordinates
Signed-off-by: Jon TURNEY <jon.turney@dronecode.org.uk>
Reviewed-by: Colin Harrison <colin.harrison@virgin.net>
Executed from the ConfigureWindow request, right before sending
ConfigureNotify to the clients.
This commit breaks the ScreenRec ABI.
Signed-off-by: Francisco Jerez <currojerez@riseup.net>
Signed-off-by: Kristian Høgsberg <krh@bitplanet.net>
Reviewed-by: Kristian Høgsberg <krh@bitplanet.net>
