Nico Huber has submitted this change. ( https://review.coreboot.org/c/coreboot/+/84341?usp=email )
Change subject: lib/jpeg: return string (not int) error messages ......................................................................
lib/jpeg: return string (not int) error messages
Change-Id: I465a6eebc2a41ca9a618b1e86dee015cea40800b Signed-off-by: Nigel Tao nigeltao@golang.org Reviewed-on: https://review.coreboot.org/c/coreboot/+/84341 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Nico Huber nico.h@gmx.de --- M src/lib/bootsplash.c M src/lib/jpeg.c M src/lib/jpeg.h 3 files changed, 30 insertions(+), 33 deletions(-)
Approvals: Nico Huber: Looks good to me, approved build bot (Jenkins): Verified
diff --git a/src/lib/bootsplash.c b/src/lib/bootsplash.c index 38d0ce7..2ec0052 100644 --- a/src/lib/bootsplash.c +++ b/src/lib/bootsplash.c @@ -28,8 +28,9 @@ }
unsigned int image_width, image_height; - if (jpeg_fetch_size(jpeg, filesize, &image_width, &image_height) != 0) { - printk(BIOS_ERR, "Could not parse bootsplash.jpg\n"); + const char *err = jpeg_fetch_size(jpeg, filesize, &image_width, &image_height); + if (err != NULL) { + printk(BIOS_ERR, "Could not parse bootsplash.jpg: %s\n", err); return; }
@@ -45,12 +46,11 @@ framebuffer += (yres - image_height) / 2 * bytes_per_line + (xres - image_width) / 2 * (fb_resolution / 8);
- int ret = jpeg_decode(jpeg, filesize, framebuffer, image_width, image_height, - bytes_per_line, fb_resolution); + err = jpeg_decode(jpeg, filesize, framebuffer, image_width, image_height, + bytes_per_line, fb_resolution); cbfs_unmap(jpeg); - if (ret != 0) { - printk(BIOS_ERR, "Bootsplash could not be decoded. jpeg_decode returned %d.\n", - ret); + if (err != NULL) { + printk(BIOS_ERR, "Could not decode bootsplash.jpg: %s\n", err); return; } printk(BIOS_INFO, "Bootsplash loaded\n"); diff --git a/src/lib/jpeg.c b/src/lib/jpeg.c index 617ab0b..d084c1a 100644 --- a/src/lib/jpeg.c +++ b/src/lib/jpeg.c @@ -19,38 +19,38 @@ /* ~16K is big enough to move this off the stack */ static wuffs_jpeg__decoder dec;
-int jpeg_fetch_size(unsigned char *filedata, size_t filesize, unsigned int *width, - unsigned int *height) +const char *jpeg_fetch_size(unsigned char *filedata, size_t filesize, unsigned int *width, + unsigned int *height) { if (!width || !height) { - return JPEG_DECODE_FAILED; + return "invalid arg"; }
wuffs_base__status status = wuffs_jpeg__decoder__initialize( &dec, sizeof(dec), WUFFS_VERSION, WUFFS_INITIALIZE__DEFAULT_OPTIONS); if (status.repr) { - return JPEG_DECODE_FAILED; + return status.repr; }
wuffs_base__image_config imgcfg; wuffs_base__io_buffer src = wuffs_base__ptr_u8__reader(filedata, filesize, true); status = wuffs_jpeg__decoder__decode_image_config(&dec, &imgcfg, &src); if (status.repr) { - return JPEG_DECODE_FAILED; + return status.repr; }
*width = wuffs_base__pixel_config__width(&imgcfg.pixcfg); *height = wuffs_base__pixel_config__height(&imgcfg.pixcfg);
- return 0; + return NULL; }
-int jpeg_decode(unsigned char *filedata, size_t filesize, unsigned char *pic, - unsigned int width, unsigned int height, unsigned int bytes_per_line, - unsigned int depth) +const char *jpeg_decode(unsigned char *filedata, size_t filesize, unsigned char *pic, + unsigned int width, unsigned int height, unsigned int bytes_per_line, + unsigned int depth) { if (!filedata || !pic) { - return JPEG_DECODE_FAILED; + return "invalid arg"; } /* Relatively arbitrary limit that shouldn't hurt anybody. * 300M (10k*10k*3bytes/pixel) is already larger than our heap, so @@ -59,7 +59,7 @@ * calculations in this function. */ if ((width > 10000) || (height > 10000)) { - return JPEG_DECODE_FAILED; + return "invalid arg"; }
uint32_t pixfmt; @@ -74,13 +74,13 @@ pixfmt = WUFFS_BASE__PIXEL_FORMAT__BGRA_NONPREMUL; break; default: - return JPEG_DECODE_FAILED; + return "invalid arg"; }
wuffs_base__status status = wuffs_jpeg__decoder__initialize( &dec, sizeof(dec), WUFFS_VERSION, WUFFS_INITIALIZE__DEFAULT_OPTIONS); if (status.repr) { - return JPEG_DECODE_FAILED; + return status.repr; }
/* Opting in to lower quality means that we can pass an empty slice as the @@ -105,7 +105,7 @@ wuffs_base__io_buffer src = wuffs_base__ptr_u8__reader(filedata, filesize, true); status = wuffs_jpeg__decoder__decode_image_config(&dec, &imgcfg, &src); if (status.repr) { - return JPEG_DECODE_FAILED; + return status.repr; }
wuffs_base__pixel_config pixcfg; @@ -117,15 +117,11 @@ wuffs_base__make_table_u8(pic, width * (depth / 8), height, bytes_per_line), wuffs_base__empty_slice_u8()); if (status.repr) { - return JPEG_DECODE_FAILED; + return status.repr; }
status = wuffs_jpeg__decoder__decode_frame(&dec, &pixbuf, &src, WUFFS_BASE__PIXEL_BLEND__SRC, wuffs_base__empty_slice_u8(), NULL); - if (status.repr) { - return JPEG_DECODE_FAILED; - } - - return 0; + return status.repr; } diff --git a/src/lib/jpeg.h b/src/lib/jpeg.h index d2e9e5f..c5179c8 100644 --- a/src/lib/jpeg.h +++ b/src/lib/jpeg.h @@ -5,12 +5,13 @@
#include <stdlib.h>
-#define JPEG_DECODE_FAILED 1 +/* These functions return NULL on success and a short error message on + * failure. Callers should not free the returned pointer. */
-int jpeg_fetch_size(unsigned char *filedata, size_t filesize, unsigned int *width, - unsigned int *height); -int jpeg_decode(unsigned char *filedata, size_t filesize, unsigned char *framebuffer, - unsigned int width, unsigned int height, unsigned int bytes_per_line, - unsigned int depth); +const char *jpeg_fetch_size(unsigned char *filedata, size_t filesize, unsigned int *width, + unsigned int *height); +const char *jpeg_decode(unsigned char *filedata, size_t filesize, unsigned char *framebuffer, + unsigned int width, unsigned int height, unsigned int bytes_per_line, + unsigned int depth);
#endif
-
Nico Huber (Code Review)