[U-Boot] [PATCH v3] easylogo: add optional gzip support

[Wolfgang Denk]

Dear Mike Frysinger,

In message <1228941641-27682-1-git-send-email-vapier at gentoo.org> you wrote:
> Some images can be quite large, so add an option to compress the image data
> with gzip in the U-Boot image.  Then at runtime, the board can decompress it
> with the normal zlib functions.
...
> +		gz = popen (gzcmd, "w");
> +		if (!gz) {
> +			perror ("\nerror: popen() failed");
> + gzerr:
> +			free (gzfilename);
> +			free (gzcmd);
> +			return -1;
> +		}
> +		if (fwrite (image->data, image->size, 1, gz) != 1) {
> +			perror ("\nerror: writing data to gzip failed");
> +			goto gzerr;
> +		}
> +		if (pclose (gz)) {
> +			perror ("\nerror: gzip process failed");
> +			goto gzerr;
> +		}
> +
> +		gz = fopen (gzfilename, "r");
> +		if (!gz) {
> +			perror ("\nerror: open() on gzip data failed");
> +			goto gzerr;
> +		}
> +		if (stat (gzfilename, &st)) {
> +			perror ("\nerror: stat() on gzip file failed");
> +			goto gzerr;
> +		}
> +		compressed = xmalloc (st.st_size);
> +		if (fread (compressed, st.st_size, 1, gz) != 1) {
> +			perror ("\nerror: reading gzip data failed");
> +			goto gzerr;
> +		}
> +		fclose (gz);
> +
> +		unlink (gzfilename);
> +
> +		dataptr = compressed;
> +		count = st.st_size;
> +		fprintf (file, "#define EASYLOGO_ENABLE_GZIP %i\n\n", count);
> +		if (use_gzip & 0x2)
> +			fprintf (file, "static unsigned char EASYLOGO_DECOMP_BUFFER[%i];\n\n", image->size);
> +
> +		free (gzfilename);
> +		free (gzcmd);

Umm... no. Jumping back to a random place upward in the code just to
get an error exit is ugly, and you duplicate code (the 2 x free()
calls).

How about something like this:

	char *errstr = NULL;

	...
	gz = popen (gzcmd, "w");
	if (!gz) {
		errstr = "\nerror: popen() failed";
		goto out;
	}
	if (fwrite (image->data, image->size, 1, gz) != 1) {
		errstr = "\nerror: writing data to gzip failed";
		goto out;
	}
	if (pclose (gz)) {
		errstr = "\nerror: gzip process failed";
		goto out;
	}

	gz = fopen (gzfilename, "r");
	if (!gz) {
		errstr = "\nerror: open() on gzip data failed";
		goto out;
	}
	if (stat (gzfilename, &st)) {
		errstr = "\nerror: stat() on gzip file failed";
		goto out;
	}
	compressed = xmalloc (st.st_size);
	if (fread (compressed, st.st_size, 1, gz) != 1) {
		errstr = "\nerror: reading gzip data failed";
		goto out;
	}
	fclose (gz);

	unlink (gzfilename);

	dataptr = compressed;
	count = st.st_size;
	fprintf (file, "#define EASYLOGO_ENABLE_GZIP %i\n\n", count);
	if (use_gzip & 0x2)
		fprintf (file, "static unsigned char EASYLOGO_DECOMP_BUFFER[%i];\n\n", image->size);

out:
	if (errstr)
		perror (errstr);
	free (gzfilename);
	free (gzcmd);
	if (errstr)
		return -1;
	...

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
The price of curiosity is a terminal experience.
                         - Terry Pratchett, _The Dark Side of the Sun_