[server,shadow] clean up code, check returns

* Fix return value checks * Mark internal functions [[nodiscard]] * Refactor x11 backend to simplify locking checks
2026-04-14 00:14:11 +09:00 · 2026-02-19 11:07:21 +01:00
parent 33bcf76950
commit dbef19705b
8 changed files with 270 additions and 171 deletions
--- a/server/shadow/X11/x11_shadow.c
+++ b/server/shadow/X11/x11_shadow.c
@@ -49,7 +49,7 @@
 #define TAG SERVER_TAG("shadow.x11")

 // #define USE_SHADOW_BLEND_CURSOR
-
+WINPR_ATTR_NODISCARD
 static UINT32 x11_shadow_enum_monitors(MONITOR_DEF* monitors, UINT32 maxMonitors);

 #ifdef WITH_PAM
@@ -71,6 +71,7 @@ typedef struct
 	SHADOW_PAM_AUTH_DATA appdata;
 } SHADOW_PAM_AUTH_INFO;

+WINPR_ATTR_NODISCARD
 static int x11_shadow_pam_conv(int num_msg, const struct pam_message** msg,
                               struct pam_response** resp, void* appdata_ptr)
 {
@@ -131,6 +132,7 @@ out_fail:
 	return pam_status;
 }

+WINPR_ATTR_NODISCARD
 static BOOL x11_shadow_pam_get_service_name(SHADOW_PAM_AUTH_INFO* info)
 {
 	const char* base = "/etc/pam.d";
@@ -153,6 +155,7 @@ static BOOL x11_shadow_pam_get_service_name(SHADOW_PAM_AUTH_INFO* info)
 	return FALSE;
 }

+WINPR_ATTR_NODISCARD
 static int x11_shadow_pam_authenticate(rdpShadowSubsystem* subsystem, rdpShadowClient* client,
                                       const char* user, const char* domain, const char* password)
 {
@@ -198,6 +201,7 @@ static int x11_shadow_pam_authenticate(rdpShadowSubsystem* subsystem, rdpShadowC

 #endif

+WINPR_ATTR_NODISCARD
 static BOOL x11_shadow_input_synchronize_event(WINPR_ATTR_UNUSED rdpShadowSubsystem* subsystem,
                                               WINPR_ATTR_UNUSED rdpShadowClient* client,
                                               WINPR_ATTR_UNUSED UINT32 flags)
@@ -207,6 +211,7 @@ static BOOL x11_shadow_input_synchronize_event(WINPR_ATTR_UNUSED rdpShadowSubsys
 	return TRUE;
 }

+WINPR_ATTR_NODISCARD
 static BOOL x11_shadow_input_keyboard_event(rdpShadowSubsystem* subsystem, rdpShadowClient* client,
                                            UINT16 flags, UINT8 code)
 {
@@ -254,6 +259,7 @@ static BOOL x11_shadow_input_keyboard_event(rdpShadowSubsystem* subsystem, rdpSh
 	return TRUE;
 }

+WINPR_ATTR_NODISCARD
 static BOOL x11_shadow_input_unicode_keyboard_event(WINPR_ATTR_UNUSED rdpShadowSubsystem* subsystem,
                                                    WINPR_ATTR_UNUSED rdpShadowClient* client,
                                                    WINPR_ATTR_UNUSED UINT16 flags,
@@ -264,6 +270,7 @@ static BOOL x11_shadow_input_unicode_keyboard_event(WINPR_ATTR_UNUSED rdpShadowS
 	return TRUE;
 }

+WINPR_ATTR_NODISCARD
 static BOOL x11_shadow_input_mouse_event(rdpShadowSubsystem* subsystem, rdpShadowClient* client,
                                         UINT16 flags, UINT16 x, UINT16 y)
 {
@@ -343,6 +350,7 @@ static BOOL x11_shadow_input_mouse_event(rdpShadowSubsystem* subsystem, rdpShado
 	return TRUE;
 }

+WINPR_ATTR_NODISCARD
 static BOOL x11_shadow_input_rel_mouse_event(rdpShadowSubsystem* subsystem, rdpShadowClient* client,
                                             UINT16 flags, INT16 xDelta, INT16 yDelta)
 {
@@ -400,6 +408,7 @@ static BOOL x11_shadow_input_rel_mouse_event(rdpShadowSubsystem* subsystem, rdpS
 	return TRUE;
 }

+WINPR_ATTR_NODISCARD
 static BOOL x11_shadow_input_extended_mouse_event(rdpShadowSubsystem* subsystem,
                                                  rdpShadowClient* client, UINT16 flags, UINT16 x,
                                                  UINT16 y)
@@ -472,6 +481,7 @@ static void x11_shadow_message_free(UINT32 id, SHADOW_MSG_OUT* msg)
 	}
 }

+WINPR_ATTR_NODISCARD
 static int x11_shadow_pointer_position_update(x11ShadowSubsystem* subsystem)
 {
 	UINT32 msgId = SHADOW_MSG_OUT_POINTER_POSITION_UPDATE_ID;
@@ -515,12 +525,12 @@ static int x11_shadow_pointer_position_update(x11ShadowSubsystem* subsystem)
 	return count;
 }

+WINPR_ATTR_NODISCARD
 static int x11_shadow_pointer_alpha_update(x11ShadowSubsystem* subsystem)
 {
-	SHADOW_MSG_OUT_POINTER_ALPHA_UPDATE* msg = NULL;
 	UINT32 msgId = SHADOW_MSG_OUT_POINTER_ALPHA_UPDATE_ID;
-	msg = (SHADOW_MSG_OUT_POINTER_ALPHA_UPDATE*)calloc(1,
-	                                                   sizeof(SHADOW_MSG_OUT_POINTER_ALPHA_UPDATE));
+	SHADOW_MSG_OUT_POINTER_ALPHA_UPDATE* msg = (SHADOW_MSG_OUT_POINTER_ALPHA_UPDATE*)calloc(
+	    1, sizeof(SHADOW_MSG_OUT_POINTER_ALPHA_UPDATE));

 	if (!msg)
 		return -1;
@@ -538,12 +548,14 @@ static int x11_shadow_pointer_alpha_update(x11ShadowSubsystem* subsystem)
 	}

 	msg->common.Free = x11_shadow_message_free;
-	return shadow_client_boardcast_msg(subsystem->common.server, NULL, msgId, (SHADOW_MSG_OUT*)msg,
-	                                   NULL)
-	           ? 1
-	           : -1;
+	const int count = shadow_client_boardcast_msg(subsystem->common.server, NULL, msgId,
+	                                              (SHADOW_MSG_OUT*)msg, NULL);
+	if (count < 0)
+		return -1;
+	return 1;
 }

+WINPR_ATTR_NODISCARD
 static int x11_shadow_query_cursor(x11ShadowSubsystem* subsystem, BOOL getImage)
 {
 	int x = 0;
@@ -590,7 +602,9 @@ static int x11_shadow_query_cursor(x11ShadowSubsystem* subsystem, BOOL getImage)
 		}

 		XFree(ci);
-		x11_shadow_pointer_alpha_update(subsystem);
+		const int rc = x11_shadow_pointer_alpha_update(subsystem);
+		if (rc < 0)
+			return rc;
 #endif
 	}
 	else
@@ -627,12 +641,13 @@ static int x11_shadow_query_cursor(x11ShadowSubsystem* subsystem, BOOL getImage)
 	{
 		subsystem->common.pointerX = (UINT32)MAX(0, x);
 		subsystem->common.pointerY = (UINT32)MAX(0, y);
-		x11_shadow_pointer_position_update(subsystem);
+		return x11_shadow_pointer_position_update(subsystem);
 	}

 	return 1;
 }

+WINPR_ATTR_NODISCARD
 static int x11_shadow_handle_xevent(x11ShadowSubsystem* subsystem, XEvent* xevent)
 {
 	if (xevent->type == MotionNotify)
@@ -642,7 +657,7 @@ static int x11_shadow_handle_xevent(x11ShadowSubsystem* subsystem, XEvent* xeven
 #ifdef WITH_XFIXES
 	else if (xevent->type == subsystem->xfixes_cursor_notify_event)
 	{
-		x11_shadow_query_cursor(subsystem, TRUE);
+		return x11_shadow_query_cursor(subsystem, TRUE);
 	}

 #endif
@@ -654,6 +669,7 @@ static int x11_shadow_handle_xevent(x11ShadowSubsystem* subsystem, XEvent* xeven
 }

 #if defined(USE_SHADOW_BLEND_CURSOR)
+WINPR_ATTR_NODISCARD
 static int x11_shadow_blend_cursor(x11ShadowSubsystem* subsystem)
 {
 	if (!subsystem)
@@ -743,6 +759,7 @@ static int x11_shadow_blend_cursor(x11ShadowSubsystem* subsystem)
 }
 #endif

+WINPR_ATTR_NODISCARD
 static BOOL x11_shadow_check_resize(x11ShadowSubsystem* subsystem)
 {
 	XWindowAttributes attr;
@@ -776,6 +793,7 @@ static BOOL x11_shadow_check_resize(x11ShadowSubsystem* subsystem)
 	return FALSE;
 }

+WINPR_ATTR_NODISCARD
 static int x11_shadow_error_handler_for_capture(Display* display, XErrorEvent* event)
 {
 	char msg[256];
@@ -792,104 +810,161 @@ static int x11_shadow_error_handler_for_capture(Display* display, XErrorEvent* e
 	return 0;
 }

-static int x11_shadow_screen_grab_locked(x11ShadowSubsystem* subsystem)
+WINPR_ATTR_NODISCARD
+static int x11_shadow_screen_grab_disp_locked(x11ShadowSubsystem* subsystem, XImage** ppimage,
+                                              RECTANGLE_16* invalidRect)
 {
-	int rc = 0;
-	int status = -1;
-	int x = 0;
-	int y = 0;
-	int width = 0;
-	int height = 0;
-	XImage* image = NULL;
-	RECTANGLE_16 invalidRect = { 0 };
-	RECTANGLE_16 surfaceRect = { 0 };
-	const RECTANGLE_16* extents = NULL;
+	WINPR_ASSERT(subsystem);
+	WINPR_ASSERT(ppimage);
+	WINPR_ASSERT(invalidRect);
+
 	rdpShadowServer* server = subsystem->common.server;
+	WINPR_ASSERT(server);
+
 	rdpShadowSurface* surface = server->surface;
-	size_t count = ArrayList_Count(server->clients);
+	WINPR_ASSERT(surface);

-	if (count < 1)
-		return 1;
+	int status = -1;

-	surfaceRect.right = WINPR_ASSERTING_INT_CAST(UINT16, surface->width);
-	surfaceRect.bottom = WINPR_ASSERTING_INT_CAST(UINT16, surface->height);
-
-	/*
-	 * Ignore BadMatch error during image capture. The screen size may be
-	 * changed outside. We will resize to correct resolution at next frame
-	 */
-	XSetErrorHandler(x11_shadow_error_handler_for_capture);
 #if defined(WITH_XDAMAGE)
 	if (subsystem->use_xshm)
 	{
-		image = subsystem->fb_image;
+		XImage* image = subsystem->fb_image;
+		*ppimage = image;
+
 		XCopyArea(subsystem->display, subsystem->root_window, subsystem->fb_pixmap,
 		          subsystem->xshm_gc, 0, 0, subsystem->width, subsystem->height, 0, 0);

+		EnterCriticalSection(&surface->lock);
 		status = shadow_capture_compare_with_format(
 		    surface->data, surface->format, surface->scanline, surface->width, surface->height,
 		    (BYTE*)&(image->data[surface->width * 4ull]), subsystem->format,
-		    WINPR_ASSERTING_INT_CAST(UINT32, image->bytes_per_line), &invalidRect);
+		    WINPR_ASSERTING_INT_CAST(UINT32, image->bytes_per_line), invalidRect);
+		LeaveCriticalSection(&surface->lock);
 	}
 	else
 #endif
 	{
-		image = XGetImage(subsystem->display, subsystem->root_window, surface->x, surface->y,
-		                  surface->width, surface->height, AllPlanes, ZPixmap);
+		EnterCriticalSection(&surface->lock);
+		XImage* image = XGetImage(subsystem->display, subsystem->root_window, surface->x,
+		                          surface->y, surface->width, surface->height, AllPlanes, ZPixmap);

 		if (image)
 		{
 			status = shadow_capture_compare_with_format(
 			    surface->data, surface->format, surface->scanline, surface->width, surface->height,
 			    (BYTE*)image->data, subsystem->format,
-			    WINPR_ASSERTING_INT_CAST(UINT32, image->bytes_per_line), &invalidRect);
-		}
-
-		if (!image)
-		{
-			/*
-			 * BadMatch error happened. The size may have been changed again.
-			 * Give up this frame and we will resize again in next frame
-			 */
-			goto fail_capture;
+			    WINPR_ASSERTING_INT_CAST(UINT32, image->bytes_per_line), invalidRect);
 		}
+		*ppimage = image;
+		LeaveCriticalSection(&surface->lock);
 	}

-	/* Restore the default error handler */
-	XSetErrorHandler(NULL);
-	XSync(subsystem->display, False);
+	return status;
+}
+
+WINPR_ATTR_NODISCARD
+static BOOL x11_shadow_surface_update_invalid(rdpShadowSurface* surface,
+                                              const RECTANGLE_16* invalidRect,
+                                              const RECTANGLE_16* surfaceRect)
+{
+	WINPR_ASSERT(surface);
+	WINPR_ASSERT(invalidRect);
+	WINPR_ASSERT(surfaceRect);
+
+	EnterCriticalSection(&surface->lock);
+	const BOOL rc1 =
+	    region16_union_rect(&(surface->invalidRegion), &(surface->invalidRegion), invalidRect);
+	const BOOL rc2 =
+	    region16_intersect_rect(&(surface->invalidRegion), &(surface->invalidRegion), surfaceRect);
+	const BOOL empty = region16_is_empty(&(surface->invalidRegion));
+	LeaveCriticalSection(&surface->lock);
+	return !rc1 || !rc2 || empty;
+}
+
+WINPR_ATTR_NODISCARD
+static BOOL x11_shadow_surface_update_contents(rdpShadowSurface* surface, UINT32 format,
+                                               const XImage* image)
+{
+	WINPR_ASSERT(surface);
+	WINPR_ASSERT(image);
+
+	EnterCriticalSection(&surface->lock);
+	const RECTANGLE_16* extents = region16_extents(&(surface->invalidRegion));
+	const UINT16 x = extents->left;
+	const UINT16 y = extents->top;
+	const UINT16 width = extents->right - extents->left;
+	const UINT16 height = extents->bottom - extents->top;
+	WINPR_ASSERT(image);
+	WINPR_ASSERT(image->bytes_per_line >= 0);
+	WINPR_ASSERT(width >= 0);
+	WINPR_ASSERT(height >= 0);
+	const BOOL success = freerdp_image_copy_no_overlap(
+	    surface->data, surface->format, surface->scanline, WINPR_ASSERTING_INT_CAST(uint32_t, x),
+	    WINPR_ASSERTING_INT_CAST(uint32_t, y), WINPR_ASSERTING_INT_CAST(uint32_t, width),
+	    WINPR_ASSERTING_INT_CAST(uint32_t, height), (BYTE*)image->data, format,
+	    WINPR_ASSERTING_INT_CAST(uint32_t, image->bytes_per_line),
+	    WINPR_ASSERTING_INT_CAST(UINT32, x), WINPR_ASSERTING_INT_CAST(UINT32, y), NULL,
+	    FREERDP_FLIP_NONE);
+	LeaveCriticalSection(&surface->lock);
+	return success;
+}
+
+WINPR_ATTR_NODISCARD
+static int x11_shadow_screen_grab(x11ShadowSubsystem* subsystem)
+{
+	int rc = 0;
+
+	WINPR_ASSERT(subsystem);
+
+	RECTANGLE_16 surfaceRect = { 0 };
+	rdpShadowServer* server = subsystem->common.server;
+	WINPR_ASSERT(server);
+
+	rdpShadowSurface* surface = server->surface;
+	WINPR_ASSERT(surface);
+
+	size_t count = ArrayList_Count(server->clients);
+
+	if (count < 1)
+		return 1;
+
+	{
+		EnterCriticalSection(&surface->lock);
+		surfaceRect.right = WINPR_ASSERTING_INT_CAST(UINT16, surface->width);
+		surfaceRect.bottom = WINPR_ASSERTING_INT_CAST(UINT16, surface->height);
+		LeaveCriticalSection(&surface->lock);
+	}
+
+	XImage* image = NULL;
+	RECTANGLE_16 invalidRect = { 0 };
+	int status = -1;
+	{
+		XLockDisplay(subsystem->display);
+		/*
+		 * Ignore BadMatch error during image capture. The screen size may be
+		 * changed outside. We will resize to correct resolution at next frame
+		 */
+		XSetErrorHandler(x11_shadow_error_handler_for_capture);
+
+		status = x11_shadow_screen_grab_disp_locked(subsystem, &image, &invalidRect);
+		if (status < 0)
+			goto fail_capture;
+
+		/* Restore the default error handler */
+		XSetErrorHandler(NULL);
+		XSync(subsystem->display, False);
+		XUnlockDisplay(subsystem->display);
+	}

 	if (status)
 	{
-		BOOL empty = 0;
-		if (!region16_union_rect(&(surface->invalidRegion), &(surface->invalidRegion),
-		                         &invalidRect))
-			goto fail_capture;
-		if (!region16_intersect_rect(&(surface->invalidRegion), &(surface->invalidRegion),
-		                             &surfaceRect))
-			goto fail_capture;
-		empty = region16_is_empty(&(surface->invalidRegion));
+		const BOOL empty = x11_shadow_surface_update_invalid(surface, &invalidRect, &surfaceRect);

 		if (!empty)
 		{
-			BOOL success = 0;
-			extents = region16_extents(&(surface->invalidRegion));
-			x = extents->left;
-			y = extents->top;
-			width = extents->right - extents->left;
-			height = extents->bottom - extents->top;
-			WINPR_ASSERT(image);
-			WINPR_ASSERT(image->bytes_per_line >= 0);
-			WINPR_ASSERT(width >= 0);
-			WINPR_ASSERT(height >= 0);
-			success = freerdp_image_copy_no_overlap(
-			    surface->data, surface->format, surface->scanline,
-			    WINPR_ASSERTING_INT_CAST(uint32_t, x), WINPR_ASSERTING_INT_CAST(uint32_t, y),
-			    WINPR_ASSERTING_INT_CAST(uint32_t, width),
-			    WINPR_ASSERTING_INT_CAST(uint32_t, height), (BYTE*)image->data, subsystem->format,
-			    WINPR_ASSERTING_INT_CAST(uint32_t, image->bytes_per_line),
-			    WINPR_ASSERTING_INT_CAST(UINT32, x), WINPR_ASSERTING_INT_CAST(UINT32, y), NULL,
-			    FREERDP_FLIP_NONE);
+			const BOOL success =
+			    x11_shadow_surface_update_contents(surface, subsystem->format, image);
 			if (!success)
 				goto fail_capture;

@@ -910,7 +985,11 @@ static int x11_shadow_screen_grab_locked(x11ShadowSubsystem* subsystem)
 					    shadow_encoder_preferred_fps(client->encoder);
 			}

-			region16_clear(&(surface->invalidRegion));
+			{
+				EnterCriticalSection(&surface->lock);
+				region16_clear(&(surface->invalidRegion));
+				LeaveCriticalSection(&surface->lock);
+			}
 		}
 	}

@@ -919,32 +998,10 @@ fail_capture:
 	if (!subsystem->use_xshm && image)
 		XDestroyImage(image);

-	if (rc != 1)
-	{
-		XSetErrorHandler(NULL);
-		XSync(subsystem->display, False);
-	}
-
-	return rc;
-}
-
-static int x11_shadow_screen_grab(x11ShadowSubsystem* subsystem)
-{
-	WINPR_ASSERT(subsystem);
-	rdpShadowServer* server = subsystem->common.server;
-	WINPR_ASSERT(server);
-
-	rdpShadowSurface* surface = server->surface;
-	WINPR_ASSERT(surface);
-
-	EnterCriticalSection(&surface->lock);
-	XLockDisplay(subsystem->display);
-	const int rc = x11_shadow_screen_grab_locked(subsystem);
-	XUnlockDisplay(subsystem->display);
-	LeaveCriticalSection(&surface->lock);
 	return rc;
 }

+WINPR_ATTR_NODISCARD
 static int x11_shadow_subsystem_process_message(x11ShadowSubsystem* subsystem, wMessage* message)
 {
 	switch (message->id)
@@ -964,6 +1021,7 @@ static int x11_shadow_subsystem_process_message(x11ShadowSubsystem* subsystem, w
 	return 1;
 }

+WINPR_ATTR_NODISCARD
 static DWORD WINAPI x11_shadow_subsystem_thread(LPVOID arg)
 {
 	x11ShadowSubsystem* subsystem = (x11ShadowSubsystem*)arg;
@@ -997,28 +1055,39 @@ static DWORD WINAPI x11_shadow_subsystem_thread(LPVOID arg)
 				if (message.id == WMQ_QUIT)
 					break;

-				x11_shadow_subsystem_process_message(subsystem, &message);
+				const int rc = x11_shadow_subsystem_process_message(subsystem, &message);
+				if (rc < 0)
+					break;
 			}
 		}

 		if (WaitForSingleObject(subsystem->common.event, 0) == WAIT_OBJECT_0)
 		{
+			int rc = 0;
 			XLockDisplay(subsystem->display);

 			if (XEventsQueued(subsystem->display, QueuedAlready))
 			{
 				XNextEvent(subsystem->display, &xevent);
-				x11_shadow_handle_xevent(subsystem, &xevent);
+				rc = x11_shadow_handle_xevent(subsystem, &xevent);
 			}

 			XUnlockDisplay(subsystem->display);
+			if (rc < 0)
+				break;
 		}

 		if ((status == WAIT_TIMEOUT) || (GetTickCount64() > frameTime))
 		{
-			x11_shadow_check_resize(subsystem);
-			x11_shadow_screen_grab(subsystem);
-			x11_shadow_query_cursor(subsystem, FALSE);
+			const int rc1 = x11_shadow_check_resize(subsystem);
+			if (rc1 < 0)
+				break;
+			const int rc2 = x11_shadow_screen_grab(subsystem);
+			if (rc2 < 0)
+				break;
+			const int rc3 = x11_shadow_query_cursor(subsystem, FALSE);
+			if (rc3 < 0)
+				break;
 			dwInterval = 1000 / subsystem->common.captureFrameRate;
 			frameTime += dwInterval;
 		}
@@ -1028,6 +1097,7 @@ static DWORD WINAPI x11_shadow_subsystem_thread(LPVOID arg)
 	return 0;
 }

+WINPR_ATTR_NODISCARD
 static int x11_shadow_subsystem_base_init(x11ShadowSubsystem* subsystem)
 {
 	if (subsystem->display)
@@ -1061,6 +1131,7 @@ static int x11_shadow_subsystem_base_init(x11ShadowSubsystem* subsystem)
 	return 1;
 }

+WINPR_ATTR_NODISCARD
 static int x11_shadow_xfixes_init(x11ShadowSubsystem* subsystem)
 {
 #ifdef WITH_XFIXES
@@ -1084,6 +1155,7 @@ static int x11_shadow_xfixes_init(x11ShadowSubsystem* subsystem)
 #endif
 }

+WINPR_ATTR_NODISCARD
 static int x11_shadow_xinerama_init(x11ShadowSubsystem* subsystem)
 {
 #ifdef WITH_XINERAMA
@@ -1113,6 +1185,7 @@ static int x11_shadow_xinerama_init(x11ShadowSubsystem* subsystem)
 #endif
 }

+WINPR_ATTR_NODISCARD
 static int x11_shadow_xdamage_init(x11ShadowSubsystem* subsystem)
 {
 #ifdef WITH_XDAMAGE
@@ -1153,6 +1226,7 @@ static int x11_shadow_xdamage_init(x11ShadowSubsystem* subsystem)
 #endif
 }

+WINPR_ATTR_NODISCARD
 static int x11_shadow_xshm_init(x11ShadowSubsystem* subsystem)
 {
 	Bool pixmaps = 0;
@@ -1315,17 +1389,14 @@ UINT32 x11_shadow_enum_monitors(MONITOR_DEF* monitors, UINT32 maxMonitors)
 	return WINPR_ASSERTING_INT_CAST(uint32_t, numMonitors);
 }

+WINPR_ATTR_NODISCARD
 static int x11_shadow_subsystem_init(rdpShadowSubsystem* sub)
 {
 	int pf_count = 0;
 	int vi_count = 0;
 	int nextensions = 0;
-	char** extensions = NULL;
-	XVisualInfo* vi = NULL;
-	XVisualInfo* vis = NULL;
 	XVisualInfo xtemplate = { 0 };
 	XPixmapFormatValues* pf = NULL;
-	XPixmapFormatValues* pfs = NULL;

 	x11ShadowSubsystem* subsystem = (x11ShadowSubsystem*)sub;

@@ -1346,66 +1417,75 @@ static int x11_shadow_subsystem_init(rdpShadowSubsystem* sub)
 		return -1;
 	}

-	extensions = XListExtensions(subsystem->display, &nextensions);
-
-	if (!extensions || (nextensions < 0))
-		return -1;
-
-	for (int i = 0; i < nextensions; i++)
 	{
-		if (strcmp(extensions[i], "Composite") == 0)
-			subsystem->composite = TRUE;
-	}
+		char** extensions = XListExtensions(subsystem->display, &nextensions);

-	XFreeExtensionList(extensions);
+		if (!extensions || (nextensions < 0))
+			return -1;
+
+		for (int i = 0; i < nextensions; i++)
+		{
+			if (strcmp(extensions[i], "Composite") == 0)
+				subsystem->composite = TRUE;
+		}
+
+		XFreeExtensionList(extensions);
+	}

 	if (subsystem->composite)
 		subsystem->use_xdamage = FALSE;

-	pfs = XListPixmapFormats(subsystem->display, &pf_count);
-
-	if (!pfs)
 	{
-		WLog_ERR(TAG, "XListPixmapFormats failed");
-		return -1;
-	}
+		XPixmapFormatValues* pfs = XListPixmapFormats(subsystem->display, &pf_count);

-	for (int i = 0; i < pf_count; i++)
-	{
-		pf = pfs + i;
-
-		if (pf->depth == (INT64)subsystem->depth)
+		if (!pfs)
 		{
-			subsystem->bpp = WINPR_ASSERTING_INT_CAST(uint32_t, pf->bits_per_pixel);
-			subsystem->scanline_pad = WINPR_ASSERTING_INT_CAST(uint32_t, pf->scanline_pad);
-			break;
+			WLog_ERR(TAG, "XListPixmapFormats failed");
+			return -1;
 		}
+
+		for (int i = 0; i < pf_count; i++)
+		{
+			pf = pfs + i;
+
+			if (pf->depth == (INT64)subsystem->depth)
+			{
+				subsystem->bpp = WINPR_ASSERTING_INT_CAST(uint32_t, pf->bits_per_pixel);
+				subsystem->scanline_pad = WINPR_ASSERTING_INT_CAST(uint32_t, pf->scanline_pad);
+				break;
+			}
+		}
+
+		XFree(pfs);
 	}

-	XFree(pfs);
 	xtemplate.class = TrueColor;
 	xtemplate.screen = subsystem->number;
-	vis = XGetVisualInfo(subsystem->display, VisualClassMask | VisualScreenMask, &xtemplate,
-	                     &vi_count);

-	if (!vis)
 	{
-		WLog_ERR(TAG, "XGetVisualInfo failed");
-		return -1;
-	}
+		XVisualInfo* vis = XGetVisualInfo(subsystem->display, VisualClassMask | VisualScreenMask,
+		                                  &xtemplate, &vi_count);

-	for (int i = 0; i < vi_count; i++)
-	{
-		vi = vis + i;
-
-		if (vi->depth == (INT64)subsystem->depth)
+		if (!vis)
 		{
-			subsystem->visual = vi->visual;
-			break;
+			WLog_ERR(TAG, "XGetVisualInfo failed");
+			return -1;
 		}
+
+		for (int i = 0; i < vi_count; i++)
+		{
+			XVisualInfo* vi = vis + i;
+
+			if (vi->depth == (INT64)subsystem->depth)
+			{
+				subsystem->visual = vi->visual;
+				break;
+			}
+		}
+
+		XFree(vis);
 	}

-	XFree(vis);
 	XSelectInput(subsystem->display, subsystem->root_window, SubstructureNotifyMask);
 	subsystem->cursorMaxWidth = 256;
 	subsystem->cursorMaxHeight = 256;
@@ -1415,7 +1495,9 @@ static int x11_shadow_subsystem_init(rdpShadowSubsystem* sub)
 	if (!subsystem->cursorPixels)
 		return -1;

-	x11_shadow_query_cursor(subsystem, TRUE);
+	const int rc1 = x11_shadow_query_cursor(subsystem, TRUE);
+	if (rc1 < 0)
+		return rc1;

 	if (subsystem->use_xfixes)
 	{
@@ -1492,6 +1574,7 @@ static int x11_shadow_subsystem_uninit(rdpShadowSubsystem* sub)
 	return 1;
 }

+WINPR_ATTR_NODISCARD
 static int x11_shadow_subsystem_start(rdpShadowSubsystem* sub)
 {
 	x11ShadowSubsystem* subsystem = (x11ShadowSubsystem*)sub;
@@ -1509,6 +1592,7 @@ static int x11_shadow_subsystem_start(rdpShadowSubsystem* sub)
 	return 1;
 }

+WINPR_ATTR_NODISCARD
 static int x11_shadow_subsystem_stop(rdpShadowSubsystem* sub)
 {
 	x11ShadowSubsystem* subsystem = (x11ShadowSubsystem*)sub;
@@ -1528,10 +1612,10 @@ static int x11_shadow_subsystem_stop(rdpShadowSubsystem* sub)
 	return 1;
 }

+WINPR_ATTR_NODISCARD
 static rdpShadowSubsystem* x11_shadow_subsystem_new(void)
 {
-	x11ShadowSubsystem* subsystem = NULL;
-	subsystem = (x11ShadowSubsystem*)calloc(1, sizeof(x11ShadowSubsystem));
+	x11ShadowSubsystem* subsystem = (x11ShadowSubsystem*)calloc(1, sizeof(x11ShadowSubsystem));

 	if (!subsystem)
 		return NULL;
@@ -1550,7 +1634,7 @@ static rdpShadowSubsystem* x11_shadow_subsystem_new(void)
 	subsystem->use_xfixes = TRUE;
 	subsystem->use_xdamage = FALSE;
 	subsystem->use_xinerama = TRUE;
-	return (rdpShadowSubsystem*)subsystem;
+	return &subsystem->common;
 }

 static void x11_shadow_subsystem_free(rdpShadowSubsystem* subsystem)
--- a/server/shadow/shadow_client.c
+++ b/server/shadow/shadow_client.c
@@ -2168,9 +2168,8 @@ static int shadow_client_subsystem_process_message(rdpShadowClient* client, wMes
 				if ((msg->xPos != client->pointerX) || (msg->yPos != client->pointerY))
 				{
 					WINPR_ASSERT(update->pointer);
-					if (CHANNEL_RC_OK != IFCALLRESULT(CHANNEL_RC_OK,
-					                                  update->pointer->PointerPosition, context,
-					                                  &pointerPosition))
+					if (!IFCALLRESULT(TRUE, update->pointer->PointerPosition, context,
+					                  &pointerPosition))
 						return -1;
 					client->pointerX = msg->xPos;
 					client->pointerY = msg->yPos;
@@ -2235,9 +2234,11 @@ static int shadow_client_subsystem_process_message(rdpShadowClient* client, wMes
 			if (client->activated && client->rdpsnd && client->rdpsnd->Activated)
 			{
 				client->rdpsnd->src_format = msg->audio_format;
-				if (CHANNEL_RC_OK != IFCALLRESULT(CHANNEL_RC_OK, client->rdpsnd->SendSamples,
-				                                  client->rdpsnd, msg->buf, msg->nFrames,
-				                                  msg->wTimestamp))
+
+				const UINT error =
+				    IFCALLRESULT(CHANNEL_RC_OK, client->rdpsnd->SendSamples, client->rdpsnd,
+				                 msg->buf, msg->nFrames, msg->wTimestamp);
+				if (CHANNEL_RC_OK != error)
 					return -1;
 			}

@@ -2251,8 +2252,9 @@ static int shadow_client_subsystem_process_message(rdpShadowClient* client, wMes

 			if (client->activated && client->rdpsnd && client->rdpsnd->Activated)
 			{
-				if (CHANNEL_RC_OK != IFCALLRESULT(CHANNEL_RC_OK, client->rdpsnd->SetVolume,
-				                                  client->rdpsnd, msg->left, msg->right))
+				const UINT error = IFCALLRESULT(CHANNEL_RC_OK, client->rdpsnd->SetVolume,
+				                                client->rdpsnd, msg->left, msg->right);
+				if (CHANNEL_RC_OK != error)
 					return -1;
 			}

@@ -2729,7 +2731,6 @@ int shadow_client_boardcast_msg(rdpShadowServer* server, void* context, UINT32 t
                                SHADOW_MSG_OUT* msg, void* lParam)
 {
 	wMessage message = { 0 };
-	rdpShadowClient* client = NULL;
 	int count = 0;

 	WINPR_ASSERT(server);
@@ -2749,7 +2750,7 @@ int shadow_client_boardcast_msg(rdpShadowServer* server, void* context, UINT32 t

 	for (size_t index = 0; index < ArrayList_Count(server->clients); index++)
 	{
-		client = (rdpShadowClient*)ArrayList_GetItem(server->clients, index);
+		rdpShadowClient* client = (rdpShadowClient*)ArrayList_GetItem(server->clients, index);

 		if (shadow_client_dispatch_msg(client, &message))
 		{
--- a/server/shadow/shadow_encomsp.c
+++ b/server/shadow/shadow_encomsp.c
@@ -102,9 +102,11 @@ encomsp_change_participant_control_level(EncomspServerContext* context,

 int shadow_client_encomsp_init(rdpShadowClient* client)
 {
-	EncomspServerContext* encomsp = NULL;
+	WINPR_ASSERT(client);

-	encomsp = client->encomsp = encomsp_server_context_new(client->vcm);
+	EncomspServerContext* encomsp = client->encomsp = encomsp_server_context_new(client->vcm);
+	if (!encomsp)
+		return -1;

 	encomsp->rdpcontext = &client->context;

@@ -124,6 +126,7 @@ int shadow_client_encomsp_init(rdpShadowClient* client)

 void shadow_client_encomsp_uninit(rdpShadowClient* client)
 {
+	WINPR_ASSERT(client);
 	if (client->encomsp)
 	{
 		client->encomsp->Stop(client->encomsp);
--- a/server/shadow/shadow_lobby.c
+++ b/server/shadow/shadow_lobby.c
@@ -33,7 +33,9 @@ BOOL shadow_client_init_lobby(rdpShadowServer* server)
 	int width = 0;
 	int height = 0;
 	rdtkSurface* surface = NULL;
-	RECTANGLE_16 invalidRect;
+	RECTANGLE_16 invalidRect = { 0 };
+
+	WINPR_ASSERT(server);
 	rdpShadowSurface* lobby = server->lobby;

 	if (!lobby)
--- a/server/shadow/shadow_rdpsnd.c
+++ b/server/shadow/shadow_rdpsnd.c
@@ -34,6 +34,7 @@

 static void rdpsnd_activated(RdpsndServerContext* context)
 {
+	WINPR_ASSERT(context);
 	for (size_t i = 0; i < context->num_client_formats; i++)
 	{
 		for (size_t j = 0; j < context->num_server_formats; j++)
@@ -53,8 +54,8 @@ static void rdpsnd_activated(RdpsndServerContext* context)

 int shadow_client_rdpsnd_init(rdpShadowClient* client)
 {
-	RdpsndServerContext* rdpsnd = NULL;
-	rdpsnd = client->rdpsnd = rdpsnd_server_context_new(client->vcm);
+	WINPR_ASSERT(client);
+	RdpsndServerContext* rdpsnd = client->rdpsnd = rdpsnd_server_context_new(client->vcm);

 	if (!rdpsnd)
 	{
@@ -77,13 +78,16 @@ int shadow_client_rdpsnd_init(rdpShadowClient* client)
 		rdpsnd->src_format = &rdpsnd->server_formats[0];

 	rdpsnd->Activated = rdpsnd_activated;
-	if (rdpsnd->Initialize(rdpsnd, TRUE) != CHANNEL_RC_OK)
+
+	const UINT error = rdpsnd->Initialize(rdpsnd, TRUE);
+	if (error != CHANNEL_RC_OK)
 		return -1;
 	return 1;
 }

 void shadow_client_rdpsnd_uninit(rdpShadowClient* client)
 {
+	WINPR_ASSERT(client);
 	if (client->rdpsnd)
 	{
 		client->rdpsnd->Stop(client->rdpsnd);
--- a/server/shadow/shadow_remdesk.c
+++ b/server/shadow/shadow_remdesk.c
@@ -26,6 +26,7 @@

 int shadow_client_remdesk_init(rdpShadowClient* client)
 {
+	WINPR_ASSERT(client);
 	RemdeskServerContext* remdesk = client->remdesk = remdesk_server_context_new(client->vcm);
 	if (!remdesk)
 		return -1;
@@ -46,6 +47,7 @@ int shadow_client_remdesk_init(rdpShadowClient* client)

 void shadow_client_remdesk_uninit(rdpShadowClient* client)
 {
+	WINPR_ASSERT(client);
 	if (client->remdesk)
 	{
 		client->remdesk->Stop(client->remdesk);
--- a/server/shadow/shadow_server.c
+++ b/server/shadow/shadow_server.c
@@ -993,8 +993,10 @@ static BOOL shadow_server_check_peer_restrictions(freerdp_listener* listener)
 int shadow_server_init(rdpShadowServer* server)
 {
 	int status = 0;
-	winpr_InitializeSSL(WINPR_SSL_INIT_DEFAULT);
-	WTSRegisterWtsApiFunctionTable(FreeRDP_InitWtsApi());
+	if (!winpr_InitializeSSL(WINPR_SSL_INIT_DEFAULT))
+		return -1;
+	if (!WTSRegisterWtsApiFunctionTable(FreeRDP_InitWtsApi()))
+		return -1;

 	if (!(server->clients = ArrayList_New(TRUE)))
 		goto fail;
--- a/server/shadow/shadow_subsystem.c
+++ b/server/shadow/shadow_subsystem.c
@@ -85,7 +85,8 @@ int shadow_subsystem_init(rdpShadowSubsystem* subsystem, rdpShadowServer* server
 	if (!(subsystem->updateEvent = shadow_multiclient_new()))
 		goto fail;

-	if ((status = subsystem->ep.Init(subsystem)) >= 0)
+	status = subsystem->ep.Init(subsystem);
+	if (status >= 0)
 		return status;

 fail: