[PATCH] Some additional return value checking, updated.

[Kristian Høgsberg]

On Mon, Nov 22, 2010 at 9:24 PM,  <Darxus at chaosreigns.com> wrote:
> ---
>  clients/window.c            |    5 ++++-
>  compositor/compositor-drm.c |    6 +++++-
>  compositor/compositor-x11.c |   22 ++++++++++++++++------
>  3 files changed, 25 insertions(+), 8 deletions(-)

Applied with a few changes, see below.

> diff --git a/clients/window.c b/clients/window.c
> index 9ed96be..8a2c4ab 100644
> --- a/clients/window.c
> +++ b/clients/window.c
> @@ -1463,7 +1463,10 @@ display_create(int *argc, char **argv[], const GOptionEntry *option_entries)
>                return NULL;
>        }
>
> -       eglBindAPI(EGL_OPENGL_API);
> +       if (EGL_FALSE == eglBindAPI(EGL_OPENGL_API)) {
> +               fprintf(stderr, "failed to bind api EGL_OPENGL_API\n");
> +               return NULL;
> +       }

I don't use the "if (EGL_FALSE == stuff())" idiom in wayland.  I get
that the idea is that by putting the constant on the left hand side of
==, we avoid accidents like "if (a = 5) ...", but gcc already warns
about that and "if (5 == a)" just isn't very intuitive. Don't take it
personal, it's one of my pet peeves ;-)

>        d->ctx = eglCreateContext(d->dpy, NULL, EGL_NO_CONTEXT, NULL);
>        if (d->ctx == NULL) {
> diff --git a/compositor/compositor-drm.c b/compositor/compositor-drm.c
> index e843e14..d06f769 100644
> --- a/compositor/compositor-drm.c
> +++ b/compositor/compositor-drm.c
> @@ -339,7 +339,11 @@ init_egl(struct drm_compositor *ec, struct udev_device *device)
>                return -1;
>        }
>
> -       eglBindAPI(EGL_OPENGL_ES_API);
> +       if (EGL_FALSE == eglBindAPI(EGL_OPENGL_ES_API)) {
> +               fprintf(stderr, "failed to bind api EGL_OPENGL_ES_API\n");
> +               return -1;
> +       }
> +
>        ec->base.context = eglCreateContext(ec->base.display, NULL,
>                                            EGL_NO_CONTEXT, context_attribs);
>        if (ec->base.context == NULL) {
> diff --git a/compositor/compositor-x11.c b/compositor/compositor-x11.c
> index f6e705b..c348296 100644
> --- a/compositor/compositor-x11.c
> +++ b/compositor/compositor-x11.c
> @@ -80,19 +80,21 @@ struct x11_input {
>  };
>
>
> -static void
> +static int
>  x11_input_create(struct x11_compositor *c)
>  {
>        struct x11_input *input;
>
>        input = malloc(sizeof *input);
>        if (input == NULL)
> -               return;
> +               return -1;
>
>        memset(input, 0, sizeof *input);
>        wlsc_input_device_init(&input->base, &c->base);
>
>        c->base.input_device = &input->base;
> +
> +       return 0;
>  }
>
>
> @@ -247,7 +249,11 @@ x11_compositor_init_egl(struct x11_compositor *c)
>                return -1;
>        }
>
> -       eglBindAPI(EGL_OPENGL_ES_API);
> +       if (EGL_FALSE == eglBindAPI(EGL_OPENGL_ES_API)) {
> +               fprintf(stderr, "failed to bind EGL_OPENGL_ES_API\n");
> +               return -1;
> +       }
> +
>        c->base.context = eglCreateContext(c->base.display, NULL,
>                                           EGL_NO_CONTEXT, context_attribs);
>        if (c->base.context == NULL) {
> @@ -661,15 +667,17 @@ x11_compositor_create(struct wl_display *display, int width, int height)
>
>        c->base.wl_display = display;
>        if (x11_compositor_init_egl(c) < 0)
> -        return NULL;
> +               return NULL;
>
>        /* Can't init base class until we have a current egl context */
>        if (wlsc_compositor_init(&c->base, display) < 0)
>                return NULL;
>
> -       x11_compositor_create_output(c, width, height);
> +       if (x11_compositor_create_output(c, width, height) < 0)
> +               return NULL;
>
> -       x11_input_create(c);
> +       if (x11_input_create(c) < 0)
> +               return NULL;
>
>        loop = wl_display_get_event_loop(c->base.wl_display);
>
> @@ -679,6 +687,8 @@ x11_compositor_create(struct wl_display *display, int width, int height)
>                                     x11_compositor_handle_event, c);
>
>        c->base.authenticate = x11_authenticate;
> +       if (c->base.authenticate < 0)
> +               return NULL;
>        c->base.present = x11_compositor_present;

This is setting a function pointer, not calling the functions,
checking for < 0 doesn't make sense.  I edited this part out.

Kristian