[PATCH weston] xwm: Add and use helper function for looking up windows in the hash table
Derek Foreman
derekf at osg.samsung.com
Wed Apr 8 11:35:44 PDT 2015
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 <derekf at osg.samsung.com>
---
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
More information about the wayland-devel
mailing list