This is an automated email from the git hooks/post-receive script.
git pushed a commit to branch master
in repository legacy-imlib2.
View the commit online.
commit 15ab46d03ee5dada9f21ad8295e3286acd57c1d1
Author: Kim Woelders <k...@woelders.dk>
AuthorDate: Tue Jan 2 06:25:36 2024 +0100
savers: Fix error returns
The savers did not return meaningful error codes in many cases.
---
src/lib/image.c | 6 +++-
src/modules/loaders/loader_argb.c | 14 +++++++--
src/modules/loaders/loader_bmp.c | 15 +++++++---
src/modules/loaders/loader_ff.c | 4 +--
src/modules/loaders/loader_jpeg.c | 6 ++--
src/modules/loaders/loader_jxl.c | 2 +-
src/modules/loaders/loader_png.c | 10 +++----
src/modules/loaders/loader_pnm.c | 20 +++++++++----
src/modules/loaders/loader_tga.c | 11 ++++---
src/modules/loaders/loader_tiff.c | 6 ++--
src/modules/loaders/loader_webp.c | 2 +-
src/modules/loaders/loader_xbm.c | 22 +++++++++-----
test/test_save.cpp | 62 +++++++++++++++++++++++++++++++++++++++
13 files changed, 140 insertions(+), 40 deletions(-)
diff --git a/src/lib/image.c b/src/lib/image.c
index 13e64ed..d42195c 100644
--- a/src/lib/image.c
+++ b/src/lib/image.c
@@ -478,7 +478,7 @@ __imlib_LoadErrorToErrno(int loader_ret, int save)
case LOAD_OOM:
return ENOMEM;
case LOAD_BADFILE:
- return errno;
+ return errno ? errno : IMLIB_ERR_INTERNAL;
case LOAD_BADIMAGE:
return IMLIB_ERR_BAD_IMAGE;
case LOAD_BADFRAME:
@@ -913,7 +913,11 @@ __imlib_SaveImage(ImlibImage *im, const char *file, ImlibLoadArgs *ila)
loader_ret = l->module->save(im);
if (!ila->fp)
+ {
+ if (fflush(im->fi->fp) != 0)
+ loader_ret = LOAD_BADFILE; /* Use errno */
fclose(fp);
+ }
__imlib_ImageFileContextPop(im);
diff --git a/src/modules/loaders/loader_argb.c b/src/modules/loaders/loader_argb.c
index 55daffa..070ff45 100644
--- a/src/modules/loaders/loader_argb.c
+++ b/src/modules/loaders/loader_argb.c
@@ -112,14 +112,20 @@ _save(ImlibImage *im)
FILE *f = im->fi->fp;
const uint32_t *imdata;
int y, alpha = 0;
+ size_t nw;
#ifdef WORDS_BIGENDIAN
uint32_t *buf = (uint32_t *) malloc(im->w * 4);
+ if (!buf)
+ QUIT_WITH_RC(LOAD_OOM);
#endif
+ rc = LOAD_BADFILE;
+
alpha = !!im->has_alpha;
- fprintf(f, "ARGB %i %i %i\n", im->w, im->h, alpha);
+ if (fprintf(f, "ARGB %i %i %i\n", im->w, im->h, alpha) <= 0)
+ goto quit;
imdata = im->data;
for (y = 0; y < im->h; y++, imdata += im->w)
@@ -131,11 +137,13 @@ _save(ImlibImage *im)
memcpy(buf, imdata, im->w * 4);
for (x = 0; x < im->w; x++)
SWAP_LE_32_INPLACE(buf[x]);
- fwrite(buf, im->w, 4, f);
+ nw = fwrite(buf, 4, im->w, f);
}
#else
- fwrite(imdata, im->w, 4, f);
+ nw = fwrite(imdata, 4, im->w, f);
#endif
+ if (nw != (size_t)im->w)
+ goto quit;
if (im->lc && __imlib_LoadProgressRows(im, y, 1))
QUIT_WITH_RC(LOAD_BREAK);
diff --git a/src/modules/loaders/loader_bmp.c b/src/modules/loaders/loader_bmp.c
index e10a42b..0ca1798 100644
--- a/src/modules/loaders/loader_bmp.c
+++ b/src/modules/loaders/loader_bmp.c
@@ -107,7 +107,7 @@ enum {
};
static int
-WriteleByte(FILE *file, unsigned char val)
+_WriteleByte(FILE *file, unsigned char val)
{
int rc;
@@ -119,7 +119,7 @@ WriteleByte(FILE *file, unsigned char val)
}
static int
-WriteleShort(FILE *file, unsigned short val)
+_WriteleShort(FILE *file, unsigned short val)
{
int rc;
@@ -134,7 +134,7 @@ WriteleShort(FILE *file, unsigned short val)
}
static int
-WriteleLong(FILE *file, unsigned long val)
+_WriteleLong(FILE *file, unsigned long val)
{
int rc;
@@ -755,6 +755,10 @@ _load(ImlibImage *im, int load_data)
return rc;
}
+#define WriteleByte(file, val) if (!_WriteleByte(file, val)) goto quit
+#define WriteleShort(file, val) if (!_WriteleShort(file, val)) goto quit
+#define WriteleLong(file, val) if (!_WriteleLong(file, val)) goto quit
+
static int
_save(ImlibImage *im)
{
@@ -763,7 +767,7 @@ _save(ImlibImage *im)
int i, j, pad;
uint32_t pixel;
- rc = LOAD_SUCCESS;
+ rc = LOAD_BADFILE;
/* calculate number of bytes to pad on end of each row */
pad = (4 - ((im->w * 3) % 4)) & 0x03;
@@ -800,6 +804,9 @@ _save(ImlibImage *im)
WriteleByte(f, 0);
}
+ rc = LOAD_SUCCESS;
+
+ quit:
return rc;
}
diff --git a/src/modules/loaders/loader_ff.c b/src/modules/loaders/loader_ff.c
index 6537ba3..1d52acb 100644
--- a/src/modules/loaders/loader_ff.c
+++ b/src/modules/loaders/loader_ff.c
@@ -90,7 +90,7 @@ _save(ImlibImage *im)
uint16_t *row;
const uint8_t *imdata;
- rc = LOAD_FAIL;
+ rc = LOAD_BADFILE;
row = NULL;
/* write header */
@@ -108,7 +108,7 @@ _save(ImlibImage *im)
rowlen = im->w * (sizeof("RGBA") - 1);
row = malloc(rowlen * sizeof(uint16_t));
if (!row)
- goto quit;
+ QUIT_WITH_RC(LOAD_OOM);
imdata = (uint8_t *) im->data;
for (i = 0; i < (uint32_t) im->h; ++i, imdata += rowlen)
diff --git a/src/modules/loaders/loader_jpeg.c b/src/modules/loaders/loader_jpeg.c
index 6cd4d57..df177ca 100644
--- a/src/modules/loaders/loader_jpeg.c
+++ b/src/modules/loaders/loader_jpeg.c
@@ -268,14 +268,14 @@ _save(ImlibImage *im)
/* allocate a small buffer to convert image data */
buf = malloc(im->w * 3 * sizeof(uint8_t));
if (!buf)
- return LOAD_FAIL;
+ return LOAD_OOM;
- rc = LOAD_FAIL;
+ rc = LOAD_BADFILE;
/* set up error handling */
jcs.err = _jdata_init(&jdata);
if (sigsetjmp(jdata.setjmp_buffer, 1))
- QUIT_WITH_RC(LOAD_FAIL);
+ QUIT_WITH_RC(LOAD_BADFILE);
/* setup compress params */
jpeg_create_compress(&jcs);
diff --git a/src/modules/loaders/loader_jxl.c b/src/modules/loaders/loader_jxl.c
index c9f0d22..1bf133c 100644
--- a/src/modules/loaders/loader_jxl.c
+++ b/src/modules/loaders/loader_jxl.c
@@ -240,7 +240,7 @@ _save(ImlibImage *im)
JxlParallelRunner *runner = NULL;
#endif
- rc = LOAD_FAIL;
+ rc = LOAD_BADFILE;
enc = JxlEncoderCreate(NULL);
if (!enc)
diff --git a/src/modules/loaders/loader_png.c b/src/modules/loaders/loader_png.c
index 18a0088..ccc73d6 100644
--- a/src/modules/loaders/loader_png.c
+++ b/src/modules/loaders/loader_png.c
@@ -615,24 +615,24 @@ _save(ImlibImage *im)
{
row_ptr = malloc(im->w * 3 * sizeof(png_byte));
if (!row_ptr)
- return LOAD_FAIL;
+ return LOAD_OOM;
}
row_buf = row_ptr;
- rc = LOAD_FAIL;
+ rc = LOAD_BADFILE;
info_ptr = NULL;
png_ptr = png_create_write_struct(PNG_LIBPNG_VER_STRING, NULL,
user_error_fn, user_warning_fn);
if (!png_ptr)
- goto quit;
+ QUIT_WITH_RC(LOAD_OOM);
info_ptr = png_create_info_struct(png_ptr);
if (!info_ptr)
- goto quit;
+ QUIT_WITH_RC(LOAD_OOM);
if (setjmp(png_jmpbuf(png_ptr)))
- QUIT_WITH_RC(LOAD_FAIL);
+ QUIT_WITH_RC(LOAD_BADFILE);
/* check whether we should use interlacing */
interlace = PNG_INTERLACE_NONE;
diff --git a/src/modules/loaders/loader_pnm.c b/src/modules/loaders/loader_pnm.c
index 93401cf..71ec4c0 100644
--- a/src/modules/loaders/loader_pnm.c
+++ b/src/modules/loaders/loader_pnm.c
@@ -620,19 +620,21 @@ _save(ImlibImage *im)
const uint32_t *imdata;
int x, y;
- rc = LOAD_FAIL;
+ rc = LOAD_BADFILE;
/* allocate a small buffer to convert image data */
buf = malloc(im->w * 4 * sizeof(uint8_t));
if (!buf)
- goto quit;
+ QUIT_WITH_RC(LOAD_OOM);
imdata = im->data;
/* if the image has a useful alpha channel */
if (im->has_alpha)
{
- fprintf(f, fmt_rgba, im->w, im->h);
+ if (fprintf(f, fmt_rgba, im->w, im->h) <= 0)
+ goto quit;
+
for (y = 0; y < im->h; y++)
{
bptr = buf;
@@ -646,7 +648,9 @@ _save(ImlibImage *im)
bptr[3] = PIXEL_A(pixel);
bptr += 4;
}
- fwrite(buf, im->w * 4, 1, f);
+
+ if (fwrite(buf, 4, im->w, f) != (size_t)im->w)
+ goto quit;
if (im->lc && __imlib_LoadProgressRows(im, y, 1))
goto quit_progress;
@@ -654,7 +658,9 @@ _save(ImlibImage *im)
}
else
{
- fprintf(f, fmt_rgb, im->w, im->h);
+ if (fprintf(f, fmt_rgb, im->w, im->h) <= 0)
+ goto quit;
+
for (y = 0; y < im->h; y++)
{
bptr = buf;
@@ -667,7 +673,9 @@ _save(ImlibImage *im)
bptr[2] = PIXEL_B(pixel);
bptr += 3;
}
- fwrite(buf, im->w * 3, 1, f);
+
+ if (fwrite(buf, 3, im->w, f) != (size_t)im->w)
+ goto quit;
if (im->lc && __imlib_LoadProgressRows(im, y, 1))
goto quit_progress;
diff --git a/src/modules/loaders/loader_tga.c b/src/modules/loaders/loader_tga.c
index 19a19b2..c47e713 100644
--- a/src/modules/loaders/loader_tga.c
+++ b/src/modules/loaders/loader_tga.c
@@ -502,7 +502,7 @@ _save(ImlibImage *im)
int y;
tga_header header;
- rc = LOAD_FAIL;
+ rc = LOAD_BADFILE;
/* assemble the TGA header information */
@@ -533,7 +533,7 @@ _save(ImlibImage *im)
/* allocate a buffer to receive the BGRA-swapped pixel values */
buf = malloc(im->w * im->h * (im->has_alpha ? 4 : 3));
if (!buf)
- goto quit;
+ QUIT_WITH_RC(LOAD_OOM);
/* now we have to read from im->data into buf, swapping RGBA to BGRA */
imdata = im->data;
@@ -562,10 +562,13 @@ _save(ImlibImage *im)
}
/* write the header */
- fwrite(&header, sizeof(header), 1, f);
+ if (fwrite(&header, 1, sizeof(header), f) != sizeof(header))
+ goto quit;
/* write the image data */
- fwrite(buf, 1, im->w * im->h * (im->has_alpha ? 4 : 3), f);
+ if (fwrite(buf, (im->has_alpha ? 4 : 3), im->w * im->h, f) !=
+ (size_t)(im->w * im->h))
+ goto quit;
rc = LOAD_SUCCESS;
diff --git a/src/modules/loaders/loader_tiff.c b/src/modules/loaders/loader_tiff.c
index ae71ded..0ea7f7c 100644
--- a/src/modules/loaders/loader_tiff.c
+++ b/src/modules/loaders/loader_tiff.c
@@ -488,9 +488,9 @@ _save(ImlibImage *im)
tif = TIFFFdOpen(fileno(im->fi->fp), im->fi->name, "w");
if (!tif)
- return LOAD_FAIL;
+ return LOAD_BADFILE;
- rc = LOAD_FAIL;
+ rc = LOAD_BADFILE;
/* None of the TIFFSetFields are checked for errors, but since they */
/* shouldn't fail, this shouldn't be a problem */
@@ -559,7 +559,7 @@ _save(ImlibImage *im)
buf = _TIFFmalloc(TIFFScanlineSize(tif));
if (!buf)
- goto quit;
+ QUIT_WITH_RC(LOAD_OOM);
imdata = im->data;
for (y = 0; y < im->h; y++)
diff --git a/src/modules/loaders/loader_webp.c b/src/modules/loaders/loader_webp.c
index ecea187..5eb19af 100644
--- a/src/modules/loaders/loader_webp.c
+++ b/src/modules/loaders/loader_webp.c
@@ -137,7 +137,7 @@ _save(ImlibImage *im)
int lossless;
int free_pic = 0;
- rc = LOAD_FAIL;
+ rc = LOAD_BADFILE;
if (!WebPConfigInit(&conf) || !WebPPictureInit(&pic))
goto quit;
diff --git a/src/modules/loaders/loader_xbm.c b/src/modules/loaders/loader_xbm.c
index 3ca53c4..4eb8b8c 100644
--- a/src/modules/loaders/loader_xbm.c
+++ b/src/modules/loaders/loader_xbm.c
@@ -218,7 +218,7 @@ _save(ImlibImage *im)
int i, k, x, y, bits, nval, val;
const uint32_t *imdata;
- rc = LOAD_SUCCESS;
+ rc = LOAD_BADFILE;
name = im->fi->name;
if ((s = strrchr(name, '/')) != 0)
@@ -226,9 +226,12 @@ _save(ImlibImage *im)
bname = strndup(name, strcspn(name, "."));
- fprintf(f, "#define %s_width %d\n", bname, im->w);
- fprintf(f, "#define %s_height %d\n", bname, im->h);
- fprintf(f, "static unsigned char %s_bits[] = {\n", bname);
+ if (fprintf(f, "#define %s_width %d\n", bname, im->w) <= 0)
+ goto quit;
+ if (fprintf(f, "#define %s_height %d\n", bname, im->h) <= 0)
+ goto quit;
+ if (fprintf(f, "static unsigned char %s_bits[] = {\n", bname) <= 0)
+ goto quit;
free(bname);
@@ -251,12 +254,17 @@ _save(ImlibImage *im)
}
k++;
DL("x, y = %2d,%2d: %d/%d\n", x, y, k, nval);
- fprintf(f, " 0x%02x%s%s", bits, k < nval ? "," : "",
- (k == nval) || ((k % 12) == 0) ? "\n" : "");
+ if (fprintf(f, " 0x%02x%s%s", bits, k < nval ? "," : "",
+ (k == nval) || ((k % 12) == 0) ? "\n" : "") <= 0)
+ goto quit;
}
- fprintf(f, "};\n");
+ if (fprintf(f, "};\n") <= 0)
+ goto quit;
+
+ rc = LOAD_SUCCESS;
+ quit:
return rc;
}
diff --git a/test/test_save.cpp b/test/test_save.cpp
index 70908db..d6c55ba 100644
--- a/test/test_save.cpp
+++ b/test/test_save.cpp
@@ -333,3 +333,65 @@ TEST(SAVE, save_2b_defer)
test_save_2("icon-64.gif", "svg", immed, false);
test_save_2("icon-64.gif", "png", immed, true, 4016720483);
}
+
+static void
+test_save_3(const char *file, const char *dest, int err_new, int err_old)
+{
+ char filei[256];
+ char fileo[256];
+ unsigned int i;
+ const char *ext;
+ int err;
+ Imlib_Image im;
+ Imlib_Load_Error lerr;
+
+ snprintf(filei, sizeof(filei), "%s/%s", IMG_SRC, file);
+ D("Load '%s'\n", filei);
+ im = imlib_load_image(filei);
+ ASSERT_TRUE(im);
+
+ imlib_context_set_image(im);
+
+ for (i = 0; i < N_PFX; i++)
+ {
+ ext = exts[i].ext;
+
+ imlib_context_set_image(im);
+ imlib_image_set_format(ext);
+ snprintf(fileo, sizeof(fileo), "%s", dest);
+ pr_info("Save %s to '%s'", ext, fileo);
+
+ imlib_save_image_with_errno_return(fileo, &err);
+ EXPECT_EQ(err, err_new);
+
+ imlib_save_image_with_error_return(fileo, &lerr);
+ EXPECT_EQ(lerr, err_old);
+
+ imlib_save_image(fileo);
+ err = imlib_get_error();
+ ASSERT_EQ(err, err_new);
+ }
+
+ imlib_context_set_image(im);
+ imlib_free_image_and_decache();
+}
+
+TEST(SAVE, save_3_full)
+{
+ test_save_3("icon-64.png", "/dev/full",
+ ENOSPC, IMLIB_LOAD_ERROR_OUT_OF_DISK_SPACE);
+}
+
+#if 0
+TEST(SAVE, save_3_dir)
+{
+ test_save_3("icon-64.png", "/tmp",
+ EISDIR, IMLIB_LOAD_ERROR_FILE_IS_DIRECTORY);
+}
+
+TEST(SAVE, save_3_rdonly)
+{
+ test_save_3("icon-64.png", IMG_GEN "/rdonly",
+ EACCES, IMLIB_LOAD_ERROR_PERMISSION_DENIED_TO_WRITE);
+}
+#endif
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
