[PATCH weston] tests: Remove the redundant NULL checks

Bryce Harrington bryce at osg.samsung.com
Thu Sep 3 13:17:59 PDT 2015


On Fri, Sep 04, 2015 at 12:17:18AM +0530, Seedo Eldho Paul wrote:
> xzalloc is guaranteed to return a non-NULL value.
> 
> Signed-off-by: Seedo Eldho Paul <seedoeldhopaul at gmail.com>
> ---
>  tests/internal-screenshot-test.c | 13 -------------
>  1 file changed, 13 deletions(-)

Good point.  The error handling code will never actually get executed.
 
> diff --git a/tests/internal-screenshot-test.c b/tests/internal-screenshot-test.c
> index e72a695..a74ae7a 100644
> --- a/tests/internal-screenshot-test.c
> +++ b/tests/internal-screenshot-test.c
> @@ -93,11 +93,6 @@ load_surface_from_png(const char *fname)
>  
>  	/* Disguise the cairo surface in a weston test surface */
>  	reference = xzalloc(sizeof *reference);
> -	if (reference == NULL) {
> -		perror("xzalloc reference");
> -		cairo_surface_destroy(reference_cairo_surface);
> -		return NULL;
> -	}

This will skip the surface destruction.  I suppose who cares, we're
exiting anyway, but the xzalloc could be moved up prior to the surface
creation, and make it a non-issue.

>  	reference->width = cairo_image_surface_get_width(reference_cairo_surface);
>  	reference->height = cairo_image_surface_get_height(reference_cairo_surface);
>  	stride = cairo_image_surface_get_stride(reference_cairo_surface);
> @@ -115,12 +110,6 @@ load_surface_from_png(const char *fname)
>  	/* Allocate new buffer for our weston reference, and copy the data from
>  	   the cairo surface so we can destroy it */
>  	reference->data = xzalloc(source_data_size);
> -	if (reference->data == NULL) {
> -		perror("xzalloc reference data");
> -		cairo_surface_destroy(reference_cairo_surface);
> -		free(reference);
> -		return NULL;
> -	}

Can't do that here though, since we depend on the surface to calculate
source_data_size.  Also freeing reference in this case is a good idea.
Maybe this is a case where we should plain zalloc instead?

>  	memcpy(reference->data,
>  	       cairo_image_surface_get_data(reference_cairo_surface),
>  	       source_data_size);
> @@ -144,8 +133,6 @@ create_screenshot_surface(struct client *client)
>  {
>  	struct surface* screenshot;
>  	screenshot = xzalloc(sizeof *screenshot);
> -	if (screenshot == NULL)
> -		return NULL;
>  	screenshot->wl_buffer = create_shm_buffer(client,
>  						  client->output->width,
>  						  client->output->height,

Same as above, maybe should use zalloc instead.

Thanks for reviewing this.  Sorry if these comments seem nit-picky for a
just a test case...  The reason is that I plan to move a lot of this
code out of this test into the testing infrastructure, so having it
handle errors cleanly in this test case will be worthwhile in the longer
run.

Bryce

> -- 
> 1.9.1
> 
> _______________________________________________
> wayland-devel mailing list
> wayland-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel


More information about the wayland-devel mailing list