[PATCH] wayland util: Handle malloc failure in wl_array_copy()

[Kristian Høgsberg]

On Thu, Jul 19, 2012 at 03:45:57AM +0200, Martin Minarik wrote:
> If the malloc in wl_array_add() fails, we are memcpy-ing to bad memory.
> This can happen only when copying array to smaller array.
> ---
>  src/wayland-util.c |   11 ++++++++---
>  src/wayland-util.h |    2 +-
>  tests/array-test.c |    8 ++++++--
>  3 files changed, 15 insertions(+), 6 deletions(-)
> 
> diff --git a/src/wayland-util.c b/src/wayland-util.c
> index a8c03ad..4e02f95 100644
> --- a/src/wayland-util.c
> +++ b/src/wayland-util.c
> @@ -135,12 +135,17 @@ wl_array_add(struct wl_array *array, size_t size)
>  	return p;
>  }
>  
> -WL_EXPORT void
> +WL_EXPORT int
>  wl_array_copy(struct wl_array *array, struct wl_array *source)
>  {
> -	array->size = 0;
> -	wl_array_add(array, source->size);
> +	if (source->size > array->size) {
> +		if (NULL == wl_array_add(array, source->size - array->size))
> +			return -1;
> +	} else {
> +		array->size = source->size;
> +	}

wl_array_add will grow the allocated area if necessary, so just
setting size to 0 and adding source->size works fine.  To check for
out-of-memory, just do

	if (wl_array_add(array, source->size) == 0)
		return -1;

instead.

>  	memcpy(array->data, source->data, source->size);
> +	return 0;
>  }
>  
>  union map_entry {
> diff --git a/src/wayland-util.h b/src/wayland-util.h
> index b588505..f54077e 100644
> --- a/src/wayland-util.h
> +++ b/src/wayland-util.h
> @@ -165,7 +165,7 @@ struct wl_array {
>  void wl_array_init(struct wl_array *array);
>  void wl_array_release(struct wl_array *array);
>  void *wl_array_add(struct wl_array *array, size_t size);
> -void wl_array_copy(struct wl_array *array, struct wl_array *source);
> +int wl_array_copy(struct wl_array *array, struct wl_array *source);
>  
>  typedef int32_t wl_fixed_t;
>  
> diff --git a/tests/array-test.c b/tests/array-test.c
> index 7639878..ff5bb8c 100644
> --- a/tests/array-test.c
> +++ b/tests/array-test.c
> @@ -60,7 +60,9 @@ TEST(array_add)
>  
>  	/* add some data */
>  	for (i = 0; i < iterations; i++) {
> -		struct mydata* ptr = wl_array_add(&array, datasize);
> +		struct mydata* ptr = NULL;
> +		while (ptr == NULL)
> +			ptr = wl_array_add(&array, datasize);
>  		assert((i + 1) * datasize == array.size);

If we want to test our malloc failure paths, we should inject malloc
errors using our malloc wrapper in test/test-runner.c, not by forcing
out-of-memory.  The typical approach here is to run the test once to
see how many mallocs there are, then run it once for each malloc,
failing malloc i in iteration i.

>  		ptr->a = i * 3;
> @@ -94,7 +96,9 @@ TEST(array_copy)
>  
>  	/* add some data */
>  	for (i = 0; i < iterations; i++) {
> -		int *p = wl_array_add(&source, sizeof(int));
> +		int *p = NULL;
> +		while (p == NULL)
> +			p = wl_array_add(&source, sizeof(int));
>  		*p = i * 2 + i;
>  	}
>  
> -- 
> 1.7.0.4
> 
> _______________________________________________
> wayland-devel mailing list
> wayland-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel