[PATCH weston] xwm: Add and use helper function for looking up windows in the hash table
ppaalanen at gmail.com
Wed Apr 8 23:43:34 PDT 2015
On Wed, 8 Apr 2015 13:35:44 -0500
Derek Foreman <derekf at osg.samsung.com> wrote:
> 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, ...)
> +static bool __attribute__ ((warn_unused_result))
> +wm_lookup_window(struct hash_table *ht, uint32_t hash,
> + struct weston_wm_window **window)
you could make the call sites even shorter if the argument here was a
struct weston_wm *wm instead of struct hash_table *ht. ;-)
Also the uint32_t hash... isn't that always a Window? XID? Would it
work to have that type as Window or whatever it actually always is?
Going to a more explicit type would be a documentation improvement.
> + *window = hash_table_lookup(ht, hash);
> + if (*window)
> + return true;
> + return false;
The patch looks good to me, so
Reviewed-by: Pekka Paalanen <pekka.paalanen at collabora.co.uk>
in any case.
More information about the wayland-devel