On Wed,  8 Apr 2015 13:35:44 -0500
Derek Foreman <[email protected]> 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 <[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)

Hi,

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 <[email protected]>
in any case.


Thanks,
pq
_______________________________________________
wayland-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

Reply via email to