On Fri, 25 Aug 2017 12:23:32 +1000
[email protected] wrote:

> From: Christopher James Halse Rogers <[email protected]>
> 
> This *technically* changes the semantics of the return value of the source 
> callbacks.
> Previously you could return a negative number from a source callback and it 
> would prevent
> *other* source callbacks from triggering a subsequent recheck.
> 
> Doing that seems like such a bad idea it's not worth supporting.
> 
> Signed-off-by: Christopher James Halse Rogers 
> <[email protected]>
> ---
>  src/event-loop.c | 16 +++++++---------
>  1 file changed, 7 insertions(+), 9 deletions(-)
> 
> diff --git a/src/event-loop.c b/src/event-loop.c
> index 6130d2a..7d4c08d 100644
> --- a/src/event-loop.c
> +++ b/src/event-loop.c
> @@ -29,6 +29,7 @@
>  #include <signal.h>
>  #include <stdlib.h>
>  #include <stdint.h>
> +#include <stdbool.h>
>  #include <string.h>
>  #include <fcntl.h>
>  #include <sys/socket.h>
> @@ -376,19 +377,18 @@ wl_event_loop_destroy(struct wl_event_loop *loop)
>       free(loop);
>  }
>  
> -static int
> +static bool
>  post_dispatch_check(struct wl_event_loop *loop)
>  {
>       struct epoll_event ep;
>       struct wl_event_source *source, *next;
> -     int n;
> +     bool needs_recheck = false;
>  
>       ep.events = 0;
> -     n = 0;
>       wl_list_for_each_safe(source, next, &loop->check_list, link)
> -             n += source->interface->dispatch(source, &ep);
> +             needs_recheck |= source->interface->dispatch(source, &ep) != 0;
>  
> -     return n;
> +     return needs_recheck;
>  }
>  
>  WL_EXPORT void
> @@ -409,7 +409,7 @@ wl_event_loop_dispatch(struct wl_event_loop *loop, int 
> timeout)
>  {
>       struct epoll_event ep[32];
>       struct wl_event_source *source;
> -     int i, count, n;
> +     int i, count;
>  
>       wl_event_loop_dispatch_idle(loop);
>  
> @@ -427,9 +427,7 @@ wl_event_loop_dispatch(struct wl_event_loop *loop, int 
> timeout)
>  
>       wl_event_loop_dispatch_idle(loop);
>  
> -     do {
> -             n = post_dispatch_check(loop);
> -     } while (n > 0);
> +     while (post_dispatch_check(loop));
>  
>       return 0;
>  }

Hi,

ooh, nasty indeed. A good find.

Reviewed-by: Pekka Paalanen <[email protected]>

The only thing I wonder if we should add a check for a negative return
from ->dispatch() and log a warning in that case. In the unfortunate
event if someone actually used a negative value, on purpose or by
accident, the behaviour change should not be silent. OTOH, I think
wl_abort() would be too rough to add in hindsight.


Thanks,
pq

Attachment: pgpsqU5AFmNfF.pgp
Description: OpenPGP digital signature

_______________________________________________
wayland-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Reply via email to