Merge pull request #2654 from python-pillow/storage-cleanup

Storage cleanup

Tests/test_image.py

@@ -7,6 +7,25 @@ import sys
 
 class TestImage(PillowTestCase):
 
+    def test_image_modes_success(self):
+        for mode in [
+            '1', 'P', 'PA',
+            'L', 'LA', 'La',
+            'F', 'I', 'I;16', 'I;16L', 'I;16B', 'I;16N',
+            'RGB', 'RGBX', 'RGBA', 'RGBa',
+            'CMYK', 'YCbCr', 'LAB', 'HSV',
+        ]:
+            Image.new(mode, (1, 1))
+
+    def test_image_modes_fail(self):
+        for mode in [
+            '', 'bad', 'very very long',
+            'BGR;15', 'BGR;16', 'BGR;24', 'BGR;32'
+        ]:
+            with self.assertRaises(ValueError) as e:
+                Image.new(mode, (1, 1));
+            self.assertEqual(str(e.exception), 'unrecognized image mode')
+
     def test_sanity(self):
 
         im = Image.new("L", (100, 100))

libImaging/Imaging.h

@@ -163,7 +163,6 @@ extern Imaging ImagingNew2(const char* mode, Imaging imOut, Imaging imIn);
 extern void    ImagingDelete(Imaging im);
 
 extern Imaging ImagingNewBlock(const char* mode, int xsize, int ysize);
-extern Imaging ImagingNewArray(const char* mode, int xsize, int ysize);
 extern Imaging ImagingNewMap(const char* filename, int readonly,
                              const char* mode, int xsize, int ysize);
 
@@ -172,7 +171,6 @@ extern Imaging ImagingNewPrologue(const char *mode,
 extern Imaging ImagingNewPrologueSubtype(const char *mode,
                                   int xsize, int ysize,
                                   int structure_size);
-extern Imaging ImagingNewEpilogue(Imaging im);
 
 extern void ImagingCopyInfo(Imaging destination, Imaging source);
 

libImaging/Storage.c

@@ -46,21 +46,20 @@ int ImagingNewCount = 0;
  */
 
 Imaging
-ImagingNewPrologueSubtype(const char *mode, int xsize, int ysize,
+ImagingNewPrologueSubtype(const char *mode, int xsize, int ysize, int size)
-                          int size)
 {
     Imaging im;
-    ImagingSectionCookie cookie;
-
-    im = (Imaging) calloc(1, size);
-    if (!im)
-        return (Imaging) ImagingError_MemoryError();
 
     /* linesize overflow check, roughly the current largest space req'd */
     if (xsize > (INT_MAX / 4) - 1) {
         return (Imaging) ImagingError_MemoryError();
     }
 
+    im = (Imaging) calloc(1, size);
+    if (!im) {
+        return (Imaging) ImagingError_MemoryError();
+    }
+
     /* Setup image descriptor */
     im->xsize = xsize;
     im->ysize = ysize;
@@ -206,47 +205,21 @@ ImagingNewPrologueSubtype(const char *mode, int xsize, int ysize,
 
     } else {
         free(im);
-        return (Imaging) ImagingError_ValueError("unrecognized mode");
+        return (Imaging) ImagingError_ValueError("unrecognized image mode");
     }
 
     /* Setup image descriptor */
     strcpy(im->mode, mode);
 
-    ImagingSectionEnter(&cookie);
-
     /* Pointer array (allocate at least one line, to avoid MemoryError
        exceptions on platforms where calloc(0, x) returns NULL) */
     im->image = (char **) calloc((ysize > 0) ? ysize : 1, sizeof(void *));
 
-    ImagingSectionLeave(&cookie);
+    if ( ! im->image) {
-
-    if (!im->image) {
         free(im);
         return (Imaging) ImagingError_MemoryError();
     }
 
-    ImagingNewCount++;
-
-    return im;
-}
-
-Imaging
-ImagingNewPrologue(const char *mode, int xsize, int ysize)
-{
-    return ImagingNewPrologueSubtype(
-        mode, xsize, ysize, sizeof(struct ImagingMemoryInstance)
-        );
-}
-
-Imaging
-ImagingNewEpilogue(Imaging im)
-{
-    /* If the raster data allocator didn't setup a destructor,
-       assume that it couldn't allocate the required amount of
-       memory. */
-    if (!im->destroy)
-        return (Imaging) ImagingError_MemoryError();
-
     /* Initialize alias pointers to pixel data. */
     switch (im->pixelsize) {
     case 1: case 2: case 3:
@@ -257,9 +230,18 @@ ImagingNewEpilogue(Imaging im)
         break;
     }
 
+    ImagingNewCount++;
+
     return im;
 }
 
+Imaging
+ImagingNewPrologue(const char *mode, int xsize, int ysize)
+{
+    return ImagingNewPrologueSubtype(
+        mode, xsize, ysize, sizeof(struct ImagingMemoryInstance));
+}
+
 void
 ImagingDelete(Imaging im)
 {
@@ -295,18 +277,13 @@ ImagingDestroyArray(Imaging im)
 }
 
 Imaging
-ImagingNewArray(const char *mode, int xsize, int ysize)
+ImagingAllocateArray(Imaging im)
 {
-    Imaging im;
     ImagingSectionCookie cookie;
 
     int y;
     char* p;
 
-    im = ImagingNewPrologue(mode, xsize, ysize);
-    if (!im)
-        return NULL;
-
     ImagingSectionEnter(&cookie);
 
     /* Allocate image as an array of lines */
@@ -322,10 +299,13 @@ ImagingNewArray(const char *mode, int xsize, int ysize)
 
     ImagingSectionLeave(&cookie);
 
-    if (y == im->ysize)
+    if (y != im->ysize) {
+        return (Imaging) ImagingError_MemoryError();
+    }
+
     im->destroy = ImagingDestroyArray;
 
-    return ImagingNewEpilogue(im);
+    return im;
 }
 
 
@@ -341,15 +321,10 @@ ImagingDestroyBlock(Imaging im)
 }
 
 Imaging
-ImagingNewBlock(const char *mode, int xsize, int ysize)
+ImagingAllocateBlock(Imaging im)
 {
-    Imaging im;
     Py_ssize_t y, i;
 
-    im = ImagingNewPrologue(mode, xsize, ysize);
-    if (!im)
-        return NULL;
-
     /* We shouldn't overflow, since the threshold defined
        below says that we're only going to allocate max 4M
        here before going to the array allocator. Check anyway.
@@ -357,7 +332,7 @@ ImagingNewBlock(const char *mode, int xsize, int ysize)
     if (im->linesize &&
         im->ysize > INT_MAX / im->linesize) {
         /* punt if we're going to overflow */
-        return NULL;
+        return (Imaging) ImagingError_MemoryError();
     }
 
     if (im->ysize * im->linesize <= 0) {
@@ -370,7 +345,10 @@ ImagingNewBlock(const char *mode, int xsize, int ysize)
         im->block = (char *) calloc(im->ysize, im->linesize);
     }
 
-    if (im->block) {
+    if ( ! im->block) {
+        return (Imaging) ImagingError_MemoryError();
+    }
+
     for (y = i = 0; y < im->ysize; y++) {
         im->image[y] = im->block + i;
         i += im->linesize;
@@ -378,9 +356,7 @@ ImagingNewBlock(const char *mode, int xsize, int ysize)
     
     im->destroy = ImagingDestroyBlock;
 
-    }
+    return im;
-
-    return ImagingNewEpilogue(im);
 }
 
 /* --------------------------------------------------------------------
@@ -395,33 +371,47 @@ ImagingNewBlock(const char *mode, int xsize, int ysize)
 Imaging
 ImagingNew(const char* mode, int xsize, int ysize)
 {
-    int bytes;
     Imaging im;
 
-    if (strcmp(mode, "") == 0)
-        return (Imaging) ImagingError_ValueError("empty mode");
-
-    if (strlen(mode) == 1) {
-        if (mode[0] == 'F' || mode[0] == 'I')
-            bytes = 4;
-        else
-            bytes = 1;
-    } else
-        bytes = strlen(mode); /* close enough */
-
     if (xsize < 0 || ysize < 0) {
         return (Imaging) ImagingError_ValueError("bad image size");
     }
 
-    if ((int64_t) xsize * (int64_t) ysize <= THRESHOLD / bytes) {
+    im = ImagingNewPrologue(mode, xsize, ysize);
-        im = ImagingNewBlock(mode, xsize, ysize);
+    if ( ! im)
-        if (im)
+        return NULL;
+
+    if (im->ysize && im->linesize <= THRESHOLD / im->ysize) {
+        if (ImagingAllocateBlock(im)) {
             return im;
+        }
         /* assume memory error; try allocating in array mode instead */
         ImagingError_Clear();
     }
 
-    return ImagingNewArray(mode, xsize, ysize);
+    if (ImagingAllocateArray(im)) {
+        return im;
+    }
+
+    ImagingDelete(im);
+    return NULL;
+}
+
+Imaging
+ImagingNewBlock(const char* mode, int xsize, int ysize)
+{
+    Imaging im;
+
+    im = ImagingNewPrologue(mode, xsize, ysize);
+    if ( ! im)
+        return NULL;
+
+    if (ImagingAllocateBlock(im)) {
+        return im;
+    }
+
+    ImagingDelete(im);
+    return NULL;
 }
 
 Imaging

map.c

@@ -229,9 +229,6 @@ mapping_readimage(ImagingMapperObject* mapper, PyObject* args)
 
     im->destroy = ImagingDestroyMap;
 
-    if (!ImagingNewEpilogue(im))
-        return NULL;
-
     mapper->offset += size;
 
     return PyImagingNew(im);
@@ -368,8 +365,7 @@ PyImaging_MapBuffer(PyObject* self, PyObject* args)
     }
 
     im = ImagingNewPrologueSubtype(
-        mode, xsize, ysize, sizeof(ImagingBufferInstance)
+        mode, xsize, ysize, sizeof(ImagingBufferInstance));
-        );
     if (!im)
         return NULL;
 
@@ -387,9 +383,6 @@ PyImaging_MapBuffer(PyObject* self, PyObject* args)
     ((ImagingBufferInstance*) im)->target = target;
     ((ImagingBufferInstance*) im)->view = view;
 
-    if (!ImagingNewEpilogue(im))
-        return NULL;
-
     return PyImagingNew(im);
 }