savers: Fix error returns

The savers did not return meaningful error codes in many cases.

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);
 

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);

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;
 }
 

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)

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);

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)

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;

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;

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;
 

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++)

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;

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);
+    if (fprintf(f, "#define %s_width %d\n", bname, im->w) <= 0)
-    fprintf(f, "#define %s_height %d\n", bname, im->h);
+        goto quit;
-    fprintf(f, "static unsigned char %s_bits[] = {\n", bname);
+    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 ? "," : "",
+        if (fprintf(f, " 0x%02x%s%s", bits, k < nval ? "," : "",
-                (k == nval) || ((k % 12) == 0) ? "\n" : "");
+                    (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;
 }
 

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