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

Dear Mike Frysinger,

In message 1228941641-27682-1-git-send-email-vapier@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