From 684558d6d742b01810929468d8033c6dfd584763 Mon Sep 17 00:00:00 2001 From: Armin Novak Date: Wed, 1 Mar 2023 16:01:08 +0100 Subject: [PATCH] [client,x11] wrap XChangeProperty to debug bug reports occuring only with certain setups wrap the function call with a logging edition so we have the arguments available in the log for debugging. --- client/X11/CMakeLists.txt | 2 ++ client/X11/xf_cliprdr.c | 33 +++++++++++++++------------- client/X11/xf_rail.c | 9 +++++--- client/X11/xf_utils.c | 45 ++++++++++++++++++++++++++++++++++++++ client/X11/xf_utils.h | 29 ++++++++++++++++++++++++ client/X11/xf_window.c | 46 +++++++++++++++++++++------------------ 6 files changed, 125 insertions(+), 39 deletions(-) create mode 100644 client/X11/xf_utils.c create mode 100644 client/X11/xf_utils.h diff --git a/client/X11/CMakeLists.txt b/client/X11/CMakeLists.txt index ea1cc7ee5..794bd648d 100644 --- a/client/X11/CMakeLists.txt +++ b/client/X11/CMakeLists.txt @@ -24,6 +24,8 @@ include_directories(${X11_INCLUDE_DIRS}) include_directories(${OPENSSL_INCLUDE_DIR}) set(${MODULE_PREFIX}_SRCS + xf_utils.h + xf_utils.c xf_gdi.c xf_gdi.h xf_gfx.c diff --git a/client/X11/xf_cliprdr.c b/client/X11/xf_cliprdr.c index 470bef038..519348bae 100644 --- a/client/X11/xf_cliprdr.c +++ b/client/X11/xf_cliprdr.c @@ -48,6 +48,7 @@ #include "xf_cliprdr.h" #include "xf_event.h" +#include "xf_utils.h" #define TAG CLIENT_TAG("x11.cliprdr") @@ -207,8 +208,8 @@ static void xf_cliprdr_set_raw_transfer_enabled(xfClipboard* clipboard, BOOL ena xfc = clipboard->xfc; WINPR_ASSERT(xfc); - XChangeProperty(xfc->display, xfc->drawable, clipboard->raw_transfer_atom, XA_INTEGER, 32, - PropModeReplace, (BYTE*)&data, 1); + LogTagAndXChangeProperty(TAG, xfc->display, xfc->drawable, clipboard->raw_transfer_atom, + XA_INTEGER, 32, PropModeReplace, (BYTE*)&data, 1); } static BOOL xf_cliprdr_is_raw_transfer_available(xfClipboard* clipboard) @@ -653,9 +654,9 @@ static void xf_cliprdr_provide_server_format_list(xfClipboard* clipboard) if (formats) { - XChangeProperty(xfc->display, xfc->drawable, clipboard->raw_format_list_atom, - clipboard->raw_format_list_atom, 8, PropModeReplace, Stream_Buffer(formats), - Stream_Length(formats)); + LogTagAndXChangeProperty(TAG, xfc->display, xfc->drawable, clipboard->raw_format_list_atom, + clipboard->raw_format_list_atom, 8, PropModeReplace, + Stream_Buffer(formats), Stream_Length(formats)); } else { @@ -1023,8 +1024,9 @@ static void xf_cliprdr_provide_targets(xfClipboard* clipboard, const XSelectionE if (respond->property != None) { - XChangeProperty(xfc->display, respond->requestor, respond->property, XA_ATOM, 32, - PropModeReplace, (BYTE*)clipboard->targets, clipboard->numTargets); + LogTagAndXChangeProperty(TAG, xfc->display, respond->requestor, respond->property, XA_ATOM, + 32, PropModeReplace, (BYTE*)clipboard->targets, + clipboard->numTargets); } } @@ -1039,8 +1041,9 @@ static void xf_cliprdr_provide_timestamp(xfClipboard* clipboard, const XSelectio if (respond->property != None) { - XChangeProperty(xfc->display, respond->requestor, respond->property, XA_INTEGER, 32, - PropModeReplace, (BYTE*)&clipboard->selection_ownership_timestamp, 1); + LogTagAndXChangeProperty(TAG, xfc->display, respond->requestor, respond->property, + XA_INTEGER, 32, PropModeReplace, + (BYTE*)&clipboard->selection_ownership_timestamp, 1); } } @@ -1056,8 +1059,8 @@ static void xf_cliprdr_provide_data(xfClipboard* clipboard, const XSelectionEven if (respond->property != None) { - XChangeProperty(xfc->display, respond->requestor, respond->property, respond->target, 8, - PropModeReplace, data, size); + LogTagAndXChangeProperty(TAG, xfc->display, respond->requestor, respond->property, + respond->target, 8, PropModeReplace, data, size); } } @@ -1640,8 +1643,8 @@ static void xf_cliprdr_prepare_to_set_selection_owner(xfContext* xfc, xfClipboar * anyway! */ Atom value = clipboard->timestamp_property_atom; - XChangeProperty(xfc->display, xfc->drawable, clipboard->timestamp_property_atom, XA_ATOM, 32, - PropModeReplace, (BYTE*)&value, 1); + LogTagAndXChangeProperty(TAG, xfc->display, xfc->drawable, clipboard->timestamp_property_atom, + XA_ATOM, 32, PropModeReplace, (BYTE*)&value, 1); XFlush(xfc->display); } @@ -1808,8 +1811,8 @@ xf_cliprdr_server_format_data_request(CliprdrClientContext* context, if (rawTransfer) { format = xf_cliprdr_get_client_format_by_id(clipboard, CF_RAW); - XChangeProperty(xfc->display, xfc->drawable, clipboard->property_atom, XA_INTEGER, 32, - PropModeReplace, (BYTE*)&formatId, 1); + LogTagAndXChangeProperty(TAG, xfc->display, xfc->drawable, clipboard->property_atom, + XA_INTEGER, 32, PropModeReplace, (BYTE*)&formatId, 1); } else format = xf_cliprdr_get_client_format_by_id(clipboard, formatId); diff --git a/client/X11/xf_rail.c b/client/X11/xf_rail.c index 492d2bad3..6ee899846 100644 --- a/client/X11/xf_rail.c +++ b/client/X11/xf_rail.c @@ -31,6 +31,7 @@ #include "xf_window.h" #include "xf_rail.h" +#include "xf_utils.h" #include #define TAG CLIENT_TAG("x11") @@ -683,9 +684,11 @@ error: static void xf_rail_set_window_icon(xfContext* xfc, xfAppWindow* railWindow, xfRailIcon* icon, BOOL replace) { - XChangeProperty(xfc->display, railWindow->handle, xfc->_NET_WM_ICON, XA_CARDINAL, 32, - replace ? PropModeReplace : PropModeAppend, (unsigned char*)icon->data, - icon->length); + WINPR_ASSERT(xfc); + + LogTagAndXChangeProperty(TAG, xfc->display, railWindow->handle, xfc->_NET_WM_ICON, XA_CARDINAL, + 32, replace ? PropModeReplace : PropModeAppend, + (unsigned char*)icon->data, icon->length); XFlush(xfc->display); } diff --git a/client/X11/xf_utils.c b/client/X11/xf_utils.c new file mode 100644 index 000000000..3813d31fb --- /dev/null +++ b/client/X11/xf_utils.c @@ -0,0 +1,45 @@ +/** + * FreeRDP: A Remote Desktop Protocol Implementation + * X11 helper utilities + * + * Copyright 2023 Armin Novak + * Copyringht 2023 Thincast Technologies GmbH + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include "xf_utils.h" + +int LogTagAndXChangeProperty(const char* tag, Display* display, Window w, Atom property, Atom type, + int format, int mode, const unsigned char* data, int nelements) +{ + wLog* log = WLog_Get(tag); + return LogDynAndXChangeProperty(log, display, w, property, type, format, mode, data, nelements); +} + +int LogDynAndXChangeProperty(wLog* log, Display* display, Window w, Atom property, Atom type, + int format, int mode, const unsigned char* data, int nelements) +{ + const DWORD level = WLOG_TRACE; + + if (WLog_IsLevelActive(log, level)) + { + char* propstr = XGetAtomName(display, property); + char* typestr = XGetAtomName(display, type); + WLog_Print(log, WLOG_DEBUG, "XChangeProperty(%p, %d, %s [%d], %s [%d], %d, %d, %p, %d)", + display, w, propstr, property, typestr, type, format, mode, data, nelements); + XFree(propstr); + XFree(typestr); + } + return XChangeProperty(display, w, property, type, format, mode, data, nelements); +} diff --git a/client/X11/xf_utils.h b/client/X11/xf_utils.h new file mode 100644 index 000000000..5f3601ce9 --- /dev/null +++ b/client/X11/xf_utils.h @@ -0,0 +1,29 @@ +/** + * FreeRDP: A Remote Desktop Protocol Implementation + * X11 helper utilities + * + * Copyright 2023 Armin Novak + * Copyringht 2023 Thincast Technologies GmbH + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +#pragma once + +#include + +#include + +int LogTagAndXChangeProperty(const char* tag, Display* display, Window w, Atom property, Atom type, + int format, int mode, _Xconst unsigned char* data, int nelements); +int LogDynAndXChangeProperty(wLog* log, Display* display, Window w, Atom property, Atom type, + int format, int mode, _Xconst unsigned char* data, int nelements); diff --git a/client/X11/xf_window.c b/client/X11/xf_window.c index 92b37312e..45a6bb3e1 100644 --- a/client/X11/xf_window.c +++ b/client/X11/xf_window.c @@ -55,6 +55,7 @@ #include "xf_rail.h" #include "xf_input.h" #include "xf_keyboard.h" +#include "xf_utils.h" #define TAG CLIENT_TAG("x11") @@ -110,12 +111,15 @@ typedef struct static void xf_SetWindowTitleText(xfContext* xfc, Window window, const char* name) { + WINPR_ASSERT(xfc); + WINPR_ASSERT(name); + const size_t i = strnlen(name, MAX_PATH); XStoreName(xfc->display, window, name); Atom wm_Name = xfc->_NET_WM_NAME; Atom utf8Str = xfc->UTF8_STRING; - XChangeProperty(xfc->display, window, wm_Name, utf8Str, 8, PropModeReplace, - (const unsigned char*)name, (int)i); + LogTagAndXChangeProperty(TAG, xfc->display, window, wm_Name, utf8Str, 8, PropModeReplace, + (const unsigned char*)name, (int)i); } /** @@ -425,35 +429,35 @@ BOOL xf_GetWorkArea(xfContext* xfc) void xf_SetWindowDecorations(xfContext* xfc, Window window, BOOL show) { - PropMotifWmHints hints; - hints.decorations = (show) ? MWM_DECOR_ALL : 0; - hints.functions = MWM_FUNC_ALL; - hints.flags = MWM_HINTS_DECORATIONS | MWM_HINTS_FUNCTIONS; - hints.inputMode = 0; - hints.status = 0; - XChangeProperty(xfc->display, window, xfc->_MOTIF_WM_HINTS, xfc->_MOTIF_WM_HINTS, 32, - PropModeReplace, (BYTE*)&hints, PROP_MOTIF_WM_HINTS_ELEMENTS); + PropMotifWmHints hints = { .decorations = (show) ? MWM_DECOR_ALL : 0, + .functions = MWM_FUNC_ALL, + .flags = MWM_HINTS_DECORATIONS | MWM_HINTS_FUNCTIONS, + .inputMode = 0, + .status = 0 }; + WINPR_ASSERT(xfc); + LogTagAndXChangeProperty(TAG, xfc->display, window, xfc->_MOTIF_WM_HINTS, xfc->_MOTIF_WM_HINTS, + 32, PropModeReplace, (BYTE*)&hints, PROP_MOTIF_WM_HINTS_ELEMENTS); } void xf_SetWindowUnlisted(xfContext* xfc, Window window) { - Atom window_state[2]; - window_state[0] = xfc->_NET_WM_STATE_SKIP_PAGER; - window_state[1] = xfc->_NET_WM_STATE_SKIP_TASKBAR; - XChangeProperty(xfc->display, window, xfc->_NET_WM_STATE, XA_ATOM, 32, PropModeReplace, - (BYTE*)&window_state, 2); + WINPR_ASSERT(xfc); + const Atom window_state[] = { xfc->_NET_WM_STATE_SKIP_PAGER, xfc->_NET_WM_STATE_SKIP_TASKBAR }; + LogTagAndXChangeProperty(TAG, xfc->display, window, xfc->_NET_WM_STATE, XA_ATOM, 32, + PropModeReplace, (BYTE*)&window_state, 2); } static void xf_SetWindowPID(xfContext* xfc, Window window, pid_t pid) { Atom am_wm_pid; + WINPR_ASSERT(xfc); if (!pid) pid = getpid(); am_wm_pid = xfc->_NET_WM_PID; - XChangeProperty(xfc->display, window, am_wm_pid, XA_CARDINAL, 32, PropModeReplace, (BYTE*)&pid, - 1); + LogTagAndXChangeProperty(TAG, xfc->display, window, am_wm_pid, XA_CARDINAL, 32, PropModeReplace, + (BYTE*)&pid, 1); } static const char* get_shm_id(void) @@ -561,8 +565,8 @@ xfWindow* xf_CreateDesktopWindow(xfContext* xfc, char* name, int width, int heig if (xfc->grab_keyboard) input_mask |= EnterWindowMask | LeaveWindowMask; - XChangeProperty(xfc->display, window->handle, xfc->_NET_WM_ICON, XA_CARDINAL, 32, - PropModeReplace, (BYTE*)xf_icon_prop, ARRAYSIZE(xf_icon_prop)); + LogTagAndXChangeProperty(TAG, xfc->display, window->handle, xfc->_NET_WM_ICON, XA_CARDINAL, 32, + PropModeReplace, (BYTE*)xf_icon_prop, ARRAYSIZE(xf_icon_prop)); if (parentWindow) XReparentWindow(xfc->display, window->handle, parentWindow, 0, 0); @@ -720,8 +724,8 @@ void xf_SetWindowStyle(xfContext* xfc, xfAppWindow* appWindow, UINT32 style, UIN XChangeWindowAttributes(xfc->display, appWindow->handle, CWOverrideRedirect, &attrs); } - XChangeProperty(xfc->display, appWindow->handle, xfc->_NET_WM_WINDOW_TYPE, XA_ATOM, 32, - PropModeReplace, (BYTE*)&window_type, 1); + LogTagAndXChangeProperty(TAG, xfc->display, appWindow->handle, xfc->_NET_WM_WINDOW_TYPE, + XA_ATOM, 32, PropModeReplace, (BYTE*)&window_type, 1); } void xf_SetWindowText(xfContext* xfc, xfAppWindow* appWindow, const char* name)