From 088681c781ae87582dc9977feed0320d703dba7b Mon Sep 17 00:00:00 2001 From: Michael Stapelberg Date: Sun, 31 Jul 2011 17:46:41 +0200 Subject: [PATCH] Bugfix: property handlers: correctly free replies in all cases --- src/handlers.c | 63 +++++++++++++++++++++++++++----------------------- src/window.c | 37 +++++++++++++++++++++++------ 2 files changed, 64 insertions(+), 36 deletions(-) diff --git a/src/handlers.c b/src/handlers.c index b20ef327..0fd7dbbd 100644 --- a/src/handlers.c +++ b/src/handlers.c @@ -543,17 +543,17 @@ static int handle_destroy_notify_event(xcb_destroy_notify_event_t *event) { * Called when a window changes its title * */ -static int handle_windowname_change(void *data, xcb_connection_t *conn, uint8_t state, +static bool handle_windowname_change(void *data, xcb_connection_t *conn, uint8_t state, xcb_window_t window, xcb_atom_t atom, xcb_get_property_reply_t *prop) { Con *con; if ((con = con_by_window_id(window)) == NULL || con->window == NULL) - return 1; + return false; window_update_name(con->window, prop, false); x_push_changes(croot); - return 1; + return true; } /* @@ -561,17 +561,17 @@ static int handle_windowname_change(void *data, xcb_connection_t *conn, uint8_t * window_update_name_legacy(). * */ -static int handle_windowname_change_legacy(void *data, xcb_connection_t *conn, uint8_t state, +static bool handle_windowname_change_legacy(void *data, xcb_connection_t *conn, uint8_t state, xcb_window_t window, xcb_atom_t atom, xcb_get_property_reply_t *prop) { Con *con; if ((con = con_by_window_id(window)) == NULL || con->window == NULL) - return 1; + return false; window_update_name_legacy(con->window, prop, false); x_push_changes(croot); - return 1; + return true; } #if 0 @@ -675,12 +675,12 @@ int handle_window_type(void *data, xcb_connection_t *conn, uint8_t state, xcb_wi * See ICCCM 4.1.2.3 for more details * */ -static int handle_normal_hints(void *data, xcb_connection_t *conn, uint8_t state, xcb_window_t window, +static bool handle_normal_hints(void *data, xcb_connection_t *conn, uint8_t state, xcb_window_t window, xcb_atom_t name, xcb_get_property_reply_t *reply) { Con *con = con_by_window_id(window); if (con == NULL) { DLOG("Received WM_NORMAL_HINTS for unknown client\n"); - return 1; + return false; } xcb_size_hints_t size_hints; @@ -779,34 +779,36 @@ static int handle_normal_hints(void *data, xcb_connection_t *conn, uint8_t state render_and_return: if (changed) tree_render(); - return 1; + FREE(reply); + return true; } /* * Handles the WM_HINTS property for extracting the urgency state of the window. * */ -static int handle_hints(void *data, xcb_connection_t *conn, uint8_t state, xcb_window_t window, +static bool handle_hints(void *data, xcb_connection_t *conn, uint8_t state, xcb_window_t window, xcb_atom_t name, xcb_get_property_reply_t *reply) { Con *con = con_by_window_id(window); if (con == NULL) { DLOG("Received WM_HINTS for unknown client\n"); - return 1; + return false; } xcb_icccm_wm_hints_t hints; if (reply != NULL) { if (!xcb_icccm_get_wm_hints_from_reply(&hints, reply)) - return 1; + return false; } else { if (!xcb_icccm_get_wm_hints_reply(conn, xcb_icccm_get_wm_hints_unchecked(conn, con->window->id), &hints, NULL)) - return 1; + return false; } if (!con->urgent && focused == con) { DLOG("Ignoring urgency flag for current client\n"); - return 1; + FREE(reply); + return true; } /* Update the flag on the client directly */ @@ -832,7 +834,8 @@ static int handle_hints(void *data, xcb_connection_t *conn, uint8_t state, xcb_w } #endif - return 1; + FREE(reply); + return true; } /* @@ -842,20 +845,20 @@ static int handle_hints(void *data, xcb_connection_t *conn, uint8_t state, xcb_w * See ICCCM 4.1.2.6 for more details * */ -static int handle_transient_for(void *data, xcb_connection_t *conn, uint8_t state, xcb_window_t window, +static bool handle_transient_for(void *data, xcb_connection_t *conn, uint8_t state, xcb_window_t window, xcb_atom_t name, xcb_get_property_reply_t *prop) { Con *con; if ((con = con_by_window_id(window)) == NULL || con->window == NULL) { DLOG("No such window\n"); - return 1; + return false; } if (prop == NULL) { prop = xcb_get_property_reply(conn, xcb_get_property_unchecked(conn, false, window, A_WM_TRANSIENT_FOR, A_WINDOW, 0, 32), NULL); if (prop == NULL) - return 1; + return false; } window_update_transient_for(con->window, prop); @@ -868,7 +871,7 @@ static int handle_transient_for(void *data, xcb_connection_t *conn, uint8_t stat } #endif - return 1; + return true; } /* @@ -876,22 +879,22 @@ static int handle_transient_for(void *data, xcb_connection_t *conn, uint8_t stat * toolwindow (or similar) and to which window it belongs (logical parent). * */ -static int handle_clientleader_change(void *data, xcb_connection_t *conn, uint8_t state, xcb_window_t window, +static bool handle_clientleader_change(void *data, xcb_connection_t *conn, uint8_t state, xcb_window_t window, xcb_atom_t name, xcb_get_property_reply_t *prop) { Con *con; if ((con = con_by_window_id(window)) == NULL || con->window == NULL) - return 1; + return false; if (prop == NULL) { prop = xcb_get_property_reply(conn, xcb_get_property_unchecked(conn, false, window, A_WM_CLIENT_LEADER, A_WINDOW, 0, 32), NULL); if (prop == NULL) - return 1; + return false; } window_update_leader(con->window, prop); - return 1; + return true; } /* @@ -931,7 +934,9 @@ static int handle_focus_in(xcb_focus_in_event_t *event) { return 1; } -typedef int (*cb_property_handler_t)(void *data, xcb_connection_t *c, uint8_t state, xcb_window_t window, xcb_atom_t atom, xcb_get_property_reply_t *property); +/* Returns false if the event could not be processed (e.g. the window could not + * be found), true otherwise */ +typedef bool (*cb_property_handler_t)(void *data, xcb_connection_t *c, uint8_t state, xcb_window_t window, xcb_atom_t atom, xcb_get_property_reply_t *property); struct property_handler_t { xcb_atom_t atom; @@ -963,10 +968,9 @@ void property_handlers_init() { property_handlers[5].atom = A_WM_TRANSIENT_FOR; } -static int property_notify(uint8_t state, xcb_window_t window, xcb_atom_t atom) { +static void property_notify(uint8_t state, xcb_window_t window, xcb_atom_t atom) { struct property_handler_t *handler = NULL; xcb_get_property_reply_t *propr = NULL; - int ret; for (int c = 0; c < sizeof(property_handlers) / sizeof(struct property_handler_t); c++) { if (property_handlers[c].atom != atom) @@ -978,7 +982,7 @@ static int property_notify(uint8_t state, xcb_window_t window, xcb_atom_t atom) if (handler == NULL) { DLOG("Unhandled property notify for atom %d (0x%08x)\n", atom, atom); - return 0; + return; } if (state != XCB_PROPERTY_DELETE) { @@ -986,8 +990,9 @@ static int property_notify(uint8_t state, xcb_window_t window, xcb_atom_t atom) propr = xcb_get_property_reply(conn, cookie, 0); } - /* the handler will free() the reply */ - return handler->cb(NULL, conn, state, window, atom, propr); + /* the handler will free() the reply unless it returns false */ + if (!handler->cb(NULL, conn, state, window, atom, propr)) + FREE(propr); } /* diff --git a/src/window.c b/src/window.c index 0449b1f5..3dd66456 100644 --- a/src/window.c +++ b/src/window.c @@ -15,6 +15,7 @@ void window_update_class(i3Window *win, xcb_get_property_reply_t *prop, bool before_mgmt) { if (prop == NULL || xcb_get_property_value_length(prop) == 0) { DLOG("empty property, not updating\n"); + FREE(prop); return; } @@ -33,8 +34,10 @@ void window_update_class(i3Window *win, xcb_get_property_reply_t *prop, bool bef LOG("WM_CLASS changed to %s (instance), %s (class)\n", win->class_instance, win->class_class); - if (before_mgmt) + if (before_mgmt) { + free(prop); return; + } run_assignments(win); @@ -49,6 +52,7 @@ void window_update_class(i3Window *win, xcb_get_property_reply_t *prop, bool bef void window_update_name(i3Window *win, xcb_get_property_reply_t *prop, bool before_mgmt) { if (prop == NULL || xcb_get_property_value_length(prop) == 0) { DLOG("_NET_WM_NAME not specified, not changing\n"); + FREE(prop); return; } @@ -58,6 +62,7 @@ void window_update_name(i3Window *win, xcb_get_property_reply_t *prop, bool befo (char*)xcb_get_property_value(prop)) == -1) { perror("asprintf()"); DLOG("Could not get window name\n"); + free(prop); return; } /* Convert it to UCS-2 here for not having to convert it later every time we want to pass it to X */ @@ -66,6 +71,7 @@ void window_update_name(i3Window *win, xcb_get_property_reply_t *prop, bool befo if (ucs2_name == NULL) { LOG("Could not convert _NET_WM_NAME to UCS-2, ignoring new hint\n"); FREE(new_name); + free(prop); return; } FREE(win->name_x); @@ -78,8 +84,10 @@ void window_update_name(i3Window *win, xcb_get_property_reply_t *prop, bool befo win->uses_net_wm_name = true; - if (before_mgmt) + if (before_mgmt) { + free(prop); return; + } run_assignments(win); @@ -96,18 +104,22 @@ void window_update_name(i3Window *win, xcb_get_property_reply_t *prop, bool befo void window_update_name_legacy(i3Window *win, xcb_get_property_reply_t *prop, bool before_mgmt) { if (prop == NULL || xcb_get_property_value_length(prop) == 0) { DLOG("prop == NULL\n"); + FREE(prop); return; } /* ignore update when the window is known to already have a UTF-8 name */ - if (win->uses_net_wm_name) + if (win->uses_net_wm_name) { + free(prop); return; + } char *new_name; if (asprintf(&new_name, "%.*s", xcb_get_property_value_length(prop), (char*)xcb_get_property_value(prop)) == -1) { perror("asprintf()"); DLOG("Could not get legacy window name\n"); + free(prop); return; } @@ -121,8 +133,10 @@ void window_update_name_legacy(i3Window *win, xcb_get_property_reply_t *prop, bo win->name_len = strlen(new_name); win->name_x_changed = true; - if (before_mgmt) + if (before_mgmt) { + free(prop); return; + } run_assignments(win); @@ -136,12 +150,15 @@ void window_update_name_legacy(i3Window *win, xcb_get_property_reply_t *prop, bo void window_update_leader(i3Window *win, xcb_get_property_reply_t *prop) { if (prop == NULL || xcb_get_property_value_length(prop) == 0) { DLOG("prop == NULL\n"); + FREE(prop); return; } xcb_window_t *leader = xcb_get_property_value(prop); - if (leader == NULL) + if (leader == NULL) { + free(prop); return; + } DLOG("Client leader changed to %08x\n", *leader); @@ -157,12 +174,15 @@ void window_update_leader(i3Window *win, xcb_get_property_reply_t *prop) { void window_update_transient_for(i3Window *win, xcb_get_property_reply_t *prop) { if (prop == NULL || xcb_get_property_value_length(prop) == 0) { DLOG("prop == NULL\n"); + FREE(prop); return; } xcb_window_t transient_for; - if (!xcb_icccm_get_wm_transient_for_from_reply(&transient_for, prop)) + if (!xcb_icccm_get_wm_transient_for_from_reply(&transient_for, prop)) { + free(prop); return; + } DLOG("Transient for changed to %08x\n", transient_for); @@ -178,12 +198,15 @@ void window_update_transient_for(i3Window *win, xcb_get_property_reply_t *prop) void window_update_strut_partial(i3Window *win, xcb_get_property_reply_t *prop) { if (prop == NULL || xcb_get_property_value_length(prop) == 0) { DLOG("prop == NULL\n"); + FREE(prop); return; } uint32_t *strut; - if (!(strut = xcb_get_property_value(prop))) + if (!(strut = xcb_get_property_value(prop))) { + free(prop); return; + } DLOG("Reserved pixels changed to: left = %d, right = %d, top = %d, bottom = %d\n", strut[0], strut[1], strut[2], strut[3]);