On Tue, 7 Apr 2015 12:12:15 -0500 Derek Foreman <[email protected]> wrote:
> Make sure we always test hash_table_lookup()s return to prevent trying to > dereference a NULL window. > > Signed-off-by: Derek Foreman <[email protected]> > --- > xwayland/window-manager.c | 103 > +++++++++++++++++++++++++++++++++------------- > 1 file changed, 74 insertions(+), 29 deletions(-) > > diff --git a/xwayland/window-manager.c b/xwayland/window-manager.c > index 3967670..9388d2e 100644 > --- a/xwayland/window-manager.c > +++ b/xwayland/window-manager.c > @@ -446,10 +446,16 @@ weston_wm_window_read_properties(struct > weston_wm_window *window) > strndup(xcb_get_property_value(reply), > xcb_get_property_value_length(reply)); > break; > - case XCB_ATOM_WINDOW: > + case XCB_ATOM_WINDOW: { > + int found; > xid = xcb_get_property_value(reply); > - hash_table_lookup(wm->window_hash, *xid, (struct > weston_wm_window **)p); > + found = hash_table_lookup(wm->window_hash, *xid, > + (struct weston_wm_window **)p); > + if (!found) > + weston_log("XCB_ATOM_WINDOW contains window" > + " id not found in hash table.\n"); > break; > + } > case XCB_ATOM_CARDINAL: > case XCB_ATOM_ATOM: > atom = xcb_get_property_value(reply); > @@ -586,14 +592,18 @@ weston_wm_handle_configure_request(struct weston_wm > *wm, xcb_generic_event_t *ev > (xcb_configure_request_event_t *) event; > struct weston_wm_window *window; > uint32_t mask, values[16]; > - int x, y, width, height, i = 0; > + int x, y, width, height, i = 0, 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); > > - hash_table_lookup(wm->window_hash, configure_request->window, &window); > + found = hash_table_lookup(wm->window_hash, > + configure_request->window, > + &window); > + if (!found) > + return; > > if (window->fullscreen) { > weston_wm_window_send_configure_notify(window); > @@ -650,6 +660,8 @@ our_resource(struct weston_wm *wm, uint32_t id) > static void > weston_wm_handle_configure_notify(struct weston_wm *wm, xcb_generic_event_t > *event) > { > + int found; > + > xcb_configure_notify_event_t *configure_notify = > (xcb_configure_notify_event_t *) event; > struct weston_wm_window *window; > @@ -659,7 +671,12 @@ weston_wm_handle_configure_notify(struct weston_wm *wm, > xcb_generic_event_t *eve > configure_notify->x, configure_notify->y, > configure_notify->width, configure_notify->height); > > - hash_table_lookup(wm->window_hash, configure_notify->window, &window); > + found = hash_table_lookup(wm->window_hash, > + configure_notify->window, > + &window); > + if (!found) > + return; Hi, on a quick look this whole patch seems fine, I'll read it properly the next time. :-) Would it save anything to have a helper: bool wm_lookup_window(struct weston_wm *wm, uint32_t xid, struct weston_wm_window **ret); or something like that? We're always passing wm->window_hash anyway. Letting you write found = wm_lookup_window(wm, blargh, &window); which is a bit shorter. That way you wouldn't need to change the pointer type for hash_table_lookup() either, leaving it generic if we happen to need a hash of something else. Hm? Thanks, pq _______________________________________________ wayland-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/wayland-devel
