Covscan report contains various memory leak defects which were marked
as important. I have spent some time analyzing them and although they
were marked as important, most of them are in error cases, so probably
nothing serious. Let's fix most of them anyway. The rest are false
positives, or too complicated to fix, or already fixed in master, or
simply I am unsure about them.
Relates: https://github.com/FreeRDP/FreeRDP/issues/6981
The cause is very simple: we didn't map the xwindow on receiving
WINDOW_SHOW. but doing that causes another problem that you can't
hide a window anymore, and that is because whlie window hiding, the
_NET_WM_STATE and WM_STATE properies of the xwindow may change, in
the function `xf_event_PropertyNotify` we just assume that windows
not maximized, not minimized, yet not showing normally should be
corrected to be shown, we just need to consider the situation that
the window is hidden here.
fix: #5078
Icons on X11 windows are configured using the _NET_WM_ICON property
described in Extended Window Manager Hints. Here we implement converison
from DIB bitmaps used by RAIL to the format expected by _NET_WM_ICON,
and actually set the icon for RAIL app windows.
Both DIB format and _NET_WM_ICON (or rather, Xlib) are weird. Let's
start with RAIL's format. That's the one used in BMP and ICO formats
on Windows. It has some strange properties but thankfully FreeRDP's
freerdp_image_copy() can handle most of them for us. (With an exception
of monochrome and 16-color formats that it does not support. Sorry, but
I'm too lazy to fix them. They are not seem to be used by any real
application either.) The one thing that it can't do is to apply the
alpha transparency bitmask so we have to do it manually. This instantly
reminds us that DIB format has HISTORY: it's vertically flipped and
each must be padded to 4 bytes. Both these quirks having reasonable
(for a certain definition of 'reason') explanations. Such is life.
(Also, 8-bit images require a color palette which we must fill in.)
So okay, now comes _NET_WM_ICON. It is more sane (or rather, easier to
deal with). The bitmap is represented with a tiny [width, height] header
followed by an array of pixels in ARGB format. There is no padding, no
weird color formats. But here's a catch: you can't simply take the
output of freerdp_image_copy() and cast to (unsigned char*) of colors.
We have to allocate an array of C's longs and copy the pixels there,
because that's what Xlib expects (and this is mentioned in the spec).
Simply casting an array of bytes causes crashes on 64-bit systems.
So don't try to cheat or "optimize" and read the docs, kids.
Note that XFlush() call after XChangeProperty(). It's there because it
seems to helps see the icon quicker with Unity on Ubuntu 14.04. I don't
know why. (And Unity does not support _NET_WM_ICON officially. But it
sorta kinda works sometimes.)
Oh, and while we're here, delete some old, unused, and commented out
code that was setting window icons in the past. It's not needed anymore.
1. Fix fullscreen toggle for window managers that do not have multimonitor fullscreen extension support
2. Fix current monitor detection
3. Fix calculation of vscreen boundaries when single monitor is being used
4. Fix start up position of window when starting (used to always go to the top left corener, now centered)
Still a problem:
1. Window decorations do not show when going windowed
2. Smart resizing makes i3 really sad :(
3. Moving window across monitors and going fullscreen always maximizes on startup screen (when not using /multimon)
issue detected by cppcheck
[channels/drive/client/drive_main.c:454] -> [channels/drive/client/drive_main.c:443]: (warning) Either the condition '!irp' is redundant or there is possible null pointer dereference: irp.
[client/X11/xf_window.c:582] -> [client/X11/xf_window.c:580]: (warning) Either the condition '!xfc' is redundant or there is possible null pointer dereference: xfc.
[winpr/libwinpr/path/test/TestPathShell.c:40] -> [winpr/libwinpr/path/test/TestPathShell.c:43]: (warning) Either the condition '!path' is redundant or there is possible null pointer dereference: path.
[winpr/libwinpr/path/test/TestPathShell.c:49] -> [winpr/libwinpr/path/test/TestPathShell.c:52]: (warning) Either the condition '!path' is redundant or there is possible null pointer dereference: path.
Some window managers do not support _NET_WM_FULLSCREEN_MONITORS.
In that case multimonitor fullscreen does not properly work, so
add a path resizing the window over all screens instead.
Based on @erbth pull request, adding proper X11 atom checks.
If the display channel is available we use it to allow the user to resize the
xfreerdp window. When the window is resized we announce a new monitor layout and
the server reacts by doing a reactivation sequence to the new size.
The minimum window size is limited to 300x300 as 2012 servers crash horribly
if we send them a smaller layout.
xf_FixWindowCoordinates occasionally set the dimensions of the window to invalid values (0) because the minimum value check was done at the beginning of the method rather than at the end
XSelection protocol does not define any global clipboard as there is on
Windows. Instead each window has its own property for clipboard content
(like CLIPBOARD or PRIMARY) and there is a global notion of clipboard
ownership. Only one window can claim ownership of some clipboard type
at the moment.
FreeRDP uses CLIPBOARD for clipboard transfers (it's the one used by
applications when Ctrl+V is pressed). For regular desktop sessions the
session window itself is used for clipboard interactions via
xfc->drawable field. However, for remote app session there is no session
window. We cannot use the current remote app window as it may change or
be destroyed without closing the session. We also cannot use the root
window as it is already used for CF_RAW transfer protocol.
Therefore we create a simple dummy window to put into xfc->drawable for
this exact job: to act as a clipboard vessel on behalf of the entire
remote app session.
xf_create_window() usually creates the window as we immediately start in
RAIL mode when possible. xf_rail_enable_remoteapp_mode() is invoked only
when autologin failed or remote desktop had to show the session window
to the user for some reason.
- fixed invalid, missing or additional arguments
- removed all type casts from arguments
- added missing (void*) typecasts for %p arguments
- use inttypes defines where appropriate
- fixed wrong calculation of xfc->fullscreenMonitors.[right|bottom]
- only use _NET_WM_FULLSCREEN_MONITORS if at least 2 monitors are involved
- call XMoveWindow before setting the _NET_WM_STATE_FULLSCREEN property
1. Remove all uses of "localWindowOffsetCorr" variables, they added an extra layer of complexity and they are not actually needed to handle coordination of window position/size between
the local coordinate system and the remote one. This logic was causing issues in the case where the window was moved off the left side of the screen.
2. Update the xf_setWindowVisibilityRects function to offset the visibility rects as necessary when the window is hanging off the left side of the screen.
3. Stop sending mouse events when doing keyboard moves/sizes(as desired), and stop sending two mouse events for non-keyboard moves/sizes
4. Move location of new UTF8_STRING variable from previous commit
5. Refresh window and window shape for any window position/size updates, this helps keep the local and server windows in sync and works around some race conditions
remove duplicate call to XStoreName when setting window title
expand WITH_XEXT #define for rail window rects as extra unecessary work was being done when WITH_XEXT was not defined