[PATCH 1/2] clients & tests: Unify multiple definitions of x*alloc and related functions

Pekka Paalanen ppaalanen at gmail.com
Wed Mar 16 09:46:14 UTC 2016


On Tue, 15 Mar 2016 15:23:30 -0700
Bryce Harrington <bryce at osg.samsung.com> wrote:

> Direct fail_on_null calls now produce output like:
> 
>     [weston-info] clients/weston-info.c:714: out of memory
> 
> xmalloc, et al produce output on failure like:
> 
>     [weston-info] out of memory (-1)
> 
> Signed-off-by: Bryce Harrington <bryce at osg.samsung.com>
> ---
>  Makefile.am                        |  6 +++-
>  clients/clickdot.c                 |  1 +
>  clients/cliptest.c                 |  1 +
>  clients/desktop-shell.c            |  1 +
>  clients/dnd.c                      |  1 +
>  clients/editor.c                   |  1 +
>  clients/ivi-shell-user-interface.c | 13 +------
>  clients/keyboard.c                 |  1 +
>  clients/multi-resource.c           | 15 +-------
>  clients/resizor.c                  |  1 +
>  clients/screenshot.c               | 16 +--------
>  clients/subsurfaces.c              |  1 +
>  clients/terminal.c                 |  1 +
>  clients/weston-info.c              | 30 +---------------
>  clients/window.c                   | 44 +++---------------------
>  clients/window.h                   | 11 ------
>  ivi-shell/hmi-controller.c         | 12 +------
>  shared/xalloc.c                    | 54 +++++++++++++++++++++++++++++
>  shared/xalloc.h                    | 70 ++++++++++++++++++++++++++++++++++++++
>  tests/ivi_layout-test.c            |  1 +
>  tests/presentation-test.c          |  1 +
>  tests/weston-test-client-helper.c  | 14 ++------
>  tests/weston-test-client-helper.h  | 15 --------
>  23 files changed, 152 insertions(+), 159 deletions(-)
>  create mode 100644 shared/xalloc.c
>  create mode 100644 shared/xalloc.h

Hi Bryce,

the idea is cool. The only comments I have are on the Makefile and one
little thing on window.c calls. Everything else looks good to me.

> diff --git a/Makefile.am b/Makefile.am
> index fe08d94..5a98b87 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -93,6 +93,7 @@ weston_SOURCES =					\
>  	shared/matrix.c					\
>  	shared/matrix.h					\
>  	shared/timespec-util.h				\
> +	shared/xalloc.h					\

I don't think this belongs in weston_SOURCES. The compositor bits
should never use such exit() calls, we have to try to shut down
gracefully. We do have SIGABRT and SEGV handlers for emergencies, but I
have a feeling that exit() will skip them, and leave e.g. the VT in a
bad state.

The compositor bits never do call fail_on_null() or x*alloc() anyway.

>  	shared/zalloc.h					\
>  	shared/platform.h				\
>  	src/weston-egl-ext.h
> @@ -210,6 +211,7 @@ westoninclude_HEADERS =				\
>  	src/timeline-object.h			\
>  	shared/matrix.h				\
>  	shared/config-parser.h			\
> +	shared/xalloc.h				\

If xalloc.h is installed, where do other projects get the xalloc.c
binary required to be able to use xalloc.h API?

Weston does not install libshared.la and should not, we just build it
into everything statically, as a helper library.

Either we should not install xalloc.h, or the whole implementation must
be in the .h without any .c file, like with zalloc.h. We certainly
don't want to install a shared library just for fail_on_null(), which
would then also need to be namespaced.

>  	shared/zalloc.h				\
>  	shared/platform.h
>  
> @@ -998,7 +1000,9 @@ libshared_la_SOURCES =				\
>  	shared/file-util.h			\
>  	shared/helpers.h			\
>  	shared/os-compatibility.c		\
> -	shared/os-compatibility.h
> +	shared/os-compatibility.h		\
> +	shared/xalloc.c			\
> +	shared/xalloc.h

This part is good.

>  
>  libshared_cairo_la_CFLAGS =			\
>  	-DDATADIR='"$(datadir)"'		\

> diff --git a/clients/window.c b/clients/window.c
> index ced867e..071b059 100644
> --- a/clients/window.c
> +++ b/clients/window.c
> @@ -68,6 +68,7 @@ typedef void *EGLContext;
>  #include <wayland-client.h>
>  #include "shared/cairo-util.h"
>  #include "shared/helpers.h"
> +#include "shared/xalloc.h"
>  #include "shared/zalloc.h"
>  #include "xdg-shell-unstable-v5-client-protocol.h"
>  #include "text-cursor-position-client-protocol.h"
> @@ -4746,7 +4747,7 @@ window_create(struct display *display)
>  		window->xdg_surface =
>  			xdg_shell_get_xdg_surface(window->display->xdg_shell,
>  						  window->main_surface->surface);
> -		fail_on_null(window->xdg_surface);
> +		fail_on_null(window->xdg_surface, 1, __FILE__, __LINE__);

Why not zero as size to avoid a spurious size report?


> diff --git a/shared/xalloc.c b/shared/xalloc.c
> new file mode 100644
> index 0000000..4bf8a3e
> --- /dev/null
> +++ b/shared/xalloc.c
> @@ -0,0 +1,54 @@
> +/*
> + * Copyright © 2008 Kristian Høgsberg
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining
> + * a copy of this software and associated documentation files (the
> + * "Software"), to deal in the Software without restriction, including
> + * without limitation the rights to use, copy, modify, merge, publish,
> + * distribute, sublicense, and/or sell copies of the Software, and to
> + * permit persons to whom the Software is furnished to do so, subject to
> + * the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the
> + * next paragraph) shall be included in all copies or substantial
> + * portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> + * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
> + * NONINFRINGEMENT.  IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS
> + * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
> + * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
> + * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
> + * SOFTWARE.
> + */
> +
> +#include "config.h"
> +
> +#include <errno.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +
> +#include "xalloc.h"
> +
> +void *
> +fail_on_null(void *p, size_t size, char *file, int32_t line)
> +{
> +	if (p == NULL) {
> +		fprintf(stderr, "[%s] ", program_invocation_short_name);
> +		if (file)
> +			fprintf(stderr, "%s:%d: ", file, line);
> +		fprintf(stderr, "out of memory");
> +		if (size)
> +			fprintf(stderr, " (%zd)", size);
> +		fprintf(stderr, "\n");
> +		exit(EXIT_FAILURE);
> +	}
> +
> +	return p;
> +}
> +
> +
> +#ifdef  __cplusplus
> +}
> +#endif
> diff --git a/shared/xalloc.h b/shared/xalloc.h
> new file mode 100644
> index 0000000..74a9b6c
> --- /dev/null
> +++ b/shared/xalloc.h
> @@ -0,0 +1,70 @@
> +/*
> + * Copyright © 2008 Kristian Høgsberg
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining
> + * a copy of this software and associated documentation files (the
> + * "Software"), to deal in the Software without restriction, including
> + * without limitation the rights to use, copy, modify, merge, publish,
> + * distribute, sublicense, and/or sell copies of the Software, and to
> + * permit persons to whom the Software is furnished to do so, subject to
> + * the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the
> + * next paragraph) shall be included in all copies or substantial
> + * portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> + * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
> + * NONINFRINGEMENT.  IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS
> + * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
> + * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
> + * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
> + * SOFTWARE.
> + */
> +
> +#ifndef WESTON_XALLOC_H
> +#define WESTON_XALLOC_H
> +
> +#ifdef  __cplusplus
> +extern "C" {
> +#endif
> +
> +#include <stdlib.h>
> +#include <string.h>
> +
> +#include "zalloc.h"
> +
> +void *
> +fail_on_null(void *p, size_t size, char *file, int32_t line);
> +
> +static inline void *
> +xmalloc(size_t s)
> +{
> +        return fail_on_null(malloc(s), s, NULL, 0);
> +}
> +
> +static inline void *
> +xzalloc(size_t s)
> +{
> +	return fail_on_null(zalloc(s), s, NULL, 0);
> +}
> +
> +static inline char *
> +xstrdup(const char *s)
> +{
> +	return fail_on_null(strdup(s), 0, NULL, 0);
> +}
> +
> +static inline void *
> +xrealloc(char *p, size_t s)
> +{
> +        return fail_on_null(realloc(p, s), s, NULL, 0);
> +}
> +
> +
> +#ifdef  __cplusplus
> +}
> +#endif
> +
> +#endif /* WESTON_XALLOC_H */


Thanks,
pq
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 811 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/wayland-devel/attachments/20160316/c21c8a0a/attachment-0001.sig>


More information about the wayland-devel mailing list