Merge pull request #39945 from akien-mga/x11-XGetWindowProperty-memleak

X11: Ensure XGetWindowProperty data gets freed

platform/linuxbsd/display_server_x11.cpp

@@ -32,12 +32,12 @@
 
 #ifdef X11_ENABLED
 
-#include "detect_prime_x11.h"
-
-#include "core/os/dir_access.h"
 #include "core/print_string.h"
-#include "errno.h"
+#include "core/project_settings.h"
+#include "detect_prime_x11.h"
 #include "key_mapping_x11.h"
+#include "main/main.h"
+#include "scene/resources/texture.h"
 
 #if defined(OPENGL_ENABLED)
 #include "drivers/gles2/rasterizer_gles2.h"
@@ -47,20 +47,14 @@
 #include "servers/rendering/rasterizer_rd/rasterizer_rd.h"
 #endif
 
-#include "scene/resources/texture.h"
-
-#ifdef HAVE_MNTENT
-#include <mntent.h>
-#endif
-
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
 
-#include "X11/Xutil.h"
+#include <X11/Xatom.h>
+#include <X11/Xutil.h>
+#include <X11/extensions/Xinerama.h>
 
-#include "X11/Xatom.h"
-#include "X11/extensions/Xinerama.h"
 // ICCCM
 #define WM_NormalState 1L // window normal state
 #define WM_IconicState 3L // window minimized
@@ -69,8 +63,6 @@
 #define _NET_WM_STATE_ADD 1L // add/set property
 #define _NET_WM_STATE_TOGGLE 2L // toggle property
 
-#include "main/main.h"
-
 #include <dlfcn.h>
 #include <fcntl.h>
 #include <sys/stat.h>
@@ -82,14 +74,9 @@
 #undef KEY_TAB
 #endif
 
-#include <X11/Xatom.h>
-
 #undef CursorShape
-
 #include <X11/XKBlib.h>
 
-#include "core/project_settings.h"
-
 // 2.2 is the first release with multitouch
 #define XINPUT_CLIENT_VERSION_MAJOR 2
 #define XINPUT_CLIENT_VERSION_MINOR 2
@@ -417,10 +404,6 @@ void DisplayServerX11::mouse_warp_to_position(const Point2i &p_to) {
 	if (mouse_mode == MOUSE_MODE_CAPTURED) {
 		last_mouse_pos = p_to;
 	} else {
-		/*XWindowAttributes xwa;
-		XGetWindowAttributes(x11_display, x11_window, &xwa);
-		printf("%d %d\n", xwa.x, xwa.y); needed? */
-
 		XWarpPointer(x11_display, None, windows[MAIN_WINDOW_ID].x11_window,
 				0, 0, 0, 0, (int)p_to.x, (int)p_to.y);
 	}
@@ -1098,18 +1081,19 @@ Size2i DisplayServerX11::window_get_real_size(WindowID p_window) const {
 	return Size2i(w, h);
 }
 
-bool DisplayServerX11::window_is_maximize_allowed(WindowID p_window) const {
-	_THREAD_SAFE_METHOD_
-
+// Just a helper to reduce code duplication in `window_is_maximize_allowed`
+// and `_set_wm_maximized`.
+bool DisplayServerX11::_window_maximize_check(WindowID p_window, const char *p_atom_name) const {
 	ERR_FAIL_COND_V(!windows.has(p_window), false);
 	const WindowData &wd = windows[p_window];
 
-	Atom property = XInternAtom(x11_display, "_NET_WM_ALLOWED_ACTIONS", False);
+	Atom property = XInternAtom(x11_display, p_atom_name, False);
 	Atom type;
 	int format;
 	unsigned long len;
 	unsigned long remaining;
 	unsigned char *data = nullptr;
+	bool retval = false;
 
 	int result = XGetWindowProperty(
 			x11_display,
@@ -1141,13 +1125,20 @@ bool DisplayServerX11::window_is_maximize_allowed(WindowID p_window) const {
 			}
 
 			if (found_wm_act_max_horz || found_wm_act_max_vert) {
-				return true;
+				retval = true;
+				break;
 			}
 		}
-		XFree(atoms);
+
+		XFree(data);
 	}
 
-	return false;
+	return retval;
+}
+
+bool DisplayServerX11::window_is_maximize_allowed(WindowID p_window) const {
+	_THREAD_SAFE_METHOD_
+	return _window_maximize_check(p_window, "_NET_WM_ALLOWED_ACTIONS");
 }
 
 void DisplayServerX11::_set_wm_maximized(WindowID p_window, bool p_enabled) {
@@ -1385,60 +1376,14 @@ DisplayServer::WindowMode DisplayServerX11::window_get_mode(WindowID p_window) c
 	if (wd.fullscreen) { //if fullscreen, it's not in another mode
 		return WINDOW_MODE_FULLSCREEN;
 	}
-	{ //test maximized
-		// Using EWMH -- Extended Window Manager Hints
-		Atom property = XInternAtom(x11_display, "_NET_WM_STATE", False);
-		Atom type;
-		int format;
-		unsigned long len;
-		unsigned long remaining;
-		unsigned char *data = nullptr;
-		bool retval = false;
 
-		int result = XGetWindowProperty(
-				x11_display,
-				wd.x11_window,
-				property,
-				0,
-				1024,
-				False,
-				XA_ATOM,
-				&type,
-				&format,
-				&len,
-				&remaining,
-				&data);
-
-		if (result == Success && data) {
-			Atom *atoms = (Atom *)data;
-			Atom wm_max_horz = XInternAtom(x11_display, "_NET_WM_STATE_MAXIMIZED_HORZ", False);
-			Atom wm_max_vert = XInternAtom(x11_display, "_NET_WM_STATE_MAXIMIZED_VERT", False);
-			bool found_wm_max_horz = false;
-			bool found_wm_max_vert = false;
-
-			for (uint64_t i = 0; i < len; i++) {
-				if (atoms[i] == wm_max_horz) {
-					found_wm_max_horz = true;
-				}
-				if (atoms[i] == wm_max_vert) {
-					found_wm_max_vert = true;
-				}
-
-				if (found_wm_max_horz && found_wm_max_vert) {
-					retval = true;
-					break;
-				}
-			}
-
-			XFree(data);
-		}
-
-		if (retval) {
-			return WINDOW_MODE_MAXIMIZED;
-		}
+	// Test maximized.
+	// Using EWMH -- Extended Window Manager Hints
+	if (_window_maximize_check(p_window, "_NET_WM_STATE")) {
+		return WINDOW_MODE_MAXIMIZED;
 	}
 
-	{ // test minimzed
+	{ // Test minimized.
 		// Using ICCCM -- Inter-Client Communication Conventions Manual
 		Atom property = XInternAtom(x11_display, "WM_STATE", True);
 		Atom type;
@@ -1471,7 +1416,7 @@ DisplayServer::WindowMode DisplayServerX11::window_get_mode(WindowID p_window) c
 		}
 	}
 
-	// all other discarded, return windowed.
+	// All other discarded, return windowed.
 
 	return WINDOW_MODE_WINDOWED;
 }

platform/linuxbsd/display_server_x11.h

@@ -36,7 +36,6 @@
 #include "servers/display_server.h"
 
 #include "core/input/input.h"
-
 #include "drivers/alsa/audio_driver_alsa.h"
 #include "drivers/alsamidi/midi_driver_alsamidi.h"
 #include "drivers/pulseaudio/audio_driver_pulseaudio.h"
@@ -231,6 +230,7 @@ class DisplayServerX11 : public DisplayServer {
 	static Property _read_property(Display *p_display, Window p_window, Atom p_property);
 
 	void _update_real_mouse_position(const WindowData &wd);
+	bool _window_maximize_check(WindowID p_window, const char *p_atom_name) const;
 	void _set_wm_fullscreen(WindowID p_window, bool p_enabled);
 	void _set_wm_maximized(WindowID p_window, bool p_enabled);
 

platform/linuxbsd/os_linuxbsd.cpp

@@ -31,8 +31,11 @@
 #include "os_linuxbsd.h"
 
 #include "core/os/dir_access.h"
-#include "core/print_string.h"
-#include "errno.h"
+#include "main/main.h"
+
+#ifdef X11_ENABLED
+#include "display_server_x11.h"
+#endif
 
 #ifdef HAVE_MNTENT
 #include <mntent.h>
@@ -48,12 +51,6 @@
 #include <sys/types.h>
 #include <unistd.h>
 
-#include "main/main.h"
-
-#ifdef X11_ENABLED
-#include "display_server_x11.h"
-#endif
-
 void OS_LinuxBSD::initialize() {
 	crash_handler.initialize();