Fix memory leaks, undefined behaviour, and resource lifecycle bugs#814
Open
KAMI911 wants to merge 4 commits intolinuxmint:masterfrom
Open
Fix memory leaks, undefined behaviour, and resource lifecycle bugs#814KAMI911 wants to merge 4 commits intolinuxmint:masterfrom
KAMI911 wants to merge 4 commits intolinuxmint:masterfrom
Conversation
When a Wayland client creates a single-pixel buffer but never commits it to a surface, meta_wayland_buffer_realize() is never called, so buffer->single_pixel.single_pixel_buffer is never populated. The buffer finalizer therefore calls meta_wayland_single_pixel_buffer_free(NULL), leaking the MetaWaylandSinglePixelBuffer allocated in single_pixel_buffer_manager_create_1px_rgba32_buffer(). Fix this by registering single_pixel_buffer_resource_destroy() as the wl_buffer resource destructor, which calls g_free() on the user data. To avoid a double-free when the buffer *has* been realized (the buffer GObject finalizer would also free the struct), clear the wl_resource user data to NULL in meta_wayland_buffer_realize() immediately after ownership is transferred to the buffer object. g_free(NULL) is a no-op, so the destructor becomes safe in both paths. As a follow-on, update meta_wayland_single_pixel_buffer_attach() to read the pointer from buffer->single_pixel.single_pixel_buffer (which is always valid at that call site) rather than wl_resource_get_user_data(), which is now NULL after realize. Also fix the stride argument passed to cogl_texture_2d_new_from_data(): COGL_PIXEL_FORMAT_BGR_888 uses 3 bytes per pixel, not 4. Pass 3 for the opaque case and keep 4 for COGL_PIXEL_FORMAT_BGRA_8888_PRE. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
frame_slots is declared as guint64 but both sites that update it use the plain integer literal 1, which is a 32-bit int. For touch slot indices >= 32 the expression (1 << slot) shifts past the width of int, which is undefined behaviour in C. Replace both occurrences with ((guint64) 1 << slot) so the shift is always performed on a 64-bit value, matching the type of frame_slots. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The return value of wl_global_create() was discarded, so the global for wp_pointer_warp_v1 was never destroyed when the seat is freed. This leaves a dangling global registered in the Wayland display, which can cause issues during compositor teardown or restart. Store the wl_global* in MetaWaylandPointerWarp and call wl_global_destroy() in meta_wayland_pointer_warp_destroy(). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replace sprintf() with snprintf() so the buffer size is enforced. The format string and input range make overflow impossible in practice, but using snprintf() matches modern C coding standards and silences compiler/static-analyser warnings. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR fixes four independent bugs found by static analysis and code review:
Memory leak in single-pixel-buffer (
meta-wayland-single-pixel-buffer.c,meta-wayland-buffer.c): When a Wayland client creates awp_single_pixel_buffer_v1but never commits it to a surface,meta_wayland_buffer_realize()is never called, leaving theMetaWaylandSinglePixelBufferstruct allocated but never freed. Fixed by adding awl_resourcedestructor that frees the struct in the unrealized case. To avoid a double-free when the buffer is realized,meta_wayland_buffer_realize()clears the resource user-data pointer toNULLimmediately after transferring ownership to the buffer GObject (g_free(NULL)is a no-op). Also fixes a wrong stride:COGL_PIXEL_FORMAT_BGR_888is 3 bytes/pixel, not 4.Undefined behaviour in touch frame-slot bitmask (
meta-wayland-touch.c):frame_slotsisguint64but both update sites used the plainintliteral1. For touch slot indices ≥ 32 the expression(1 << slot)shifts past the width ofint, which is undefined behaviour in C. Changed to((guint64) 1 << slot).wl_globalleak in pointer-warp (meta-wayland-pointer-warp.c): The return value ofwl_global_create()was discarded, so the compositor global forwp_pointer_warp_v1was never destroyed when the seat is freed. Stored the handle inMetaWaylandPointerWarpand callwl_global_destroy()in the destroy function.sprintf→snprintf(meta-x11-display.c): Replacedsprintf()withsnprintf()for theWM_S<n>atom name to enforce the buffer size and suppress static-analyser warnings.Commits
Each fix is a separate self-contained commit.
Test plan
-DWITH_WAYLAND=trueand verify no compile errorsmeson test -C build🤖 Generated with Claude Code