[PATCH] V2: load_image always prints a message on failure if filename is not empty

[Pekka Paalanen]

On Mon, 11 Aug 2014 14:28:38 -0700
Bill Spitzak <spitzak at gmail.com> wrote:

> Small change to remove the trailing whitespace error.
> 
> It was rather inconsistent before. This may help users figure out why
> backgrounds and icons don't show up. A better api where the error can
> be queried might be nice, but this seems sufficient for current Weston use.
> ---
>  clients/image.c       |    1 -
>  shared/image-loader.c |   11 +++++++++--
>  2 files changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/clients/image.c b/clients/image.c
> index 573117c..aee8112 100644
> --- a/clients/image.c
> +++ b/clients/image.c
> @@ -373,7 +373,6 @@ image_create(struct display *display, const char *filename,
>  	image->image = load_cairo_surface(filename);
>  
>  	if (!image->image) {
> -		fprintf(stderr, "could not find the image %s!\n", b);

This is an unrelated change, and needs to be in a different patch.

>  		free(image->filename);
>  		free(image);
>  		return NULL;
> diff --git a/shared/image-loader.c b/shared/image-loader.c
> index 35dadd3..f04fb48 100644
> --- a/shared/image-loader.c
> +++ b/shared/image-loader.c
> @@ -23,6 +23,7 @@
>  
>  #include "config.h"
>  
> +#include <errno.h>
>  #include <stdlib.h>
>  #include <stdio.h>
>  #include <string.h>
> @@ -374,12 +375,15 @@ load_image(const char *filename)
>  	FILE *fp;
>  	unsigned int i;
>  
> +	if (! filename || ! *filename) return NULL;

Extra spaces, needs new line for 'return'.

> +
>  	fp = fopen(filename, "rb");
> -	if (fp == NULL)
> -		return NULL;
> +	if (fp == NULL) goto ERROR;

Needs new line for 'goto'.

>  
>  	if (fread(header, sizeof header, 1, fp) != 1) {
>  		fclose(fp);
> +	ERROR:

Jump into a block? Looks nasty, usually we do this like

	return ...;

foofoo:
	fclose();

foobar:
	fprintf(...);
	return NULL;
}

> +		fprintf(stderr, "%s: %s\n", filename, strerror(errno));
>  		return NULL;
>  	}
>  
> @@ -399,6 +403,9 @@ load_image(const char *filename)
>  			"0x%02x 0x%02x 0x%02x 0x%02x\n",
>  			filename, header[0], header[1], header[2], header[3]);
>  		image = NULL;
> +	} else if (!image) {
> +		/* load probably printed something, but just in case */
> + 		fprintf(stderr, "error reading image file %s!\n", filename);
>  	}
>  
>  	return image;


Thanks,
pq