[PATCH] client: check for error before prepare/read

[Marek Chalupa]

On 22 August 2014 12:40, Pekka Paalanen <ppaalanen at gmail.com> wrote:

> On Tue,  5 Aug 2014 11:43:35 +0200
> Marek Chalupa <mchqwerty at gmail.com> wrote:
>
> > This prevents from blocking shown in one display test. Also, it
> > makes sense to not proceed further in the code of these function
> > when an error occurred.
> > ---
> >  src/wayland-client.c | 6 ++++++
> >  1 file changed, 6 insertions(+)
> >
> > diff --git a/src/wayland-client.c b/src/wayland-client.c
> > index 68a91f6..f176dd4 100644
> > --- a/src/wayland-client.c
> > +++ b/src/wayland-client.c
> > @@ -1183,6 +1183,9 @@ wl_display_read_events(struct wl_display *display)
> >  {
> >       int ret;
> >
> > +     if (display->last_error)
> > +             return -1;
>
> The documentation requires that errno is set. We definitely need to set
> it, as errno is thread-local. If we are on a thread where
> libwayland-client already set errno, we should still overwrite it to
> say that there already was an error.
>


> > +
> >       pthread_mutex_lock(&display->mutex);
> >
> >       ret = read_events(display);
> > @@ -1229,6 +1232,9 @@ wl_display_prepare_read_queue(struct wl_display
> *display,
> >  {
> >       int ret;
> >
> > +     if (display->last_error)
> > +             return -1;
>
> This will need a documentation update for wl_display_prepare_read(),
> that -1 can be returned if the wl_display is already in an error state.
> It should also set errno.
>
>
Ok


> > +
> >       pthread_mutex_lock(&display->mutex);
> >
> >       if (!wl_list_empty(&queue->event_list)) {
>
> What errno should we use?
>
> EDEADLK, because this is preventing a deadlock?
> EIO, because... nothing really says "existing error"?
> EACCESS, EPERM, because you're not supposed to do this?
> EBUSY, because the wl_display has gone to lunch?
> ECANCELED, because we cancelled your attempt for your own good?
>
> "Operation canceled", hmm, I think I like that one. ECANCELED ok?
>
>
Hmm, yes, "Operation canceled" feels good for this case.


> Also, it seems last_error should always be handled with the mutex
> locked.
>
>
No, when you're not writing to it, it don't need mutex locked. Once
last_error is set, it is not modified
anymore. At least not in the current code, so the mutex is not necessary I
think.


>
> Thanks,
> pq
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/wayland-devel/attachments/20140822/2a04fc22/attachment.html>