The ‘RRCrtcNotify() and RRCrtcSet() functions are exported, so there's chance
that a buggy driver could call them with NULL parameter, leading to segfault.
Those are hard to trace, so it's better having a BUG_* check here.
| ../randr/rrcrtc.c: In function ‘RRCrtcNotify’:
| ../randr/rrcrtc.c:187:5: warning: use of NULL ‘outputs’ where non-null expected [CWE-476] [-Wanalyzer-null-argument]
| 187 | memcpy(crtc->outputs, outputs, numOutputs * sizeof(RROutputPtr));
| | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
| ../randr/rrcrtc.c: In function ‘RRCrtcSet’:
| ../randr/rrcrtc.c:742:20: warning: dereference of NULL ‘outputs’ [CWE-476] [-Wanalyzer-null-dereference]
| 742 | if (outputs[o]) {
| | ~~~~~~~^~~
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
If there's no data to send, the whole reply payload can be skipped entirely.
This can also ease the whole code flow, and we don't need to rely on the
individual copy loops never trying to dereference a NULL pointer.
(what the analyzer can't proof). Also scoping several some variables that
are only used when there actually is data to send.
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
If there's no data to send, the whole reply payload can be skipped entirely.
This can also ease the whole code flow, and we don't need to rely on the
individual copy loops never trying to dereference a NULL pointer.
(what the analyzer can't proof). Also scoping several some variables that
are only used when there actually is data to send.
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
Moving payload buffer assembly right into the same branch where the buffer is
allocated, so making the whole code flow easier to understand. Also moving the
byteswap there (when the fields should still be in CPU cache), instead of having
some callback doing it much later, so even more simplication.
As a nice by-product, that's also reducing some analyzer noise.
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
The code can be much simpler by just using CopySwap32Write().
And we also don't need the callback in WriteSwappedDataToClient(),
just call the corresponding write function directly.
This also makes some analyzer warnings go away.
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
Make it a bit easier to understand how exactly the name string is copied into
the reply payload: just do the little memcpy() right where the target position
is decided any the rest of the payload is filled.
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
Instead of relying on memcpy() coping with NULL buffer when size == 0,
move the call to the branch where we actually have things to copy.
This also silences yet another analyzer warning.
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
Instead of relying on memcpy() coping with NULL buffer when size == 0,
move the call to the branch where we actually have things to copy.
This also silences yet another analyzer warning.
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
Instead of relying on memcpy() coping with NULL buffer when size == 0,
move the call to the branch where we actually have things to copy.
This also silences yet another analyzer warning.
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
Simplifying the code flow allocating/checking/copying some buffers in
RRConfigureOutputProperty() and RRConfigureProviderProperty() so it's
easier to understand for both the human reader as well as the analyzer.
Depending on whether we have elements to process, a temporary buffer needs
to be allocated, checked for successful allocation and copy over data. The
way it's currently done is technically correct, but unnecessarily complex to
understand: instead of just branching on whether there are elements and doing
all the buffer-related things only then, the branching is done just somewhere
in the middle, only on checking for allocation failure, and relying on both
calloc() and memcpy() not doing weird things when size is zero.
It's easy to simplify by putting it all behind one if statement and so make
things easier for both human reader as well as the analyzer (so it's not
spilling out false alarms here anymore) and also drops unnecessary calls
in the zero-size case.
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
Mixed up reply vs request. Obviously, the size substracted from reply
struct's one has to be the one of the generic reply, not generic requst :o
Background: the meaning of the length field isn't entirely intuitive.
a) the size is in 4-byte units, instead of bytes (therefore passing through
bytes_to_int32() call)
b) it's not the total packet size, but only the *extra* payload size, ergo:
how much is the packet longer than a xGenericReply = 8 units = 32 bytes.
(min. packet size is 32 bytes -> length = 0)
In order to prevent those kind of coding errors ever happening again, it might
be a good idea putting that into a generic macro.
Fixes: c6f1b8a735
Fixes: 0ca5aaba50
Issue: https://gitlab.freedesktop.org/xorg/xserver/-/issues/1797
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/1845>
The whole struct is already allocated by calloc(), so no need to explicitly
zero-out individual fields.
Fixes: 479b2be4ba - Clear allocated RandR screen private structure
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/1794>
Simplify reply payload preparation and sendout by using SwapShort()
and SwapLong() instead of WriteToClientSwapped() and callbacks.
This also allows even further simplifications by using generic macros
for the request send path.
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/1794>
Simplify reply payload preparation and sendout by using SwapShort()
and SwapLong() instead of WriteToClientSwapped() and callbacks.
This also allows even further simplifications by using generic macros
for the request send path.
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/1794>
Simplify reply payload preparation and sendout by using SwapShort()
and SwapLong() instead of WriteToClientSwapped() and callbacks.
This also allows even further simplifications by using generic macros
for the request send path.
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/1794>
Simplify reply payload preparation and sendout by using SwapShort()
and SwapLong() instead of WriteToClientSwapped() and callbacks.
This also allows even further simplifications by using generic macros
for the request send path.
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/1794>
Simplify reply payload preparation and sendout by using SwapShort()
and SwapLong() instead of WriteToClientSwapped() and callbacks.
This also allows even further simplifications by using generic macros
for the request send path.
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/1794>
Harmonize it with all the other reply struct fields, so we can later
use generic macros for final preparation and writeout.
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/1794>
We can rely on everything being cleared. And usually even faster, as the
compiler can emit optimized instructions for clearing a whole block at once.
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/1794>
We can rely on everything being cleared. And usually even faster, as the
compiler can emit optimized instructions for clearing a whole block at once.
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/1794>
Instead of arbitrary count of individual WriteToClient() calls on small
chunks, collect the whole payload in a buffer and write it out all at once.
This also makes possible to use generic macros for reply sending, as well
as further simplifications in the write-out path.
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/1794>
WriteSwappedDataToClient() calls a callback on each single field.
We can have it easier and more efficient by just using SwapLongs()
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/1794>
Using struct initializer for the reply header and only allocating the
payload on heap. This allows using generic macros for reply preparation
and send-out later.
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/1794>
Improve readability, move the declarations to where they're needed first
and get rid of extra individual assignments. In some cases this should also
allow the compiler to produce a bit more efficient code.
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/1794>
No need to go indirectly through a vector table, since everything's fixed
anyways. It's not a pretty robust programming style: any changes need great
care, in order to not mix up things.
Replacing this by direct switch/case statement, which is using the defines
from the xrandr protocol headers. Also adding a little bit more protection
against subtle programming errors and reducing cognitive load (source size)
on understanding the code by using a tiny macro for deducing define name and
function name from the request's name.
This approach actually uncovered some subtle bug that had been waiting in
the dark for over 15 years.
As collateral benefit, getting a tiny bit better performance.
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/1794>
No need to go indirectly through a vector table, since everything's fixed
anyways. It's not a pretty robust programming style: any changes need great
care, in order to not mix up things.
Replacing this by direct switch/case statement, which is using the defines
from the xrandr protocol headers. Also adding a little bit more protection
against subtle programming errors and reducing cognitive load (source size)
on understanding the code by using a tiny macro for deducing define name and
function name from the request's name.
This approach actually uncovered some subtle bug that had been waiting in the
dark for over 15 years (see commit b87314c876)
As collateral benefit, getting a tiny bit better performance.
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/1794>
No need to go indirectly through an vector table. It's much clearer and
easier to understand when calling them directly. And a tiny bit performance
improvement as collateral benefit.
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/1794>
The OS abstraction isn't really the right place for those flags,
they are're probably better off in their corresponding extensions.
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/1519>
The request struct's length fields aren't used anymore - we have the
client->req_len field instead, which also is bigreq-compatible.
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/1639>
The authorative source of the request frame size is client->req_len,
especially with big requests larger than 2^18 bytes.
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/1639>
PANORAMIX was the original working title of the extension, before it became
official standard. Just nobody cared about fixing the symbols to the official
naming.
For backwards compatibility with drivers, the old PANORAMIX symbol will
still be set.
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/1258>
ProcRRGetScreenResources() vs. RRGetScreenResourcesCurrent() have different
semantics - this also must be followed in byte-swapped case.
Fixes: fc70839431 - Add server support for RRGetScreenResourcesCurrent
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/1630>
The symbol controls whether to include dix-config.h, and it's always set,
thus we don't need it (and dozens of ifdef's) anymore.
This commit only removes them from our own source files, where we can
guarantee that dix-config.h is present - leaving the (potentially exported)
headers untouched.
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
Windows' native headers using some our RT_* define's names for other things.
Since the naming isn't very nice anyways, introducing some new ones
(X11_RESTYPE_NONE, X11_RESTYPE_FONT, X11_RESTYPE_CURSOR) and define the old
ones as an alias to them, in case some out-of-tree code still uses them.
With thins change, we don't need to be so extremely careful about include
ordering and have explicit #undef's in order to prevent name clashes on
Win32 targets.
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/1355>
Over 1.5 decades ago, pixmap handling was moved to using pixman library,
but there's still a bit fallout from that left. Cleaning it up now.
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/1287>
In commit 7e1f86d4 monitor support was added to randr. At this time it seemed to be reasonable not to have
more than one (virtual) monitor on a particular physical display. The code was never changed since.
Nowadays, extremely large displays exists (4k displays, ultra-wide displays). In some use cases it makes sense to
split these large physical displays into multiple virtual monitors. An example are ultra-wide screens that can be
split into 2 monitors. The change in this commit makes this work.
Besides that, removing a monitor in a function that is called "RRMonitorAdd" is bad practice and causes
unexpected behaviour.
Enrico Weigelt, metux IT consult
Michael Wyraz