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

X11: Ensure XGetWindowProperty data gets freed

platform/x11/os_x11.cpp

@@ -29,14 +29,14 @@
 /*************************************************************************/
 
 #include "os_x11.h"
-#include "detect_prime.h"
 
 #include "core/os/dir_access.h"
 #include "core/print_string.h"
+#include "detect_prime.h"
 #include "drivers/gles2/rasterizer_gles2.h"
 #include "drivers/gles3/rasterizer_gles3.h"
-#include "errno.h"
 #include "key_mapping_x11.h"
+#include "main/main.h"
 #include "servers/visual/visual_server_raster.h"
 #include "servers/visual/visual_server_wrap_mt.h"
 
@@ -44,14 +44,15 @@
 #include <mntent.h>
 #endif
 
+#include <errno.h>
 #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
@@ -60,8 +61,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>
@@ -73,8 +72,6 @@
 #undef KEY_TAB
 #endif
 
-#include <X11/Xatom.h>
-
 #undef CursorShape
 
 #include <X11/XKBlib.h>
@@ -1504,6 +1501,7 @@ bool OS_X11::is_window_minimized() const {
 	unsigned long len;
 	unsigned long remaining;
 	unsigned char *data = NULL;
+	bool retval = false;
 
 	int result = XGetWindowProperty(
 			x11_display,
@@ -1521,10 +1519,13 @@ bool OS_X11::is_window_minimized() const {
 
 	if (result == Success) {
 		long *state = (long *)data;
-		if (state[0] == WM_IconicState)
+		if (state[0] == WM_IconicState) {
-			return true;
+			retval = true;
+		}
+		XFree(data);
 	}
-	return false;
+
+	return retval;
 }
 
 void OS_X11::set_window_maximized(bool p_enabled) {
@@ -1560,13 +1561,16 @@ void OS_X11::set_window_maximized(bool p_enabled) {
 	maximized = p_enabled;
 }
 
-bool OS_X11::is_window_maximize_allowed() {
+// Just a helper to reduce code duplication in `is_window_maximize_allowed`
-	Atom property = XInternAtom(x11_display, "_NET_WM_ALLOWED_ACTIONS", False);
+// and `is_window_maximized`.
+bool OS_X11::window_maximize_check(const char *p_atom_name) const {
+	Atom property = XInternAtom(x11_display, p_atom_name, False);
 	Atom type;
 	int format;
 	unsigned long len;
 	unsigned long remaining;
 	unsigned char *data = NULL;
+	bool retval = false;
 
 	int result = XGetWindowProperty(
 			x11_display,
@@ -1595,63 +1599,27 @@ bool OS_X11::is_window_maximize_allowed() {
 			if (atoms[i] == wm_act_max_vert)
 				found_wm_act_max_vert = true;
 
-			if (found_wm_act_max_horz || found_wm_act_max_vert)
+			if (found_wm_act_max_horz || found_wm_act_max_vert) {
-				return true;
-		}
-		XFree(atoms);
-	}
-
-	return false;
-}
-
-bool OS_X11::is_window_maximized() const {
-	// 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 = NULL;
-	bool retval = false;
-
-	int result = XGetWindowProperty(
-			x11_display,
-			x11_window,
-			property,
-			0,
-			1024,
-			False,
-			XA_ATOM,
-			&type,
-			&format,
-			&len,
-			&remaining,
-			&data);
-
-	if (result == Success) {
-		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);
 	}
 
-	XFree(data);
 	return retval;
 }
 
+bool OS_X11::is_window_maximize_allowed() const {
+	return window_maximize_check("_NET_WM_ALLOWED_ACTIONS");
+}
+
+bool OS_X11::is_window_maximized() const {
+	// Using EWMH -- Extended Window Manager Hints
+	return window_maximize_check("_NET_WM_STATE");
+}
+
 void OS_X11::set_window_always_on_top(bool p_enabled) {
 	if (is_window_always_on_top() == p_enabled)
 		return;

platform/x11/os_x11.h

@@ -44,7 +44,6 @@
 #include "servers/audio_server.h"
 #include "servers/visual/rasterizer.h"
 #include "servers/visual_server.h"
-//#include "servers/visual/visual_server_wrap_mt.h"
 
 #include <X11/Xcursor/Xcursor.h>
 #include <X11/Xlib.h>
@@ -222,7 +221,8 @@ protected:
 
 	void _window_changed(XEvent *event);
 
-	bool is_window_maximize_allowed();
+	bool window_maximize_check(const char *p_atom_name) const;
+	bool is_window_maximize_allowed() const;
 
 public:
 	virtual String get_name() const;