BMP loader: Major makeover - numerous bug fixes and feature enhancements

The test images at https://entropymine.com/jason/bmpsuite were most
helpful.

Several features have NOT been implemented, including
- Non-square resolution
- "Bottom up" format
- PNG compression
- JPEG compression
- Huffman 1D (OS/2) compression
- 2 bpp (OS/2)
- BA file type (bitmap array - OS/2)

1 files changed, 619 insertions, 589 deletions

diff --git a/src/modules/loaders/loader_bmp.c b/src/modules/loaders/loader_bmp.c
index f6a8888..0193cb9 100644
--- a/src/modules/loaders/loader_bmp.c
+++ b/src/modules/loaders/loader_bmp.c
@@ -2,7 +2,6 @@
  * Based off of Peter Alm's BMP loader from xmms, with additions from
  * imlib's old BMP loader
  */
-
 /*
  * 21.3.2006 - Changes made by Petr Kobalicek
  * - Simplify and make secure RLE encoding
@@ -11,19 +10,75 @@
 #include "loader_common.h"
 #include <sys/stat.h>
 
-typedef struct tagRGBQUAD {
+#define DEBUG 0
+#if DEBUG
+#define D(fmt...) fprintf(stdout, "BMP loader: " fmt)
+#else
+#define D(fmt...)
+#endif
+#define Dx(fmt...)
+
+/* The BITMAPFILEHEADER (size 14) */
+typedef struct {
+   DATA8               header[2];
+   DATA8               size[4];
+   DATA8               rsvd1[2];
+   DATA8               rsvd2[2];
+   DATA8               offs[4];
+} bfh_t;
+
+/* The BITMAPINFOHEADER */
+typedef union {
+   DATA32              header_size;
+   struct {
+      /* BITMAPCOREHEADER (size 12) */
+      DATA32              header_size;
+      DATA16              width;
+      DATA16              height;
+      DATA16              planes;
+      DATA16              bpp;
+   } bch;
+   struct {
+      /* BITMAPINFOHEADER (size 40) */
+      DATA32              header_size;
+      DATA32              width;
+      DATA32              height;
+      DATA16              planes;
+      DATA16              bpp;
+      DATA32              compression;
+      DATA32              size;
+      DATA32              res_hor;
+      DATA32              res_ver;
+      DATA32              colors;
+      DATA32              colors_important;
+      /* BITMAPV3INFOHEADER (size 56) */
+      DATA32              mask_r;
+      DATA32              mask_g;
+      DATA32              mask_b;
+      DATA32              mask_a;
+   } bih;
+   char                bytes[124];
+} bih_t;
+
+typedef struct {
    unsigned char       rgbBlue;
    unsigned char       rgbGreen;
    unsigned char       rgbRed;
    unsigned char       rgbReserved;
 } RGBQUAD;
 
-#define BI_RGB       0
-#define BI_RLE8      1
-#define BI_RLE4      2
-#define BI_BITFIELDS 3
+/* Compression methods */
+#define BI_RGB                  0
+#define BI_RLE8                 1
+#define BI_RLE4                 2
+#define BI_BITFIELDS            3
+#define BI_JPEG                 4       /* Unsupported */
+#define BI_PNG                  5       /* Unsupported */
+#define BI_ALPHABITFIELDS       6
+#define BI_CMYK                11       /* Unsupported */
+#define BI_CMYKRLE8            12       /* Unsupported */
+#define BI_CMYKRLE4            13       /* Unsupported */
 
-/* 21.3.3006 - Use enumeration for RLE encoding. This makes it more readable */
 enum {
    RLE_NEXT = 0,                /* Next line */
    RLE_END = 1,                 /* End of RLE encoding */
@@ -31,30 +86,6 @@ enum {
 };
 
 static int
-ReadleShort(FILE * file, unsigned short *ret)
-{
-   unsigned char       b[2];
-
-   if (fread(b, sizeof(unsigned char), 2, file) != 2)
-      return 0;
-
-   *ret = (b[1] << 8) | b[0];
-   return 1;
-}
-
-static int
-ReadleLong(FILE * file, unsigned long *ret)
-{
-   unsigned char       b[4];
-
-   if (fread(b, sizeof(unsigned char), 4, file) != 4)
-      return 0;
-
-   *ret = (b[3] << 24) | (b[2] << 16) | (b[1] << 8) | b[0];
-   return 1;
-}
-
-static int
 WriteleByte(FILE * file, unsigned char val)
 {
    int                 rc;
@@ -109,114 +140,141 @@ load(ImlibImage * im, ImlibProgressFunction progress,
    FILE               *f;
    char                pper = 0;
    int                 pl = 0;
-   char                type[2];
-   unsigned long       size, offset, headSize, comp, imgsize, j, k, l;
-   unsigned short      tmpShort, planes, bitcount, ncols, skip;
-   unsigned char       byte = 0, g, b, r;
-   unsigned long       i, w, h;
-   unsigned short      x, y;
-   DATA32             *ptr;
-   unsigned char      *buffer_ptr, *buffer, *buffer_end;
+   unsigned int        offset;
+   unsigned int        size, comp, imgsize;
+   unsigned int        bitcount, ncols, skip;
+   unsigned char       a, r, g, b;
+   unsigned char       byte = 0, byte1, byte2;
+   unsigned int        i, k;
+   int                 w, h, x, y, j, l;
+   DATA32             *ptr, pixel;
+   unsigned char      *buffer_ptr, *buffer, *buffer_end, *buffer_end_safe;
    RGBQUAD             rgbQuads[256];
-   unsigned long       rmask = 0xff, gmask = 0xff, bmask = 0xff;
-   unsigned long       rshift = 0, gshift = 0, bshift = 0;
-   unsigned long       rleftshift = 0, gleftshift = 0, bleftshift = 0;
-
-   /*
-    * 21.3.2006:
-    * Added these two variables for RLE.
-    */
-   unsigned char       byte1, byte2;
+   DATA32              argbCmap[256];
+   unsigned int        amask, rmask, gmask, bmask;
+   int                 ashift1, rshift1, gshift1, bshift1;
+   int                 ashift2, rshift2, gshift2, bshift2;
+   bih_t               bih;
 
    f = fopen(im->real_file, "rb");
    if (!f)
       return 0;
 
-   /* header */
+   buffer = NULL;
+
+   /* Load header */
    {
       struct stat         statbuf;
+      bfh_t               bfh;
+
+      if (fstat(fileno(f), &statbuf) < 0)
+         goto quit_err;
 
-      if (stat(im->real_file, &statbuf) == -1)
-        {
-           fclose(f);
-           return 0;
-        }
       size = statbuf.st_size;
+      if (size != statbuf.st_size)
+         goto quit_err;
 
-      if (fread(type, 1, 2, f) != 2)
-        {
-           fclose(f);
-           return 0;
-        }
-      if (strncmp(type, "BM", 2))
-        {
-           fclose(f);
-           return 0;
-        }
+      if (fread(&bfh, sizeof(bfh), 1, f) != 1)
+         goto quit_err;
+
+      if (bfh.header[0] != 'B' || bfh.header[1] != 'M')
+         goto quit_err;
+
+#define WORD_LE_32(p8) ((p8[3] << 24) | (p8[2] << 16) | (p8[1] << 8) | p8[0])
+      offset = WORD_LE_32(bfh.offs);
 
-      offset = 0;
-      headSize = 0;
-      fseek(f, 8, SEEK_CUR);
-      ReadleLong(f, &offset);
-      ReadleLong(f, &headSize);
       if (offset >= size)
-        {
-           fclose(f);
-           return 0;
-        }
+         goto quit_err;
+
+      memset(&bih, 0, sizeof(bih));
+      if (fread(&bih, 4, 1, f) != 1)
+         goto quit_err;
+
+      SWAP_LE_32_INPLACE(bih.header_size);
+
+      D("fsize=%u, hsize=%u, header: fsize=%u offs=%u\n",
+        size, bih.header_size, WORD_LE_32(bfh.size), offset);
+
+      if (bih.header_size < 12 || bih.header_size > sizeof(bih))
+         goto quit_err;
+
+      if (fread(&bih.header_size + 1, bih.header_size - 4, 1, f) != 1)
+         goto quit_err;
 
       w = h = 0;
       bitcount = 0;
       comp = BI_RGB;
-      if (headSize == 12)
+      amask = rmask = gmask = bmask = 0;
+      ashift1 = rshift1 = gshift1 = bshift1 = 0;
+      ashift2 = rshift2 = gshift2 = bshift2 = 1;
+
+      UNSET_FLAG(im->flags, F_HAS_ALPHA);
+
+      if (bih.header_size == 12)
         {
-           tmpShort = 0;
-           ReadleShort(f, &tmpShort);
-           w = tmpShort;
-           ReadleShort(f, &tmpShort);
-           h = tmpShort;
-           ReadleShort(f, &planes);
-           ReadleShort(f, &bitcount);
-           imgsize = size - offset;
+           w = SWAP_LE_16(bih.bch.width);
+           h = SWAP_LE_16(bih.bch.height);
+//         planes = SWAP_LE_16(bih.bch.planes);
+           bitcount = SWAP_LE_16(bih.bch.bpp);
         }
-      else if (headSize == 40)
+      else if (bih.header_size >= 16)
         {
-           ReadleLong(f, &w);
-           ReadleLong(f, &h);
-           ReadleShort(f, &planes);
-           ReadleShort(f, &bitcount);
-           ReadleLong(f, &comp);
-           ReadleLong(f, &imgsize);
-           imgsize = size - offset;
-
-           fseek(f, 16, SEEK_CUR);
+           w = SWAP_LE_32(bih.bih.width);
+           h = SWAP_LE_32(bih.bih.height);
+//         planes = SWAP_LE_16(bih.bih.planes);
+           bitcount = SWAP_LE_16(bih.bih.bpp);
+           comp = SWAP_LE_32(bih.bih.compression);
+           imgsize = SWAP_LE_32(bih.bih.size);  /* We don't use this */
+
+           if (bih.header_size >= 40 &&
+               (comp == BI_BITFIELDS || comp == BI_ALPHABITFIELDS))
+             {
+                if (bih.header_size == 40)
+                  {
+                     ncols = (comp == BI_ALPHABITFIELDS) ? 4 : 3;
+                     if (fread(&bih.bih.mask_r, 4, ncols, f) != ncols)
+                        goto quit_err;
+                  }
+                rmask = SWAP_LE_32(bih.bih.mask_r);
+                gmask = SWAP_LE_32(bih.bih.mask_g);
+                bmask = SWAP_LE_32(bih.bih.mask_b);
+                amask = SWAP_LE_32(bih.bih.mask_a);
+                if (amask)
+                   SET_FLAG(im->flags, F_HAS_ALPHA);
+             }
         }
       else
         {
-           fclose(f);
-           return 0;
+           goto quit_err;
         }
 
+      imgsize = size - offset;
+      D("w=%3d h=%3d bitcount=%d comp=%d imgsize=%d\n",
+        w, h, bitcount, comp, imgsize);
+
+      /* "Bottom-up" images are loaded but not properly flipped */
+      h = abs(h);
+
       if (!IMAGE_DIMENSIONS_OK(w, h))
-        {
-           fclose(f);
-           return 0;
-        }
+         goto quit_err;
 
-      if (bitcount < 16)
+      switch (bitcount)
         {
-           ncols = (offset - headSize - 14);
-           if (headSize == 12)
+        default:
+           goto quit_err;
+
+        case 1:
+        case 4:
+        case 8:
+           ncols = (offset - bih.header_size - 14);
+           if (bih.header_size == 12)
              {
                 ncols /= 3;
                 if (ncols > 256)
                    ncols = 256;
                 for (i = 0; i < ncols; i++)
                    if (fread(&rgbQuads[i], 3, 1, f) != 1)
-                     {
-                        fclose(f);
-                        return 0;
-                     }
+                      goto quit_err;
              }
            else
              {
@@ -224,437 +282,168 @@ load(ImlibImage * im, ImlibProgressFunction progress,
                 if (ncols > 256)
                    ncols = 256;
                 if (fread(rgbQuads, 4, ncols, f) != ncols)
-                  {
-                     fclose(f);
-                     return 0;
-                  }
+                   goto quit_err;
              }
-        }
-      else if (bitcount == 16 || bitcount == 32)
-        {
-           if (comp == BI_BITFIELDS)
+           for (i = 0; i < ncols; i++)
+              argbCmap[i] =
+                 PIXEL_ARGB(0xff, rgbQuads[i].rgbRed, rgbQuads[i].rgbGreen,
+                            rgbQuads[i].rgbBlue);
+           D("ncols=%d\n", ncols);
+           break;
+
+        case 24:
+           break;
+
+        case 16:
+        case 32:
+           if (comp == BI_BITFIELDS || comp == BI_ALPHABITFIELDS)
              {
-                int                 bit;
+                unsigned int        bit, bithi;
+                unsigned int        mask;
 
-                ReadleLong(f, &rmask);
-                ReadleLong(f, &gmask);
-                ReadleLong(f, &bmask);
+                D("mask   ARGB: %08x %08x %08x %08x\n",
+                  amask, rmask, gmask, bmask);
                 if (bitcount == 16)
                   {
+                     amask &= 0xffffU;
                      rmask &= 0xffffU;
                      gmask &= 0xffffU;
                      bmask &= 0xffffU;
                   }
-                if (rmask == 0 || gmask == 0 || bmask == 0)
-                  {
-                     fclose(f);
-                     return 0;
-                  }
-                for (bit = bitcount - 1; bit >= 0; bit--)
-                  {
-                     if (bmask & (1 << bit))
-                        bshift = bit;
-                     if (gmask & (1 << bit))
-                        gshift = bit;
-                     if (rmask & (1 << bit))
-                        rshift = bit;
-                  }
-                while (((((0xffffL & bmask) >> bshift) << bleftshift) & 0x80) ==
-                       0)
-                  {
-                     bleftshift++;
-                  }
-                while (((((0xffffL & gmask) >> gshift) << gleftshift) & 0x80) ==
-                       0)
+                if (rmask == 0 && gmask == 0 && bmask == 0)
+                   goto quit_err;
+                for (bit = 0; bit < bitcount; bit++)
                   {
-                     gleftshift++;
-                  }
-                while (((((0xffffL & rmask) >> rshift) << rleftshift) & 0x80) ==
-                       0)
-                  {
-                     rleftshift++;
+                     /* Find LSB bit positions */
+                     bithi = bitcount - bit - 1;
+                     mask = 1 << bithi;
+                     if (amask & mask)
+                        ashift1 = bithi;
+                     if (bmask & mask)
+                        bshift1 = bithi;
+                     if (gmask & mask)
+                        gshift1 = bithi;
+                     if (rmask & mask)
+                        rshift1 = bithi;
+
+                     /* Find MSB bit positions */
+                     mask = 1 << bit;
+                     if (amask & mask)
+                        ashift2 = bit;
+                     if (rmask & mask)
+                        rshift2 = bit;
+                     if (gmask & mask)
+                        gshift2 = bit;
+                     if (bmask & mask)
+                        bshift2 = bit;
                   }
+
+                /* Calculate shift2s as bits in mask */
+                ashift2 -= ashift1 - 1;
+                rshift2 -= rshift1 - 1;
+                gshift2 -= gshift1 - 1;
+                bshift2 -= bshift1 - 1;
              }
            else if (bitcount == 16)
              {
                 rmask = 0x7C00;
                 gmask = 0x03E0;
                 bmask = 0x001F;
-                rshift = 10;
-                gshift = 5;
-                bshift = 0;
-                rleftshift = gleftshift = bleftshift = 3;
+                rshift1 = 10;
+                gshift1 = 5;
+                bshift1 = 0;
+                rshift2 = gshift2 = bshift2 = 5;
              }
            else if (bitcount == 32)
              {
+                amask = 0xFF000000;
                 rmask = 0x00FF0000;
                 gmask = 0x0000FF00;
                 bmask = 0x000000FF;
-                rshift = 16;
-                gshift = 8;
-                bshift = 0;
+                ashift1 = 24;
+                rshift1 = 16;
+                gshift1 = 8;
+                bshift1 = 0;
+                ashift2 = rshift2 = gshift2 = bshift2 = 8;
              }
+
+           /* Calculate shift2s as scale factor */
+           ashift2 = ashift2 > 0 ? (1 << ashift2) - 1 : 1;
+           rshift2 = rshift2 > 0 ? (1 << rshift2) - 1 : 1;
+           gshift2 = gshift2 > 0 ? (1 << gshift2) - 1 : 1;
+           bshift2 = bshift2 > 0 ? (1 << bshift2) - 1 : 1;
+
+#define SCALE(c, x) ((((x & c##mask)>> (c##shift1 - 0)) * 255) / c##shift2)
+
+           D("mask   ARGB: %08x %08x %08x %08x\n", amask, rmask, gmask, bmask);
+           D("shift1 ARGB: %8d %8d %8d %8d\n",
+             ashift1, rshift1, gshift1, bshift1);
+           D("shift2 ARGB: %8d %8d %8d %8d\n",
+             ashift2, rshift2, gshift2, bshift2);
+           D("check  ARGB: %08x %08x %08x %08x\n",
+             SCALE(a, amask), SCALE(r, rmask),
+             SCALE(g, gmask), SCALE(b, bmask));
+           break;
         }
 
       im->w = w;
       im->h = h;
-      UNSET_FLAG(im->flags, F_HAS_ALPHA);
    }
-   if (im->loader || immediate_load || progress)
-     {
-        fseek(f, offset, SEEK_SET);
-        buffer = malloc(imgsize);
-        if (!buffer)
-          {
-             fclose(f);
-             return 0;
-          }
-        if (!__imlib_AllocateData(im, w, h))
-          {
-             im->w = 0;
-             free(buffer);
-             fclose(f);
-             return 0;
-          }
 
-        if (fread(buffer, imgsize, 1, f) != 1)
-          {
-             __imlib_FreeData(im);
-             free(buffer);
-             fclose(f);
-             return 0;
-          }
+   if (!(im->loader || immediate_load || progress))
+     {
         fclose(f);
-        buffer_ptr = buffer;
-        buffer_end = buffer + imgsize;
-
-        ptr = im->data + ((h - 1) * w);
+        return 1;
+     }
 
-        if (bitcount == 1)
-          {
-             if (comp == BI_RGB)
-               {
-                  skip = ((((w + 31) / 32) * 32) - w) / 8;
-                  for (y = 0; y < h; y++)
-                    {
-                       for (x = 0; x < w && buffer_ptr < buffer_end; x++)
-                         {
-                            if ((x & 7) == 0)
-                               byte = *(buffer_ptr++);
-                            k = (byte >> 7) & 1;
-                            *ptr++ = 0xff000000 |
-                               (rgbQuads[k].rgbRed << 16) |
-                               (rgbQuads[k].rgbGreen << 8) |
-                               rgbQuads[k].rgbBlue;
-                            byte <<= 1;
-                         }
-                       buffer_ptr += skip;
-                       ptr -= w * 2;
-                       if (progress)
-                         {
-                            char                per;
-                            int                 ll;
+   /* Load data */
 
-                            per = (char)((100 * y) / im->h);
-                            if (((per - pper) >= progress_granularity) ||
-                                (y == (im->h - 1)))
-                              {
-                                 ll = y - pl;
-                                 if (!progress
-                                     (im, per, 0, im->h - y - 1, im->w,
-                                      im->h - y + ll))
-                                   {
-                                      free(buffer);
-                                      return 2;
-                                   }
-                                 pper = per;
-                                 pl = y;
-                              }
-                         }
-                    }
-               }
-          }
+   fseek(f, offset, SEEK_SET);
 
-        /*
-         * 21.3.2006
-         * Bug fixes and optimization:
-         * 
-         * RLE encoding is dangerous and can be used by attackers by creating special files.
-         * We has 'buffer_ptr' and 'buffer_end' variables and buffer_end points to first 
-         * unaccessible byte in buffer.
-         * - If we use 'byte = *(buffer_ptr++) in main loop we must check if 
-         *   'buffer_ptr != buffer_end', because special or incomplete bmp file can generate
-         *   segfault (I was writing it, because in RLE we need to read depending count of
-         *   bytes that depends on requester operation).
-         *   SOLUTION: Don't read one byte, read two bytes and check.
-         * - If RLE teels us than single color length will be larger than allowed, we can
-         *   stop, because bitmap is corrupted or crawled.
-         *   SOLUTION: Check for length ('l' variable in RLE) and break loop if it's invalid
-         *   IMPROVEMENTS: We can stop checking if 'x' is out of rangle, because it never be.
-         * - In RLE4 use one bigger loop that fills two pixels. This is faster and cleaner.
-         *   If one pixel remains (the tail), do it on end of the loop.
-         * - If we will check x and y (new line and skipping), we can't go outsize imlib
-         *   image buffer.
-         */
-
-        if (bitcount == 4)
-          {
-             if (comp == BI_RLE4)
-               {
-                  /*
-                   * 21.3.2006: This is better than using 'if buffer_ptr + 1 < buffer_end'
-                   */
-                  unsigned char      *buffer_end_minus_1 = buffer_end - 1;
+   buffer = malloc(imgsize);
+   if (!buffer)
+      goto quit_err;
 
-                  x = 0;
-                  y = 0;
+   if (!__imlib_AllocateData(im, w, h))
+      goto quit_err;
 
-                  for (i = 0; i < imgsize && buffer_ptr < buffer_end_minus_1;
-                       i++)
-                    {
-                       byte1 = buffer_ptr[0];
-                       byte2 = buffer_ptr[1];
-                       buffer_ptr += 2;
-                       if (byte1)
-                         {
-                            DATA32              t1, t2;
+   if (fread(buffer, imgsize, 1, f) != 1)
+     {
+        __imlib_FreeData(im);
+        goto quit_err;
+     }
+   fclose(f);
 
-                            l = byte1;
-                            /* Check for invalid length */
-                            if (l + x > w)
-                               goto _bail;
-
-                            t1 = 0xff000000 |
-                               (rgbQuads[byte2 >> 4].rgbRed << 16) |
-                               (rgbQuads[byte2 >> 4].rgbGreen << 8) |
-                               (rgbQuads[byte2 >> 4].rgbBlue);
-                            t2 = 0xff000000 |
-                               (rgbQuads[byte2 & 0xF].rgbRed << 16) |
-                               (rgbQuads[byte2 & 0xF].rgbGreen << 8) |
-                               (rgbQuads[byte2 & 0xF].rgbBlue);
-                            for (j = l / 2; j; j--)
-                              {
-                                 ptr[0] = t1;
-                                 ptr[1] = t2;
-                                 ptr += 2;
-                              }
-                            /* tail */
-                            if (l & 1)
-                               *ptr++ = t1;
-                            x += l;
-                         }
-                       else
-                         {
-                            switch (byte2)
-                              {
-                              case RLE_NEXT:
-                                 x = 0;
-                                 if (++y >= h)
-                                    goto _bail;
-                                 ptr = im->data + (h - y - 1) * w;
-                                 break;
-                              case RLE_END:
-                                 goto _bail;
-                              case RLE_MOVE:
-                                 /* Need to read two bytes */
-                                 if (buffer_ptr >= buffer_end_minus_1)
-                                    goto _bail;
-                                 x += buffer_ptr[0];
-                                 y += buffer_ptr[1];
-                                 buffer_ptr += 2;
-                                 /* Check for correct coordinates */
-                                 if (x >= w)
-                                    goto _bail;
-                                 if (y >= h)
-                                    goto _bail;
-                                 ptr = im->data + (h - y - 1) * w + x;
-                                 break;
-                              default:
-                                 l = byte2;
-                                 /* Check for invalid length and valid buffer size */
-                                 if (l + x > w)
-                                    goto _bail;
-                                 if (buffer_ptr + (l >> 1) + (l & 1) >
-                                     buffer_end)
-                                    goto _bail;
-
-                                 for (j = l / 2; j; j--)
-                                   {
-                                      byte = *buffer_ptr++;
-                                      ptr[0] =
-                                         0xff000000 |
-                                         (rgbQuads[byte >> 4].rgbRed << 16) |
-                                         (rgbQuads[byte >> 4].rgbGreen << 8) |
-                                         (rgbQuads[byte >> 4].rgbBlue);
-                                      ptr[1] =
-                                         0xff000000 |
-                                         (rgbQuads[byte & 0xF].rgbRed << 16) |
-                                         (rgbQuads[byte & 0xF].rgbGreen << 8) |
-                                         (rgbQuads[byte & 0xF].rgbBlue);
-                                      ptr += 2;
-                                   }
-                                 if (l & 1)
-                                   {
-                                      byte = *buffer_ptr++;
-                                      *ptr++ =
-                                         0xff000000 |
-                                         (rgbQuads[byte >> 4].rgbRed << 16) |
-                                         (rgbQuads[byte >> 4].rgbGreen << 8) |
-                                         (rgbQuads[byte >> 4].rgbBlue);
-                                   }
-                                 x += l;
-
-                                 if ((l & 3) == 1)
-                                    buffer_ptr += 2;
-                                 else if ((l & 3) == 2)
-                                    buffer_ptr++;
-                                 break;
-                              }
-                         }
-                       if (progress)
-                         {
-                            char                per;
-                            int                 ll;
+   buffer_ptr = buffer;
+   buffer_end = buffer + imgsize;
 
-                            per = (char)((100 * y) / im->h);
-                            if (((per - pper) >= progress_granularity) ||
-                                (y == (im->h - 1)))
-                              {
-                                 ll = y - pl;
-                                 if (!progress
-                                     (im, per, 0, im->h - y - 1, im->w,
-                                      im->h - y + ll))
-                                   {
-                                      free(buffer);
-                                      return 2;
-                                   }
-                                 pper = per;
-                                 pl = y;
-                              }
-                         }
+   ptr = im->data + ((h - 1) * w);
 
-                    }
-               }
-             else if (comp == BI_RGB)
-               {
-                  skip = ((((w + 7) / 8) * 8) - w) / 2;
-                  for (y = 0; y < h; y++)
-                    {
-                       for (x = 0; x < w && buffer_ptr < buffer_end; x++)
-                         {
-                            if ((x & 1) == 0)
-                               byte = *(buffer_ptr++);
-                            k = (byte & 0xF0) >> 4;
-                            *ptr++ = 0xff000000 |
-                               (rgbQuads[k].rgbRed << 16) |
-                               (rgbQuads[k].rgbGreen << 8) |
-                               rgbQuads[k].rgbBlue;
-                            byte <<= 4;
-                         }
-                       buffer_ptr += skip;
-                       ptr -= w * 2;
-                       if (progress)
-                         {
-                            char                per;
-                            int                 ll;
+   switch (bitcount)
+     {
+     default:                  /* It should not be possible to go here */
+        goto quit_err2;
 
-                            per = (char)((100 * y) / im->h);
-                            if (((per - pper) >= progress_granularity) ||
-                                (y == (im->h - 1)))
-                              {
-                                 ll = y - pl;
-                                 if (!progress
-                                     (im, per, 0, im->h - y - 1, im->w,
-                                      im->h - y + ll))
-                                   {
-                                      free(buffer);
-                                      return 2;
-                                   }
-                                 pper = per;
-                                 pl = y;
-                              }
-                         }
-                    }
-               }
-          }
-        if (bitcount == 8)
+     case 1:
+        switch (comp)
           {
-             if (comp == BI_RLE8)
+          default:
+             goto quit_err2;
+
+          case BI_RGB:
+             skip = ((((w + 31) / 32) * 32) - w) / 8;
+             for (y = 0; y < h; y++)
                {
-                  /*
-                   * 21.3.2006: This is better than using 'if buffer_ptr + 1 < buffer_end'
-                   */
-                  unsigned char      *buffer_end_minus_1 = buffer_end - 1;
-
-                  x = 0;
-                  y = 0;
-                  for (i = 0; i < imgsize && buffer_ptr < buffer_end_minus_1;
-                       i++)
+                  for (x = 0; x < w && buffer_ptr < buffer_end; x++)
                     {
-                       byte1 = buffer_ptr[0];
-                       byte2 = buffer_ptr[1];
-                       buffer_ptr += 2;
-                       if (byte1)
-                         {
-                            DATA32              pix =
-                               0xff000000 | (rgbQuads[byte2].rgbRed << 16) |
-                               (rgbQuads[byte2].rgbGreen << 8) |
-                               (rgbQuads[byte2].rgbBlue);
-                            l = byte1;
-                            if (x + l > w)
-                               goto _bail;
-                            for (j = l; j; j--)
-                               *ptr++ = pix;
-                            x += l;
-                         }
-                       else
-                         {
-                            switch (byte2)
-                              {
-                              case RLE_NEXT:
-                                 x = 0;
-                                 if (++y >= h)
-                                    goto _bail;
-                                 ptr = im->data + ((h - y - 1) * w) + x;
-                                 break;
-                              case RLE_END:
-                                 goto _bail;
-                              case RLE_MOVE:
-                                 /* Need to read two bytes */
-                                 if (buffer_ptr >= buffer_end_minus_1)
-                                    goto _bail;
-                                 x += buffer_ptr[0];
-                                 y += buffer_ptr[1];
-                                 buffer_ptr += 2;
-                                 /* Check for correct coordinates */
-                                 if (x >= w)
-                                    goto _bail;
-                                 if (y >= h)
-                                    goto _bail;
-                                 ptr = im->data + ((h - y - 1) * w) + x;
-                                 break;
-                              default:
-                                 l = byte2;
-                                 if (x + l > w)
-                                    goto _bail;
-                                 if (buffer_ptr + l > buffer_end)
-                                    goto _bail;
-                                 for (j = 0; j < l; j++)
-                                   {
-                                      byte = *(buffer_ptr++);
-
-                                      *ptr++ = 0xff000000 |
-                                         (rgbQuads[byte].rgbRed << 16) |
-                                         (rgbQuads[byte].rgbGreen << 8) |
-                                         rgbQuads[byte].rgbBlue;
-                                   }
-                                 x += l;
-                                 if (l & 1)
-                                    buffer_ptr++;
-                                 break;
-                              }
-                         }
+                       if ((x & 7) == 0)
+                          byte = *buffer_ptr++;
+                       k = (byte >> 7) & 1;
+                       *ptr++ = argbCmap[k];
+                       byte <<= 1;
                     }
+                  buffer_ptr += skip;
+                  ptr -= w * 2;
                   if (progress)
                     {
                        char                per;
@@ -677,76 +466,100 @@ load(ImlibImage * im, ImlibProgressFunction progress,
                          }
                     }
                }
-             else if (comp == BI_RGB)
+             break;
+          }
+        break;
+
+     case 4:
+        switch (comp)
+          {
+          default:
+             goto quit_err2;
+
+          case BI_RLE4:
+             buffer_end_safe = buffer_end - 1;
+
+             x = 0;
+             y = 0;
+
+             for (i = 0; i < imgsize && buffer_ptr < buffer_end_safe; i++)
                {
-                  skip = (((w + 3) / 4) * 4) - w;
-                  for (y = 0; y < h; y++)
+                  byte1 = buffer_ptr[0];
+                  byte2 = buffer_ptr[1];
+                  buffer_ptr += 2;
+                  Dx("%3d %3d: %02x %02x\n", x, y, byte1, byte2);
+                  if (byte1)
                     {
-                       for (x = 0; x < w && buffer_ptr < buffer_end; x++)
+                       DATA32              t1, t2;
+
+                       l = byte1;
+                       /* Check for invalid length */
+                       if (l + x > w)
+                          goto bail_bc4;
+                       Dx("%3d %3d:   n=%d:  %d %d\n", x, y, byte1, byte2 >> 4,
+                          byte2 & 0xf);
+
+                       t1 = argbCmap[byte2 >> 4];
+                       t2 = argbCmap[byte2 & 0xF];
+                       for (j = 0; j < l; j++)
                          {
-                            byte = *(buffer_ptr++);
-                            *ptr++ = 0xff000000 |
-                               (rgbQuads[byte].rgbRed << 16) |
-                               (rgbQuads[byte].rgbGreen << 8) |
-                               rgbQuads[byte].rgbBlue;
+                            *ptr++ = t1;
+                            if (++j < l)
+                               *ptr++ = t2;
                          }
-                       ptr -= w * 2;
-                       buffer_ptr += skip;
-                       if (progress)
+                       x += l;
+                    }
+                  else
+                    {
+                       switch (byte2)
                          {
-                            char                per;
-                            int                 ll;
+                         case RLE_NEXT:
+                            x = 0;
+                            if (++y >= h)
+                               goto bail_bc4;
+                            ptr = im->data + (h - y - 1) * w;
+                            break;
+                         case RLE_END:
+                            goto bail_bc4;
+                         case RLE_MOVE:
+                            /* Need to read two bytes */
+                            if (buffer_ptr >= buffer_end_safe)
+                               goto bail_bc4;
+                            x += buffer_ptr[0];
+                            y += buffer_ptr[1];
+                            buffer_ptr += 2;
+                            /* Check for correct coordinates */
+                            if (x >= w)
+                               goto bail_bc4;
+                            if (y >= h)
+                               goto bail_bc4;
+                            ptr = im->data + (h - y - 1) * w + x;
+                            break;
+                         default:
+                            l = byte2;
+                            /* Check for invalid length and valid buffer size */
+                            if (l + x > w)
+                               goto bail_bc4;
+                            if (buffer_ptr + (l >> 1) + (l & 1) > buffer_end)
+                               goto bail_bc4;
 
-                            per = (char)((100 * y) / im->h);
-                            if (((per - pper) >= progress_granularity) ||
-                                (y == (im->h - 1)))
+                            for (j = 0; j < l; j++)
                               {
-                                 ll = y - pl;
-                                 if (!progress
-                                     (im, per, 0, im->h - y - 1, im->w,
-                                      im->h - y + ll))
-                                   {
-                                      free(buffer);
-                                      return 2;
-                                   }
-                                 pper = per;
-                                 pl = y;
+                                 byte = *buffer_ptr++;
+                                 Dx("%3d %3d:   %d/%d: %2d %2d\n",
+                                    x, y, j, l, byte >> 4, byte & 0xf);
+                                 *ptr++ = argbCmap[byte >> 4];
+                                 if (++j < l)
+                                    *ptr++ = argbCmap[byte & 0xF];
                               }
-                         }
-                    }
-               }
-
-          }
-        else if (bitcount == 16)
-          {
-             /* 21.3.2006 - Need to check for buffer_ptr + 1 < buffer_end */
-             unsigned char      *buffer_end_minus_1 = buffer_end - 1;
+                            x += l;
 
-             skip = (((w * 16 + 31) / 32) * 4) - (w * 2);
-             for (y = 0; y < h; y++)
-               {
-                  for (x = 0; x < w && buffer_ptr < buffer_end_minus_1; x++)
-                    {
-                       /*
-                        * THIS WAS OLD CODE 
-                        *
-                        * r = ((unsigned short)(*buffer_ptr) & rmask) >> rshift;
-                        * g = ((unsigned short)(*buffer_ptr) & gmask) >> gshift;
-                        * b = ((unsigned short)(*(buffer_ptr++)) & bmask) >>
-                        *   bshift;
-                        * *ptr++ = 0xff000000 | (r << 16) | (g << 8) | b;
-                        */
-                       unsigned short      pix = *(unsigned short *)buffer_ptr;
-
-                       *ptr++ =
-                          0xff000000 |
-                          ((((pix & rmask) >> rshift) << rleftshift) << 16) |
-                          ((((pix & gmask) >> gshift) << gleftshift) << 8) |
-                          ((((pix & bmask) >> bshift) << bleftshift));
-                       buffer_ptr += 2;
+                            /* Pad to even number of palette bytes */
+                            buffer_ptr += ((l + 1) / 2) & 1;
+                            break;
+                         }
                     }
-                  ptr -= w * 2;
-                  buffer_ptr += skip;
+                bail_bc4:
                   if (progress)
                     {
                        char                per;
@@ -769,24 +582,22 @@ load(ImlibImage * im, ImlibProgressFunction progress,
                          }
                     }
                }
-          }
-        else if (bitcount == 24)
-          {
-             /* 21.3.2006 - Fix: need to check for buffer_ptr + 2 < buffer_end */
-             unsigned char      *buffer_end_minus_2 = buffer_end - 2;
+             break;
 
-             skip = (4 - ((w * 3) % 4)) & 3;
+          case BI_RGB:
+             skip = ((((w + 7) / 8) * 8) - w) / 2;
              for (y = 0; y < h; y++)
                {
-                  for (x = 0; x < w && buffer_ptr < buffer_end_minus_2; x++)
+                  for (x = 0; x < w && buffer_ptr < buffer_end; x++)
                     {
-                       b = *(buffer_ptr++);
-                       g = *(buffer_ptr++);
-                       r = *(buffer_ptr++);
-                       *ptr++ = 0xff000000 | (r << 16) | (g << 8) | b;
+                       if ((x & 1) == 0)
+                          byte = *buffer_ptr++;
+                       k = (byte & 0xF0) >> 4;
+                       *ptr++ = argbCmap[k];
+                       byte <<= 4;
                     }
-                  ptr -= w * 2;
                   buffer_ptr += skip;
+                  ptr -= w * 2;
                   if (progress)
                     {
                        char                per;
@@ -809,38 +620,118 @@ load(ImlibImage * im, ImlibProgressFunction progress,
                          }
                     }
                }
+             break;
           }
-        else if (bitcount == 32)
+        break;
+
+     case 8:
+        switch (comp)
           {
-             /* 21.3.2006 - Need to check buffer_ptr + 3 < buffer_end */
-             unsigned char      *buffer_end_minus_3 = buffer_end_minus_3;
+          default:
+             goto quit_err2;
+
+          case BI_RLE8:
+             buffer_end_safe = buffer_end - 1;
+
+             x = 0;
+             y = 0;
+             for (i = 0; i < imgsize && buffer_ptr < buffer_end_safe; i++)
+               {
+                  byte1 = buffer_ptr[0];
+                  byte2 = buffer_ptr[1];
+                  buffer_ptr += 2;
+                  if (byte1)
+                    {
+                       pixel = argbCmap[byte2];
+                       l = byte1;
+                       if (x + l > w)
+                          goto bail_bc8;
+                       for (j = l; j; j--)
+                          *ptr++ = pixel;
+                       x += l;
+                    }
+                  else
+                    {
+                       switch (byte2)
+                         {
+                         case RLE_NEXT:
+                            x = 0;
+                            if (++y >= h)
+                               goto bail_bc8;
+                            ptr = im->data + ((h - y - 1) * w) + x;
+                            break;
+                         case RLE_END:
+                            goto bail_bc8;
+                         case RLE_MOVE:
+                            /* Need to read two bytes */
+                            if (buffer_ptr >= buffer_end_safe)
+                               goto bail_bc8;
+                            x += buffer_ptr[0];
+                            y += buffer_ptr[1];
+                            buffer_ptr += 2;
+                            /* Check for correct coordinates */
+                            if (x >= w)
+                               goto bail_bc8;
+                            if (y >= h)
+                               goto bail_bc8;
+                            ptr = im->data + ((h - y - 1) * w) + x;
+                            break;
+                         default:
+                            l = byte2;
+                            if (x + l > w)
+                               goto bail_bc8;
+                            if (buffer_ptr + l > buffer_end)
+                               goto bail_bc8;
+                            for (j = 0; j < l; j++)
+                              {
+                                 byte = *buffer_ptr++;
+                                 *ptr++ = argbCmap[byte];
+                              }
+                            x += l;
+                            if (l & 1)
+                               buffer_ptr++;
+                            break;
+                         }
+                    }
+               }
+           bail_bc8:
+             if (progress)
+               {
+                  char                per;
+                  int                 ll;
+
+                  per = (char)((100 * y) / im->h);
+#if 0
+                  /* Always call progress() at least once */
+                  if (((per - pper) >= progress_granularity) ||
+                      (y == (im->h - 1)))
+#endif
+                    {
+                       ll = y - pl;
+                       if (!progress
+                           (im, per, 0, im->h - y - 1, im->w, im->h - y + ll))
+                         {
+                            free(buffer);
+                            return 2;
+                         }
+                       pper = per;
+                       pl = y;
+                    }
+               }
+             break;
 
-             skip = (((w * 32 + 31) / 32) * 4) - (w * 4);
+          case BI_RGB:
+             skip = (((w + 3) / 4) * 4) - w;
              for (y = 0; y < h; y++)
                {
-                  for (x = 0; x < w && buffer_ptr < buffer_end_minus_3; x++)
+                  for (x = 0; x < w && buffer_ptr < buffer_end; x++)
                     {
-                       /*
-                        * THIS WAS OLD CODE: I don't understand it and it's invalid.
-                        *
-                        * r = ((unsigned long)(*buffer_ptr) & rmask) >> rshift;
-                        * g = ((unsigned long)(*buffer_ptr) & gmask) >> gshift;
-                        * b = ((unsigned long)(*buffer_ptr) & bmask) >> bshift;
-                        * *ptr++ = 0xff000000 | (r << 16) | (g << 8) | b;
-                        * r = *(buffer_ptr++);
-                        * r = *(buffer_ptr++);
-                        */
-
-                       /* TODO: What about alpha channel...Is used? */
-                       DATA32              pix = *(unsigned int *)buffer_ptr;
-
-                       *ptr++ = 0xff000000 | (((pix & rmask) >> rshift) << 16) |
-                          (((pix & gmask) >> gshift) << 8) |
-                          (((pix & bmask) >> bshift));
-                       buffer_ptr += 4;
+                       byte = *buffer_ptr++;
+                       *ptr++ = argbCmap[byte];
                     }
                   ptr -= w * 2;
                   buffer_ptr += skip;
+
                   if (progress)
                     {
                        char                per;
@@ -863,11 +754,150 @@ load(ImlibImage * im, ImlibProgressFunction progress,
                          }
                     }
                }
+             break;
+          }
+        break;
+
+     case 16:
+        buffer_end_safe = buffer_end - 1;
+
+        skip = (((w * 16 + 31) / 32) * 4) - (w * 2);
+        for (y = 0; y < h; y++)
+          {
+             for (x = 0; x < w && buffer_ptr < buffer_end_safe; x++)
+               {
+                  pixel = *(unsigned short *)buffer_ptr;
+
+                  if (im->flags & F_HAS_ALPHA)
+                     a = SCALE(a, pixel);
+                  else
+                     a = 0xff;
+                  r = SCALE(r, pixel);
+                  g = SCALE(g, pixel);
+                  b = SCALE(b, pixel);
+                  *ptr++ = PIXEL_ARGB(a, r, g, b);
+                  buffer_ptr += 2;
+               }
+             ptr -= w * 2;
+             buffer_ptr += skip;
+
+             if (progress)
+               {
+                  char                per;
+                  int                 ll;
+
+                  per = (char)((100 * y) / im->h);
+                  if (((per - pper) >= progress_granularity) ||
+                      (y == (im->h - 1)))
+                    {
+                       ll = y - pl;
+                       if (!progress
+                           (im, per, 0, im->h - y - 1, im->w, im->h - y + ll))
+                         {
+                            free(buffer);
+                            return 2;
+                         }
+                       pper = per;
+                       pl = y;
+                    }
+               }
+          }
+        break;
+
+     case 24:
+        buffer_end_safe = buffer_end - 2;
+
+        skip = (4 - ((w * 3) % 4)) & 3;
+        for (y = 0; y < h; y++)
+          {
+             for (x = 0; x < w && buffer_ptr < buffer_end_safe; x++)
+               {
+                  b = *buffer_ptr++;
+                  g = *buffer_ptr++;
+                  r = *buffer_ptr++;
+                  *ptr++ = PIXEL_ARGB(0xff, r, g, b);
+               }
+             ptr -= w * 2;
+             buffer_ptr += skip;
+
+             if (progress)
+               {
+                  char                per;
+                  int                 ll;
+
+                  per = (char)((100 * y) / im->h);
+                  if (((per - pper) >= progress_granularity) ||
+                      (y == (im->h - 1)))
+                    {
+                       ll = y - pl;
+                       if (!progress
+                           (im, per, 0, im->h - y - 1, im->w, im->h - y + ll))
+                         {
+                            free(buffer);
+                            return 2;
+                         }
+                       pper = per;
+                       pl = y;
+                    }
+               }
+          }
+        break;
+
+     case 32:
+        buffer_end_safe = buffer_end - 3;
+
+        skip = (((w * 32 + 31) / 32) * 4) - (w * 4);
+        for (y = 0; y < h; y++)
+          {
+             for (x = 0; x < w && buffer_ptr < buffer_end_safe; x++)
+               {
+                  pixel = *(unsigned int *)buffer_ptr;
+
+                  if (im->flags & F_HAS_ALPHA)
+                     a = SCALE(a, pixel);
+                  else
+                     a = 0xff;
+                  r = SCALE(r, pixel);
+                  g = SCALE(g, pixel);
+                  b = SCALE(b, pixel);
+                  *ptr++ = PIXEL_ARGB(a, r, g, b);
+                  buffer_ptr += 4;
+               }
+             ptr -= w * 2;
+             buffer_ptr += skip;
+
+             if (progress)
+               {
+                  char                per;
+                  int                 ll;
+
+                  per = (char)((100 * y) / im->h);
+                  if (((per - pper) >= progress_granularity) ||
+                      (y == (im->h - 1)))
+                    {
+                       ll = y - pl;
+                       if (!progress
+                           (im, per, 0, im->h - y - 1, im->w, im->h - y + ll))
+                         {
+                            free(buffer);
+                            return 2;
+                         }
+                       pper = per;
+                       pl = y;
+                    }
+               }
           }
-      _bail:
-        free(buffer);
+        break;
      }
+
+   free(buffer);
    return 1;
+
+ quit_err:
+   fclose(f);
+ quit_err2:
+   free(buffer);
+   return 0;
 }
 
 char