[PATCH wayland] add wl_abort private function

Bill Spitzak spitzak at gmail.com
Mon Nov 16 11:04:56 PST 2015


This sure looks correct to me.

Using assert(0) seems like it was a mistake, as it would not happen if
NDEBUG was defined but it appears the code is relying on it. And since it
was already doing a call to wl_log no overhead was reduced by the removal.


On Mon, Nov 16, 2015 at 2:49 AM, Marek Chalupa <mchqwerty at gmail.com> wrote:

> On many places in the code we use wl_log + abort or wl_log + assert(0).
> Replace these with one call to wl_abort, so that we don't mix abort(),
> assert(0) and we'll save few lines
>
> Signed-off-by: Marek Chalupa <mchqwerty at gmail.com>
> ---
>  src/connection.c      | 22 +++++++---------------
>  src/wayland-client.c  | 12 ++++--------
>  src/wayland-private.h |  1 +
>  src/wayland-util.c    | 12 ++++++++++++
>  tests/sanity-test.c   |  7 ++++++-
>  5 files changed, 30 insertions(+), 24 deletions(-)
>
> diff --git a/src/connection.c b/src/connection.c
> index b3d9bd4..6742f19 100644
> --- a/src/connection.c
> +++ b/src/connection.c
> @@ -33,7 +33,6 @@
>  #include <stdio.h>
>  #include <errno.h>
>  #include <sys/uio.h>
> -#include <assert.h>
>  #include <fcntl.h>
>  #include <unistd.h>
>  #include <sys/types.h>
> @@ -569,16 +568,12 @@ wl_closure_marshal(struct wl_object *sender,
> uint32_t opcode,
>                 case 'h':
>                         fd = args[i].h;
>                         dup_fd = wl_os_dupfd_cloexec(fd, 0);
> -                       if (dup_fd < 0) {
> -                               wl_log("dup failed: %m");
> -                               abort();
> -                       }
> +                       if (dup_fd < 0)
> +                               wl_abort("dup failed: %s\n",
> strerror(errno));
>                         closure->args[i].h = dup_fd;
>                         break;
>                 default:
> -                       wl_log("unhandled format code: '%c'\n",
> -                               arg.type);
> -                       assert(0);
> +                       wl_abort("unhandled format code: '%c'\n",
> arg.type);
>                         break;
>                 }
>         }
> @@ -771,8 +766,7 @@ wl_connection_demarshal(struct wl_connection
> *connection,
>                         closure->args[i].h = fd;
>                         break;
>                 default:
> -                       wl_log("unknown type\n");
> -                       assert(0);
> +                       wl_abort("unknown type\n");
>                         break;
>                 }
>         }
> @@ -906,8 +900,7 @@ convert_arguments_to_ffi(const char *signature,
> uint32_t flags,
>                         ffi_args[i] = &args[i].h;
>                         break;
>                 default:
> -                       wl_log("unknown type\n");
> -                       assert(0);
> +                       wl_abort("unknown type\n");
>                         break;
>                 }
>         }
> @@ -938,9 +931,8 @@ wl_closure_invoke(struct wl_closure *closure, uint32_t
> flags,
>
>         implementation = target->implementation;
>         if (!implementation[opcode]) {
> -               wl_log("listener function for opcode %u of %s is NULL\n",
> -                       opcode, target->interface->name);
> -               abort();
> +               wl_abort("listener function for opcode %u of %s is NULL\n",
> +                        opcode, target->interface->name);
>         }
>         ffi_call(&cif, implementation[opcode], NULL, ffi_args);
>  }
> diff --git a/src/wayland-client.c b/src/wayland-client.c
> index b1c600f..509be08 100644
> --- a/src/wayland-client.c
> +++ b/src/wayland-client.c
> @@ -597,18 +597,14 @@ wl_proxy_marshal_array_constructor(struct wl_proxy
> *proxy,
>         }
>
>         closure = wl_closure_marshal(&proxy->object, opcode, args,
> message);
> -       if (closure == NULL) {
> -               wl_log("Error marshalling request: %m\n");
> -               abort();
> -       }
> +       if (closure == NULL)
> +               wl_abort("Error marshalling request: %s\n",
> strerror(errno));
>
>         if (debug_client)
>                 wl_closure_print(closure, &proxy->object, true);
>
> -       if (wl_closure_send(closure, proxy->display->connection)) {
> -               wl_log("Error sending request: %m\n");
> -               abort();
> -       }
> +       if (wl_closure_send(closure, proxy->display->connection))
> +               wl_abort("Error sending request: %s\n", strerror(errno));
>
>         wl_closure_destroy(closure);
>
> diff --git a/src/wayland-private.h b/src/wayland-private.h
> index da9040a..58ac952 100644
> --- a/src/wayland-private.h
> +++ b/src/wayland-private.h
> @@ -209,6 +209,7 @@ wl_closure_destroy(struct wl_closure *closure);
>  extern wl_log_func_t wl_log_handler;
>
>  void wl_log(const char *fmt, ...);
> +void wl_abort(const char *fmt, ...);
>
>  struct wl_display;
>
> diff --git a/src/wayland-util.c b/src/wayland-util.c
> index 00265e9..e782309 100644
> --- a/src/wayland-util.c
> +++ b/src/wayland-util.c
> @@ -385,3 +385,15 @@ wl_log(const char *fmt, ...)
>         wl_log_handler(fmt, argp);
>         va_end(argp);
>  }
> +
> +void
> +wl_abort(const char *fmt, ...)
> +{
> +       va_list argp;
> +
> +       va_start(argp, fmt);
> +       wl_log_handler(fmt, argp);
> +       va_end(argp);
> +
> +       abort();
> +}
> diff --git a/tests/sanity-test.c b/tests/sanity-test.c
> index 3f589b5..65d986d 100644
> --- a/tests/sanity-test.c
> +++ b/tests/sanity-test.c
> @@ -31,8 +31,8 @@
>
>  #include "test-runner.h"
>  #include "wayland-util.h"
> +#include "wayland-private.h"
>
> -#define WL_HIDE_DEPRECATED
>  #include "test-compositor.h"
>
>  extern int leak_check_enabled;
> @@ -56,6 +56,11 @@ FAIL_TEST(fail_abort)
>         abort();
>  }
>
> +FAIL_TEST(fail_wl_abort)
> +{
> +       wl_abort("Abort the program\n");
> +}
> +
>  FAIL_TEST(fail_kill)
>  {
>         kill(getpid(), SIGTERM);
> --
> 2.5.0
>
> _______________________________________________
> wayland-devel mailing list
> wayland-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/wayland-devel/attachments/20151116/3b5016f3/attachment.html>


More information about the wayland-devel mailing list