This lets us verify that all callers are actually testing for a successful hash lookup at compile time.
All current users of hash_table_lookup are converted to the new wm_lookup_window() and the appropriate success check is added. This fixes any call sites that used to assume a successful return and dereference a NULL pointer. This closes http://bugs.freedesktop.org/show_bug.cgi?id=83994 The xwayland test has been failing because weston crashes due to a hash lookup failure and a subsequent dereference of the returned NULL pointer. Signed-off-by: Derek Foreman <[email protected]> --- This is a rewrite of two previous patches, differences are: Use a helper function instead of changing hash_table_lookup combine the two patches into one try to limit the use of temporary variables to places where if statements become ugly (due to 80 col constraints). xwayland/window-manager.c | 91 ++++++++++++++++++++++++++++++++++++----------- 1 file changed, 70 insertions(+), 21 deletions(-) diff --git a/xwayland/window-manager.c b/xwayland/window-manager.c index 83ebfae..966962e 100644 --- a/xwayland/window-manager.c +++ b/xwayland/window-manager.c @@ -195,6 +195,15 @@ wm_log_continue(const char *fmt, ...) #endif } +static bool __attribute__ ((warn_unused_result)) +wm_lookup_window(struct hash_table *ht, uint32_t hash, + struct weston_wm_window **window) +{ + *window = hash_table_lookup(ht, hash); + if (*window) + return true; + return false; +} const char * get_atom_name(xcb_connection_t *c, xcb_atom_t atom) @@ -448,8 +457,10 @@ weston_wm_window_read_properties(struct weston_wm_window *window) break; case XCB_ATOM_WINDOW: xid = xcb_get_property_value(reply); - *(struct weston_wm_window **) p = - hash_table_lookup(wm->window_hash, *xid); + if (!wm_lookup_window(wm->window_hash, *xid, + (struct weston_wm_window **)p)) + weston_log("XCB_ATOM_WINDOW contains window" + " id not found in hash table.\n"); break; case XCB_ATOM_CARDINAL: case XCB_ATOM_ATOM: @@ -588,13 +599,18 @@ weston_wm_handle_configure_request(struct weston_wm *wm, xcb_generic_event_t *ev struct weston_wm_window *window; uint32_t mask, values[16]; int x, y, width, height, i = 0; + bool found; wm_log("XCB_CONFIGURE_REQUEST (window %d) %d,%d @ %dx%d\n", configure_request->window, configure_request->x, configure_request->y, configure_request->width, configure_request->height); - window = hash_table_lookup(wm->window_hash, configure_request->window); + found = wm_lookup_window(wm->window_hash, + configure_request->window, + &window); + if (!found) + return; if (window->fullscreen) { weston_wm_window_send_configure_notify(window); @@ -654,13 +670,19 @@ weston_wm_handle_configure_notify(struct weston_wm *wm, xcb_generic_event_t *eve xcb_configure_notify_event_t *configure_notify = (xcb_configure_notify_event_t *) event; struct weston_wm_window *window; + bool found; wm_log("XCB_CONFIGURE_NOTIFY (window %d) %d,%d @ %dx%d\n", configure_notify->window, configure_notify->x, configure_notify->y, configure_notify->width, configure_notify->height); - window = hash_table_lookup(wm->window_hash, configure_notify->window); + found = wm_lookup_window(wm->window_hash, + configure_notify->window, + &window); + if (!found) + return; + window->x = configure_notify->x; window->y = configure_notify->y; if (window->override_redirect) { @@ -925,6 +947,7 @@ weston_wm_handle_map_request(struct weston_wm *wm, xcb_generic_event_t *event) xcb_map_request_event_t *map_request = (xcb_map_request_event_t *) event; struct weston_wm_window *window; + bool found; if (our_resource(wm, map_request->window)) { wm_log("XCB_MAP_REQUEST (window %d, ours)\n", @@ -932,7 +955,11 @@ weston_wm_handle_map_request(struct weston_wm *wm, xcb_generic_event_t *event) return; } - window = hash_table_lookup(wm->window_hash, map_request->window); + found = wm_lookup_window(wm->window_hash, + map_request->window, + &window); + if (!found) + return; weston_wm_window_read_properties(window); @@ -970,6 +997,7 @@ weston_wm_handle_unmap_notify(struct weston_wm *wm, xcb_generic_event_t *event) xcb_unmap_notify_event_t *unmap_notify = (xcb_unmap_notify_event_t *) event; struct weston_wm_window *window; + bool found; wm_log("XCB_UNMAP_NOTIFY (window %d, event %d%s)\n", unmap_notify->window, @@ -984,7 +1012,12 @@ weston_wm_handle_unmap_notify(struct weston_wm *wm, xcb_generic_event_t *event) * as it may come in after we've destroyed the window. */ return; - window = hash_table_lookup(wm->window_hash, unmap_notify->window); + found = wm_lookup_window(wm->window_hash, + unmap_notify->window, + &window); + if (!found) + return; + if (wm->focus_window == window) wm->focus_window = NULL; if (window->surface) @@ -1112,9 +1145,12 @@ weston_wm_handle_property_notify(struct weston_wm *wm, xcb_generic_event_t *even xcb_property_notify_event_t *property_notify = (xcb_property_notify_event_t *) event; struct weston_wm_window *window; + bool found; - window = hash_table_lookup(wm->window_hash, property_notify->window); - if (!window) + found = wm_lookup_window(wm->window_hash, + property_notify->window, + &window); + if (!found) return; window->properties_dirty = 1; @@ -1223,6 +1259,7 @@ weston_wm_handle_destroy_notify(struct weston_wm *wm, xcb_generic_event_t *event xcb_destroy_notify_event_t *destroy_notify = (xcb_destroy_notify_event_t *) event; struct weston_wm_window *window; + bool found; wm_log("XCB_DESTROY_NOTIFY, win %d, event %d%s\n", destroy_notify->window, @@ -1232,7 +1269,12 @@ weston_wm_handle_destroy_notify(struct weston_wm *wm, xcb_generic_event_t *event if (our_resource(wm, destroy_notify->window)) return; - window = hash_table_lookup(wm->window_hash, destroy_notify->window); + found = wm_lookup_window(wm->window_hash, + destroy_notify->window, + &window); + if (!found) + return; + weston_wm_window_destroy(window); } @@ -1242,6 +1284,7 @@ weston_wm_handle_reparent_notify(struct weston_wm *wm, xcb_generic_event_t *even xcb_reparent_notify_event_t *reparent_notify = (xcb_reparent_notify_event_t *) event; struct weston_wm_window *window; + bool found; wm_log("XCB_REPARENT_NOTIFY (window %d, parent %d, event %d)\n", reparent_notify->window, @@ -1253,8 +1296,11 @@ weston_wm_handle_reparent_notify(struct weston_wm *wm, xcb_generic_event_t *even reparent_notify->x, reparent_notify->y, reparent_notify->override_redirect); } else if (!our_resource(wm, reparent_notify->parent)) { - window = hash_table_lookup(wm->window_hash, - reparent_notify->window); + found = wm_lookup_window(wm->window_hash, + reparent_notify->window, + &window); + if (!found) + return; weston_wm_window_destroy(window); } } @@ -1491,8 +1537,11 @@ weston_wm_handle_client_message(struct weston_wm *wm, xcb_client_message_event_t *client_message = (xcb_client_message_event_t *) event; struct weston_wm_window *window; + bool found; - window = hash_table_lookup(wm->window_hash, client_message->window); + found = wm_lookup_window(wm->window_hash, + client_message->window, + &window); wm_log("XCB_CLIENT_MESSAGE (%s %d %d %d %d %d win %d)\n", get_atom_name(wm->conn, client_message->type), @@ -1506,7 +1555,7 @@ weston_wm_handle_client_message(struct weston_wm *wm, /* The window may get created and destroyed before we actually * handle the message. If it doesn't exist, bail. */ - if (!window) + if (!found) return; if (client_message->type == wm->atom.net_wm_moveresize) @@ -1722,8 +1771,8 @@ weston_wm_handle_button(struct weston_wm *wm, xcb_generic_event_t *event) button->response_type == XCB_BUTTON_PRESS ? "PRESS" : "RELEASE", button->detail); - window = hash_table_lookup(wm->window_hash, button->event); - if (!window || !window->decorate) + if (!wm_lookup_window(wm->window_hash, button->event, &window) || + !window->decorate) return; if (button->detail != 1 && button->detail != 2) @@ -1786,8 +1835,8 @@ weston_wm_handle_motion(struct weston_wm *wm, xcb_generic_event_t *event) enum theme_location location; int cursor; - window = hash_table_lookup(wm->window_hash, motion->event); - if (!window || !window->decorate) + if (!wm_lookup_window(wm->window_hash, motion->event, &window) || + !window->decorate) return; location = frame_pointer_motion(window->frame, NULL, @@ -1807,8 +1856,8 @@ weston_wm_handle_enter(struct weston_wm *wm, xcb_generic_event_t *event) enum theme_location location; int cursor; - window = hash_table_lookup(wm->window_hash, enter->event); - if (!window || !window->decorate) + if (!wm_lookup_window(wm->window_hash, enter->event, &window) || + !window->decorate) return; location = frame_pointer_enter(window->frame, NULL, @@ -1826,8 +1875,8 @@ weston_wm_handle_leave(struct weston_wm *wm, xcb_generic_event_t *event) xcb_leave_notify_event_t *leave = (xcb_leave_notify_event_t *) event; struct weston_wm_window *window; - window = hash_table_lookup(wm->window_hash, leave->event); - if (!window || !window->decorate) + if (!wm_lookup_window(wm->window_hash, leave->event, &window) || + !window->decorate) return; frame_pointer_leave(window->frame, NULL); -- 2.1.4 _______________________________________________ wayland-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/wayland-devel
