<div dir="ltr"><br><div class="gmail_extra"><br><br><div class="gmail_quote">On 22 August 2014 12:40, Pekka Paalanen <span dir="ltr"><<a href="mailto:ppaalanen@gmail.com" target="_blank">ppaalanen@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="">On Tue,  5 Aug 2014 11:43:35 +0200<br>
Marek Chalupa <<a href="mailto:mchqwerty@gmail.com">mchqwerty@gmail.com</a>> wrote:<br>
<br>
</div><div class="">> This prevents from blocking shown in one display test. Also, it<br>
> makes sense to not proceed further in the code of these function<br>
> when an error occurred.<br>
> ---<br>
>  src/wayland-client.c | 6 ++++++<br>
>  1 file changed, 6 insertions(+)<br>
><br>
> diff --git a/src/wayland-client.c b/src/wayland-client.c<br>
> index 68a91f6..f176dd4 100644<br>
> --- a/src/wayland-client.c<br>
> +++ b/src/wayland-client.c<br>
> @@ -1183,6 +1183,9 @@ wl_display_read_events(struct wl_display *display)<br>
>  {<br>
>       int ret;<br>
><br>
> +     if (display->last_error)<br>
> +             return -1;<br>
<br>
</div>The documentation requires that errno is set. We definitely need to set<br>
it, as errno is thread-local. If we are on a thread where<br>
libwayland-client already set errno, we should still overwrite it to<br>
say that there already was an error.<br>
</blockquote><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="">
> +<br>
>       pthread_mutex_lock(&display->mutex);<br>
><br>
>       ret = read_events(display);<br>
> @@ -1229,6 +1232,9 @@ wl_display_prepare_read_queue(struct wl_display *display,<br>
>  {<br>
>       int ret;<br>
><br>
> +     if (display->last_error)<br>
> +             return -1;<br>
<br>
</div>This will need a documentation update for wl_display_prepare_read(),<br>
that -1 can be returned if the wl_display is already in an error state.<br>
It should also set errno.<br>
<div class=""><br></div></blockquote><div><br></div><div>Ok<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="">
> +<br>
>       pthread_mutex_lock(&display->mutex);<br>
><br>
>       if (!wl_list_empty(&queue->event_list)) {<br>
<br>
</div>What errno should we use?<br>
<br>
EDEADLK, because this is preventing a deadlock?<br>
EIO, because... nothing really says "existing error"?<br>
EACCESS, EPERM, because you're not supposed to do this?<br>
EBUSY, because the wl_display has gone to lunch?<br>
ECANCELED, because we cancelled your attempt for your own good?<br>
<br>
"Operation canceled", hmm, I think I like that one. ECANCELED ok?<br>
<br></blockquote><div><br></div><div>Hmm, yes, "Operation canceled" feels good for this case.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">

Also, it seems last_error should always be handled with the mutex<br>
locked.<br>
<br></blockquote><div><br>No, when you're not writing to it, it don't need mutex locked. Once last_error is set, it is not modified<br></div><div>anymore. At least not in the current code, so the mutex is not necessary I think.<br>
</div><div> <br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
Thanks,<br>
pq<br>
</blockquote></div><br></div></div>