From 148ff54f180d5bbd4ecae0298668f3f99847883b Mon Sep 17 00:00:00 2001 From: Albert Safin Date: Wed, 8 Apr 2020 06:20:07 +0000 Subject: [PATCH 1/4] handlers.c: remove unused arguments from cb_property_handler_t Also, use `conn` global variable instead of passing it as an argument. --- src/handlers.c | 37 +++++++++++++------------------------ 1 file changed, 13 insertions(+), 24 deletions(-) diff --git a/src/handlers.c b/src/handlers.c index 7926fec5..ad394795 100644 --- a/src/handlers.c +++ b/src/handlers.c @@ -558,8 +558,7 @@ static bool window_name_changed(i3Window *window, char *old_name) { * Called when a window changes its title * */ -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) { +static bool handle_windowname_change(xcb_window_t window, xcb_get_property_reply_t *prop) { Con *con; if ((con = con_by_window_id(window)) == NULL || con->window == NULL) return false; @@ -585,8 +584,7 @@ static bool handle_windowname_change(void *data, xcb_connection_t *conn, uint8_t * window_update_name_legacy(). * */ -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) { +static bool handle_windowname_change_legacy(xcb_window_t window, xcb_get_property_reply_t *prop) { Con *con; if ((con = con_by_window_id(window)) == NULL || con->window == NULL) return false; @@ -611,8 +609,7 @@ static bool handle_windowname_change_legacy(void *data, xcb_connection_t *conn, * Called when a window changes its WM_WINDOW_ROLE. * */ -static bool handle_windowrole_change(void *data, xcb_connection_t *conn, uint8_t state, - xcb_window_t window, xcb_atom_t atom, xcb_get_property_reply_t *prop) { +static bool handle_windowrole_change(xcb_window_t window, xcb_get_property_reply_t *prop) { Con *con; if ((con = con_by_window_id(window)) == NULL || con->window == NULL) return false; @@ -956,8 +953,7 @@ static void handle_client_message(xcb_client_message_event_t *event) { } } -static bool handle_window_type(void *data, xcb_connection_t *conn, uint8_t state, xcb_window_t window, - xcb_atom_t atom, xcb_get_property_reply_t *reply) { +static bool handle_window_type(xcb_window_t window, xcb_get_property_reply_t *reply) { Con *con; if ((con = con_by_window_id(window)) == NULL || con->window == NULL) return false; @@ -973,8 +969,7 @@ static bool handle_window_type(void *data, xcb_connection_t *conn, uint8_t state * See ICCCM 4.1.2.3 for more details * */ -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) { +static bool handle_normal_hints(xcb_window_t window, 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"); @@ -999,8 +994,7 @@ static bool handle_normal_hints(void *data, xcb_connection_t *conn, uint8_t stat * Handles the WM_HINTS property for extracting the urgency state of the 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) { +static bool handle_hints(xcb_window_t window, xcb_get_property_reply_t *reply) { Con *con = con_by_window_id(window); if (con == NULL) { DLOG("Received WM_HINTS for unknown client\n"); @@ -1024,8 +1018,7 @@ static bool handle_hints(void *data, xcb_connection_t *conn, uint8_t state, xcb_ * See ICCCM 4.1.2.6 for more details * */ -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) { +static bool handle_transient_for(xcb_window_t window, xcb_get_property_reply_t *prop) { Con *con; if ((con = con_by_window_id(window)) == NULL || con->window == NULL) { @@ -1050,8 +1043,7 @@ static bool handle_transient_for(void *data, xcb_connection_t *conn, uint8_t sta * toolwindow (or similar) and to which window it belongs (logical parent). * */ -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) { +static bool handle_clientleader_change(xcb_window_t window, xcb_get_property_reply_t *prop) { Con *con; if ((con = con_by_window_id(window)) == NULL || con->window == NULL) return false; @@ -1144,8 +1136,7 @@ static void handle_configure_notify(xcb_configure_notify_event_t *event) { * Handles the WM_CLASS property for assignments and criteria selection. * */ -static bool handle_class_change(void *data, xcb_connection_t *conn, uint8_t state, xcb_window_t window, - xcb_atom_t name, xcb_get_property_reply_t *prop) { +static bool handle_class_change(xcb_window_t window, xcb_get_property_reply_t *prop) { Con *con; if ((con = con_by_window_id(window)) == NULL || con->window == NULL) return false; @@ -1169,8 +1160,7 @@ static bool handle_class_change(void *data, xcb_connection_t *conn, uint8_t stat * Handles the _MOTIF_WM_HINTS property of specifing window deocration settings. * */ -static bool handle_motif_hints_change(void *data, xcb_connection_t *conn, uint8_t state, xcb_window_t window, - xcb_atom_t name, xcb_get_property_reply_t *prop) { +static bool handle_motif_hints_change(xcb_window_t window, xcb_get_property_reply_t *prop) { Con *con; if ((con = con_by_window_id(window)) == NULL || con->window == NULL) return false; @@ -1200,8 +1190,7 @@ static bool handle_motif_hints_change(void *data, xcb_connection_t *conn, uint8_ * Handles the _NET_WM_STRUT_PARTIAL property for allocating space for dock clients. * */ -static bool handle_strut_partial_change(void *data, xcb_connection_t *conn, uint8_t state, xcb_window_t window, - xcb_atom_t name, xcb_get_property_reply_t *prop) { +static bool handle_strut_partial_change(xcb_window_t window, xcb_get_property_reply_t *prop) { DLOG("strut partial change for window 0x%08x\n", window); Con *con; @@ -1279,7 +1268,7 @@ static bool handle_strut_partial_change(void *data, xcb_connection_t *conn, uint /* 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); +typedef bool (*cb_property_handler_t)(xcb_window_t window, xcb_get_property_reply_t *property); struct property_handler_t { xcb_atom_t atom; @@ -1345,7 +1334,7 @@ static void property_notify(uint8_t state, xcb_window_t window, xcb_atom_t atom) } /* the handler will free() the reply unless it returns false */ - if (!handler->cb(NULL, conn, state, window, atom, propr)) + if (!handler->cb(window, propr)) FREE(propr); } From 5716ff541ffa8b20c8bd8af8411987dffede9e50 Mon Sep 17 00:00:00 2001 From: Albert Safin Date: Wed, 8 Apr 2020 07:52:00 +0000 Subject: [PATCH 2/4] handlers.c: remove redundant property fetching Some property handlers trying to fetch property again if `prop == NULL`. This is redundant since these properties are either fetched by property_notify() just before or deleted. --- src/handlers.c | 49 ------------------------------------------------- 1 file changed, 49 deletions(-) diff --git a/src/handlers.c b/src/handlers.c index ad394795..1108b383 100644 --- a/src/handlers.c +++ b/src/handlers.c @@ -1002,8 +1002,6 @@ static bool handle_hints(xcb_window_t window, xcb_get_property_reply_t *reply) { } bool urgency_hint; - if (reply == NULL) - reply = xcb_get_property_reply(conn, xcb_icccm_get_wm_hints(conn, window), NULL); window_update_hints(con->window, reply, &urgency_hint); con_set_urgency(con, urgency_hint); tree_render(); @@ -1026,13 +1024,6 @@ static bool handle_transient_for(xcb_window_t window, xcb_get_property_reply_t * return false; } - if (prop == NULL) { - prop = xcb_get_property_reply(conn, xcb_get_property_unchecked(conn, false, window, XCB_ATOM_WM_TRANSIENT_FOR, XCB_ATOM_WINDOW, 0, 32), - NULL); - if (prop == NULL) - return false; - } - window_update_transient_for(con->window, prop); return true; @@ -1048,13 +1039,6 @@ static bool handle_clientleader_change(xcb_window_t window, xcb_get_property_rep if ((con = con_by_window_id(window)) == NULL || con->window == NULL) return false; - if (prop == NULL) { - prop = xcb_get_property_reply(conn, xcb_get_property_unchecked(conn, false, window, A_WM_CLIENT_LEADER, XCB_ATOM_WINDOW, 0, 32), - NULL); - if (prop == NULL) - return false; - } - window_update_leader(con->window, prop); return true; @@ -1141,14 +1125,6 @@ static bool handle_class_change(xcb_window_t window, xcb_get_property_reply_t *p if ((con = con_by_window_id(window)) == NULL || con->window == NULL) return false; - if (prop == NULL) { - prop = xcb_get_property_reply(conn, xcb_get_property_unchecked(conn, false, window, XCB_ATOM_WM_CLASS, XCB_ATOM_STRING, 0, 32), - NULL); - - if (prop == NULL) - return false; - } - window_update_class(con->window, prop); con = remanage_window(con); @@ -1165,14 +1141,6 @@ static bool handle_motif_hints_change(xcb_window_t window, xcb_get_property_repl if ((con = con_by_window_id(window)) == NULL || con->window == NULL) return false; - if (prop == NULL) { - prop = xcb_get_property_reply(conn, xcb_get_property_unchecked(conn, false, window, A__MOTIF_WM_HINTS, XCB_GET_PROPERTY_TYPE_ANY, 0, 5 * sizeof(uint64_t)), - NULL); - - if (prop == NULL) - return false; - } - border_style_t motif_border_style; window_update_motif_hints(con->window, prop, &motif_border_style); @@ -1198,23 +1166,6 @@ static bool handle_strut_partial_change(xcb_window_t window, xcb_get_property_re return false; } - if (prop == NULL) { - xcb_generic_error_t *err = NULL; - xcb_get_property_cookie_t strut_cookie = xcb_get_property(conn, false, window, A__NET_WM_STRUT_PARTIAL, - XCB_GET_PROPERTY_TYPE_ANY, 0, UINT32_MAX); - prop = xcb_get_property_reply(conn, strut_cookie, &err); - - if (err != NULL) { - DLOG("got error when getting strut partial property: %d\n", err->error_code); - free(err); - return false; - } - - if (prop == NULL) { - return false; - } - } - DLOG("That is con %p / %s\n", con, con->name); window_update_strut_partial(con->window, prop); From e03fdef3e5f57eefbd18b6a290b6553dc9f277de Mon Sep 17 00:00:00 2001 From: Albert Safin Date: Wed, 8 Apr 2020 07:59:51 +0000 Subject: [PATCH 3/4] handlers.c: property_notify(): DLOG and return in case of an error --- src/handlers.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/handlers.c b/src/handlers.c index 1108b383..5176e7c1 100644 --- a/src/handlers.c +++ b/src/handlers.c @@ -1265,6 +1265,7 @@ void property_handlers_init(void) { 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; + xcb_generic_error_t *err = NULL; for (size_t c = 0; c < NUM_HANDLERS; c++) { if (property_handlers[c].atom != atom) @@ -1281,7 +1282,12 @@ static void property_notify(uint8_t state, xcb_window_t window, xcb_atom_t atom) if (state != XCB_PROPERTY_DELETE) { xcb_get_property_cookie_t cookie = xcb_get_property(conn, 0, window, atom, XCB_GET_PROPERTY_TYPE_ANY, 0, handler->long_len); - propr = xcb_get_property_reply(conn, cookie, 0); + propr = xcb_get_property_reply(conn, cookie, &err); + if (err != NULL) { + DLOG("got error %d when getting property of atom %d\n", err->error_code, atom); + FREE(err); + return; + } } /* the handler will free() the reply unless it returns false */ From d9d366a65609784481bdf92b5d4765fcd6fb164e Mon Sep 17 00:00:00 2001 From: Albert Safin Date: Wed, 8 Apr 2020 08:32:39 +0000 Subject: [PATCH 4/4] handlers.c: cb_property_handler_t: take Con instead of xcb_window_t Since every handler calls con_by_window_id() and checks for NULL, it is better to move this call into property_notify(). --- src/handlers.c | 93 +++++++++++--------------------------------------- 1 file changed, 19 insertions(+), 74 deletions(-) diff --git a/src/handlers.c b/src/handlers.c index 5176e7c1..22a974ac 100644 --- a/src/handlers.c +++ b/src/handlers.c @@ -558,11 +558,7 @@ static bool window_name_changed(i3Window *window, char *old_name) { * Called when a window changes its title * */ -static bool handle_windowname_change(xcb_window_t window, xcb_get_property_reply_t *prop) { - Con *con; - if ((con = con_by_window_id(window)) == NULL || con->window == NULL) - return false; - +static bool handle_windowname_change(Con *con, xcb_get_property_reply_t *prop) { char *old_name = (con->window->name != NULL ? sstrdup(i3string_as_utf8(con->window->name)) : NULL); window_update_name(con->window, prop); @@ -584,11 +580,7 @@ static bool handle_windowname_change(xcb_window_t window, xcb_get_property_reply * window_update_name_legacy(). * */ -static bool handle_windowname_change_legacy(xcb_window_t window, xcb_get_property_reply_t *prop) { - Con *con; - if ((con = con_by_window_id(window)) == NULL || con->window == NULL) - return false; - +static bool handle_windowname_change_legacy(Con *con, xcb_get_property_reply_t *prop) { char *old_name = (con->window->name != NULL ? sstrdup(i3string_as_utf8(con->window->name)) : NULL); window_update_name_legacy(con->window, prop); @@ -609,11 +601,7 @@ static bool handle_windowname_change_legacy(xcb_window_t window, xcb_get_propert * Called when a window changes its WM_WINDOW_ROLE. * */ -static bool handle_windowrole_change(xcb_window_t window, xcb_get_property_reply_t *prop) { - Con *con; - if ((con = con_by_window_id(window)) == NULL || con->window == NULL) - return false; - +static bool handle_windowrole_change(Con *con, xcb_get_property_reply_t *prop) { window_update_role(con->window, prop); con = remanage_window(con); @@ -953,11 +941,7 @@ static void handle_client_message(xcb_client_message_event_t *event) { } } -static bool handle_window_type(xcb_window_t window, xcb_get_property_reply_t *reply) { - Con *con; - if ((con = con_by_window_id(window)) == NULL || con->window == NULL) - return false; - +static bool handle_window_type(Con *con, xcb_get_property_reply_t *reply) { window_update_type(con->window, reply); return true; } @@ -969,13 +953,7 @@ static bool handle_window_type(xcb_window_t window, xcb_get_property_reply_t *re * See ICCCM 4.1.2.3 for more details * */ -static bool handle_normal_hints(xcb_window_t window, 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 false; - } - +static bool handle_normal_hints(Con *con, xcb_get_property_reply_t *reply) { bool changed = window_update_normal_hints(con->window, reply, NULL); if (changed) { @@ -994,18 +972,11 @@ static bool handle_normal_hints(xcb_window_t window, xcb_get_property_reply_t *r * Handles the WM_HINTS property for extracting the urgency state of the window. * */ -static bool handle_hints(xcb_window_t window, 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 false; - } - +static bool handle_hints(Con *con, xcb_get_property_reply_t *reply) { bool urgency_hint; window_update_hints(con->window, reply, &urgency_hint); con_set_urgency(con, urgency_hint); tree_render(); - return true; } @@ -1016,16 +987,8 @@ static bool handle_hints(xcb_window_t window, xcb_get_property_reply_t *reply) { * See ICCCM 4.1.2.6 for more details * */ -static bool handle_transient_for(xcb_window_t window, 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 false; - } - +static bool handle_transient_for(Con *con, xcb_get_property_reply_t *prop) { window_update_transient_for(con->window, prop); - return true; } @@ -1034,13 +997,8 @@ static bool handle_transient_for(xcb_window_t window, xcb_get_property_reply_t * * toolwindow (or similar) and to which window it belongs (logical parent). * */ -static bool handle_clientleader_change(xcb_window_t window, xcb_get_property_reply_t *prop) { - Con *con; - if ((con = con_by_window_id(window)) == NULL || con->window == NULL) - return false; - +static bool handle_clientleader_change(Con *con, xcb_get_property_reply_t *prop) { window_update_leader(con->window, prop); - return true; } @@ -1120,15 +1078,9 @@ static void handle_configure_notify(xcb_configure_notify_event_t *event) { * Handles the WM_CLASS property for assignments and criteria selection. * */ -static bool handle_class_change(xcb_window_t window, xcb_get_property_reply_t *prop) { - Con *con; - if ((con = con_by_window_id(window)) == NULL || con->window == NULL) - return false; - +static bool handle_class_change(Con *con, xcb_get_property_reply_t *prop) { window_update_class(con->window, prop); - con = remanage_window(con); - return true; } @@ -1136,11 +1088,7 @@ static bool handle_class_change(xcb_window_t window, xcb_get_property_reply_t *p * Handles the _MOTIF_WM_HINTS property of specifing window deocration settings. * */ -static bool handle_motif_hints_change(xcb_window_t window, xcb_get_property_reply_t *prop) { - Con *con; - if ((con = con_by_window_id(window)) == NULL || con->window == NULL) - return false; - +static bool handle_motif_hints_change(Con *con, xcb_get_property_reply_t *prop) { border_style_t motif_border_style; window_update_motif_hints(con->window, prop, &motif_border_style); @@ -1158,16 +1106,7 @@ static bool handle_motif_hints_change(xcb_window_t window, xcb_get_property_repl * Handles the _NET_WM_STRUT_PARTIAL property for allocating space for dock clients. * */ -static bool handle_strut_partial_change(xcb_window_t window, xcb_get_property_reply_t *prop) { - DLOG("strut partial change for window 0x%08x\n", window); - - Con *con; - if ((con = con_by_window_id(window)) == NULL || con->window == NULL) { - return false; - } - - DLOG("That is con %p / %s\n", con, con->name); - +static bool handle_strut_partial_change(Con *con, xcb_get_property_reply_t *prop) { window_update_strut_partial(con->window, prop); /* we only handle this change for dock clients */ @@ -1219,7 +1158,7 @@ static bool handle_strut_partial_change(xcb_window_t window, xcb_get_property_re /* 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)(xcb_window_t window, xcb_get_property_reply_t *property); +typedef bool (*cb_property_handler_t)(Con *con, xcb_get_property_reply_t *property); struct property_handler_t { xcb_atom_t atom; @@ -1266,6 +1205,7 @@ 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; xcb_generic_error_t *err = NULL; + Con *con; for (size_t c = 0; c < NUM_HANDLERS; c++) { if (property_handlers[c].atom != atom) @@ -1280,6 +1220,11 @@ static void property_notify(uint8_t state, xcb_window_t window, xcb_atom_t atom) return; } + if ((con = con_by_window_id(window)) == NULL || con->window == NULL) { + DLOG("Received property for atom %d for unknown client\n", atom); + return; + } + if (state != XCB_PROPERTY_DELETE) { xcb_get_property_cookie_t cookie = xcb_get_property(conn, 0, window, atom, XCB_GET_PROPERTY_TYPE_ANY, 0, handler->long_len); propr = xcb_get_property_reply(conn, cookie, &err); @@ -1291,7 +1236,7 @@ static void property_notify(uint8_t state, xcb_window_t window, xcb_atom_t atom) } /* the handler will free() the reply unless it returns false */ - if (!handler->cb(window, propr)) + if (!handler->cb(con, propr)) FREE(propr); }