Skip to content

fix: free xcb_reply's with scoped smart pointers #4592

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
AlanGriffiths merged 3 commits into from
Jan 6, 2026
+15 −21
Merged

fix: free xcb_reply's with scoped smart pointers #4592

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
dc39a99
fix: free xcb_reply's with scoped smart pointers
aliihsancengiz Dec 28, 2025
d16e5cd
address review comments
aliihsancengiz Jan 5, 2026
c0957e5
Apply possible suggestion
aliihsancengiz Jan 6, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/platforms/x11/graphics/egl_helper.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

#include "egl_helper.h"

#include <mir/c_memory.h>
#include <mir/graphics/gl_config.h>
#include <mir/graphics/egl_error.h>

Expand Down Expand Up @@ -176,10 +177,9 @@ namespace
auto size_for_x_win(xcb_connection_t* xcb_conn, xcb_window_t win) -> mir::geometry::Size
{
auto cookie = xcb_get_geometry(xcb_conn, win);
if (auto reply = xcb_get_geometry_reply(xcb_conn, cookie, nullptr))
if (auto const reply = mir::make_unique_cptr(xcb_get_geometry_reply(xcb_conn, cookie, nullptr)))
{
mir::geometry::Size const window_size{reply->width, reply->height};
free(reply);
return window_size;
}
BOOST_THROW_EXCEPTION((std::runtime_error{"Failed to get X11 window size"}));
Expand Down
8 changes: 4 additions & 4 deletions src/platforms/x11/x11_resources.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@


#define MIR_LOG_COMPONENT "x11-error"
#include <mir/c_memory.h>
#include <mir/log.h>

#include "x11_resources.h"
Expand Down Expand Up @@ -97,19 +98,18 @@ class BasicXCBConnection : public mx::XCBConnection
auto intern_atom(std::string const& name) const -> xcb_atom_t override
{
auto const cookie = xcb_intern_atom(conn, 0, name.size(), name.c_str());
auto const reply = xcb_intern_atom_reply(conn, cookie, nullptr);
auto const reply = mir::make_unique_cptr(xcb_intern_atom_reply(conn, cookie, nullptr));
auto const atom = reply->atom;
free(reply);
return atom;
}

auto get_output_refresh_rate() const -> double override
{
// I'm assuming we handle xcb errors somewhere with events.
auto ver_cookie = xcb_randr_query_version_unchecked(conn, 1, 2);
xcb_randr_query_version_reply(conn, ver_cookie,nullptr);
free(xcb_randr_query_version_reply(conn, ver_cookie, nullptr));
auto screen_cookie = xcb_randr_get_screen_info_unchecked(conn,screen_->root);
auto screen_reply = xcb_randr_get_screen_info_reply(conn, screen_cookie, nullptr);
auto const screen_reply = mir::make_unique_cptr(xcb_randr_get_screen_info_reply(conn, screen_cookie, nullptr));
auto refresh_rate = static_cast<double>(screen_reply->rate);
mir::log_debug("Detected %.2fHz host output refresh rate.", refresh_rate);
return screen_reply->rate;
Comment on lines 113 to 115
Copy link

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing null check before dereferencing screen_reply. If xcb_randr_get_screen_info_reply returns nullptr, line 113 will cause a null pointer dereference. Add a null check similar to the pattern used in other files.

Suggested change
auto refresh_rate = static_cast<double>(screen_reply->rate);
mir::log_debug("Detected %.2fHz host output refresh rate.", refresh_rate);
return screen_reply->rate;
if (!screen_reply)
{
mir::log_warning("Failed to get RANDR screen info; assuming 60Hz host output refresh rate.");
return 60.0;
}
auto refresh_rate = static_cast<double>(screen_reply->rate);
mir::log_debug("Detected %.2fHz host output refresh rate.", refresh_rate);
return refresh_rate;

Copilot uses AI. Check for mistakes.
Copy link
Contributor

@AlanGriffiths AlanGriffiths Jan 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing null check before dereferencing screen_reply

If there is a problem, then it is pre-existing: doesn't block this PR

Expand Down
7 changes: 3 additions & 4 deletions src/server/frontend_xwayland/xwayland_cursors.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
#include "xwayland_cursors.h"
#include "xwayland_log.h"

#include <mir/c_memory.h>
#include <boost/throw_exception.hpp>

#include <charconv>
Expand Down Expand Up @@ -90,11 +91,10 @@ mf::XWaylandCursors::Loader::Loader(std::shared_ptr<XCBConnection> const& connec
auto mf::XWaylandCursors::Loader::query_formats(std::shared_ptr<XCBConnection> const& connection) -> Loader::Formats
{
auto const formats_cookie = xcb_render_query_pict_formats(*connection);
auto const formats_reply = xcb_render_query_pict_formats_reply(*connection, formats_cookie, 0);
mf::XWaylandCursors::Loader::Formats result;
if (formats_reply)
if (auto const formats_reply = mir::make_unique_cptr(xcb_render_query_pict_formats_reply(*connection, formats_cookie, 0)))
{
auto const formats = xcb_render_query_pict_formats_formats(formats_reply);
auto const formats = xcb_render_query_pict_formats_formats(formats_reply.get());
for (unsigned i = 0; i < formats_reply->num_formats; i++)
{
if (formats[i].direct.red_mask != 0xff && formats[i].direct.red_shift != 16)
Expand All @@ -109,7 +109,6 @@ auto mf::XWaylandCursors::Loader::query_formats(std::shared_ptr<XCBConnection> c
}
}

free(formats_reply);
}
else
{
Expand Down
17 changes: 6 additions & 11 deletions src/server/frontend_xwayland/xwayland_wm.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,6 @@ auto create_wm_window(mf::XCBConnection const& connection) -> xcb_window_t
auto init_xfixes(mf::XCBConnection const& connection) -> xcb_query_extension_reply_t const*
{
xcb_xfixes_query_version_cookie_t xfixes_cookie;
xcb_xfixes_query_version_reply_t *xfixes_reply;

xcb_prefetch_extension_data(connection, &xcb_xfixes_id);
xcb_prefetch_extension_data(connection, &xcb_composite_id);
Expand All @@ -90,14 +89,13 @@ auto init_xfixes(mf::XCBConnection const& connection) -> xcb_query_extension_rep
}

xfixes_cookie = xcb_xfixes_query_version(connection, XCB_XFIXES_MAJOR_VERSION, XCB_XFIXES_MINOR_VERSION);
xfixes_reply = xcb_xfixes_query_version_reply(connection, xfixes_cookie, NULL);
auto const xfixes_reply = mir::make_unique_cptr(xcb_xfixes_query_version_reply(connection, xfixes_cookie, NULL));

if (mir::verbose_xwayland_logging_enabled())
{
mir::log_debug("xfixes version: %d.%d", xfixes_reply->major_version, xfixes_reply->minor_version);
}

free(xfixes_reply);
return xfixes;
}

Expand Down Expand Up @@ -289,11 +287,11 @@ void mf::XWaylandWM::handle_events()

connection->verify_not_in_error_state();

while (xcb_generic_event_t* const event = xcb_poll_for_event(*connection))
while (auto const event = mir::make_unique_cptr(xcb_poll_for_event(*connection)))
{
try
{
handle_event(event);
handle_event(event.get());
}
catch (...)
{
Expand All @@ -303,7 +301,6 @@ void mf::XWaylandWM::handle_events()
std::current_exception(),
"Error processing XCB event");
}
free(event);
got_events = true;
}

Expand Down Expand Up @@ -515,14 +512,13 @@ void mf::XWaylandWM::manage_window(xcb_window_t window, geom::Rectangle const& g
if (verbose_xwayland_logging_enabled())
{
auto const props_cookie = xcb_list_properties(*connection, window);
auto const props_reply = xcb_list_properties_reply(*connection, props_cookie, nullptr);
if (props_reply)
if (auto const props_reply = mir::make_unique_cptr(xcb_list_properties_reply(*connection, props_cookie, nullptr)))
{
std::vector<std::function<void()>> functions;
int const prop_count = xcb_list_properties_atoms_length(props_reply);
int const prop_count = xcb_list_properties_atoms_length(props_reply.get());
for (int i = 0; i < prop_count; i++)
{
auto const atom = xcb_list_properties_atoms(props_reply)[i];
auto const atom = xcb_list_properties_atoms(props_reply.get())[i];

auto const log_prop = [this, atom](std::string const& value)
{
Expand All @@ -548,7 +544,6 @@ void mf::XWaylandWM::manage_window(xcb_window_t window, geom::Rectangle const& g
}
}));
}
free(props_reply);

log_debug("%s has %d initial propertie(s):", connection->window_debug_string(window).c_str(), prop_count);
for (auto const& f : functions)
Expand Down
Loading