add SkSafeRef / SkSafeUnref as inline static functions, to use in place of our

existing obj->safeRef(), which is unsafe since it can its 'if (this)' can be
optimized away by some compilers.

fix some overflows in mimpmap generation



git-svn-id: http://skia.googlecode.com/svn/trunk@181 2bbb7eff-a529-9590-31e7-b0007b416f81

1 files changed, 88 insertions, 45 deletions

diff --git a/src/core/SkBitmap.cpp b/src/core/SkBitmap.cpp
index 0d475086ee..d7a311e778 100644
--- a/src/core/SkBitmap.cpp
+++ b/src/core/SkBitmap.cpp
@@ -26,6 +26,10 @@
 #include "SkPackBits.h"
 #include <new>
 
+static bool isPos32Bits(const Sk64& value) {
+    return !value.isNeg() && value.is32();
+}
+
 #ifdef SK_SUPPORT_MIPMAP
 struct MipLevel {
     void*       fPixels;
@@ -40,9 +44,17 @@ struct SkBitmap::MipMap : SkNoncopyable {
 //  Pixels[]
 
     static MipMap* Alloc(int levelCount, size_t pixelSize) {
-        MipMap* mm = (MipMap*)sk_malloc_throw(sizeof(MipMap) +
-                                              levelCount * sizeof(MipLevel) +
-                                              pixelSize);
+        if (levelCount < 0) {
+            return NULL;
+        }
+        Sk64 size;
+        size.setMul(levelCount + 1, sizeof(MipLevel));
+        size.add(sizeof(MipMap));
+        size.add(pixelSize);
+        if (!isPos32Bits(size)) {
+            return NULL;
+        }
+        MipMap* mm = (MipMap*)sk_malloc_throw(size.get32());
         mm->fRefCnt = 1;
         mm->fLevelCount = levelCount;
         return mm;
@@ -54,18 +66,15 @@ struct SkBitmap::MipMap : SkNoncopyable {
     const void* pixels() const { return levels() + fLevelCount; }
     void* pixels() { return levels() + fLevelCount; }
 
-    void safeRef() {
-        if (this) {
-            SkASSERT(fRefCnt > 0);
-            sk_atomic_inc(&fRefCnt);
+    void ref() {
+        if (SK_MaxS32 == sk_atomic_inc(&fRefCnt)) {
+            sk_throw();
         }
     }
-    void safeUnref() {
-        if (this) {
-            SkASSERT(fRefCnt > 0);
-            if (sk_atomic_dec(&fRefCnt) == 1) {
-                sk_free(this);
-            }
+    void unref() {
+        SkASSERT(fRefCnt > 0);
+        if (sk_atomic_dec(&fRefCnt) == 1) {
+            sk_free(this);
         }
     }
 };
@@ -98,7 +107,7 @@ SkBitmap& SkBitmap::operator=(const SkBitmap& src) {
         // inc src reference counts
         src.fPixelRef->safeRef();
 #ifdef SK_SUPPORT_MIPMAP
-        src.fMipMap->safeRef();
+        SkSafeRef(src.fMipMap);
 #endif
 
         // we reset our locks if we get blown away
@@ -175,33 +184,40 @@ int SkBitmap::ComputeBytesPerPixel(SkBitmap::Config config) {
 }
 
 int SkBitmap::ComputeRowBytes(Config c, int width) {
-    int rowBytes = 0;
+    if (width < 0) {
+        return 0;
+    }
+
+    Sk64 rowBytes;
+    rowBytes.setZero();
 
     switch (c) {
         case kNo_Config:
         case kRLE_Index8_Config:
-            // assume that the bitmap has no pixels to draw to
-            rowBytes = 0;
             break;
         case kA1_Config:
-            rowBytes = (width + 7) >> 3;
+            rowBytes.set(width);
+            rowBytes.add(7);
+            rowBytes.shiftRight(3);
             break;
         case kA8_Config:
         case kIndex8_Config:
-            rowBytes = width;
+            rowBytes.set(width);
             break;
         case kRGB_565_Config:
         case kARGB_4444_Config:
-            rowBytes = width << 1;
+            rowBytes.set(width);
+            rowBytes.shiftLeft(1);
             break;
         case kARGB_8888_Config:
-            rowBytes = width << 2;
+            rowBytes.set(width);
+            rowBytes.shiftLeft(2);
             break;
         default:
             SkASSERT(!"unknown config");
             break;
     }
-    return rowBytes;
+    return isPos32Bits(rowBytes) ? rowBytes.get32() : 0;
 }
 
 Sk64 SkBitmap::ComputeSize64(Config c, int width, int height) {
@@ -212,18 +228,25 @@ Sk64 SkBitmap::ComputeSize64(Config c, int width, int height) {
 
 size_t SkBitmap::ComputeSize(Config c, int width, int height) {
     Sk64 size = SkBitmap::ComputeSize64(c, width, height);
-    if (size.isNeg() || !size.is32()) {
-        return 0;
-    }
-    return size.get32();
+    return isPos32Bits(size) ? size.get32() : 0;
 }
 
 void SkBitmap::setConfig(Config c, int width, int height, int rowBytes) {
     this->freePixels();
 
+    if ((width | height | rowBytes) < 0) {
+    ERROR:
+        this->reset();
+        return;
+    }
+
     if (rowBytes == 0) {
         rowBytes = SkBitmap::ComputeRowBytes(c, width);
+        if (0 == rowBytes && kNo_Config != c) {
+            goto ERROR;
+        }
     }
+    
     fConfig     = SkToU8(c);
     fWidth      = width;
     fHeight     = height;
@@ -248,8 +271,10 @@ void SkBitmap::updatePixelsFromRef() const {
         } else {
             SkASSERT(0 == fPixelLockCount);
             fPixels = NULL;
-            fColorTable->safeUnref();
-            fColorTable = NULL;
+            if (fColorTable) {
+                fColorTable->unref();
+                fColorTable = NULL;
+            }
         }
     }
 }
@@ -315,8 +340,10 @@ void SkBitmap::freePixels() {
     // if we're gonna free the pixels, we certainly need to free the mipmap
     this->freeMipMap();
 
-    fColorTable->safeUnref();
-    fColorTable = NULL;
+    if (fColorTable) {
+        fColorTable->unref();
+        fColorTable = NULL;
+    }
 
     if (NULL != fPixelRef) {
         if (fPixelLockCount > 0) {
@@ -332,8 +359,10 @@ void SkBitmap::freePixels() {
 
 void SkBitmap::freeMipMap() {
 #ifdef SK_SUPPORT_MIPMAP
-    fMipMap->safeUnref();
-    fMipMap = NULL;
+    if (fMipMap) {
+        fMipMap->unref();
+        fMipMap = NULL;
+    }
 #endif
 }
 
@@ -359,7 +388,7 @@ SkMallocPixelRef::SkMallocPixelRef(void* storage, size_t size,
 }
 
 SkMallocPixelRef::~SkMallocPixelRef() {
-    fCTable->safeUnref();
+    SkSafeUnref(fCTable);
     sk_free(fStorage);
 }
 
@@ -898,13 +927,18 @@ void SkBitmap::buildMipMap(bool forceRebuild) {
         default:
             return; // don't build mipmaps for these configs
     }
+    
+    SkAutoLockPixels alp(*this);
+    if (!this->readyToDraw()) {
+        return;
+    }
 
     // whip through our loop to compute the exact size needed
     size_t  size = 0;
     int     maxLevels = 0;
     {
-        unsigned    width = this->width();
-        unsigned    height = this->height();
+        int width = this->width();
+        int height = this->height();
         for (;;) {
             width >>= 1;
             height >>= 1;
@@ -915,20 +949,29 @@ void SkBitmap::buildMipMap(bool forceRebuild) {
             maxLevels += 1;
         }
     }
+    
+    // nothing to build
     if (0 == maxLevels) {
         return;
     }
 
-    MipMap*     mm = MipMap::Alloc(maxLevels, size);
+    SkBitmap srcBM(*this);
+    srcBM.lockPixels();
+    if (!srcBM.readyToDraw()) {
+        return;
+    }
+
+    MipMap* mm = MipMap::Alloc(maxLevels, size);
+    if (NULL == mm) {
+        return;
+    }
+
     MipLevel*   level = mm->levels();
     uint8_t*    addr = (uint8_t*)mm->pixels();
-
-    unsigned    width = this->width();
-    unsigned    height = this->height();
+    int         width = this->width();
+    int         height = this->height();
     unsigned    rowBytes = this->rowBytes();
-    SkBitmap    srcBM(*this), dstBM;
-
-    srcBM.lockPixels();
+    SkBitmap    dstBM;
 
     for (int i = 0; i < maxLevels; i++) {
         width >>= 1;
@@ -943,8 +986,8 @@ void SkBitmap::buildMipMap(bool forceRebuild) {
         dstBM.setConfig(config, width, height, rowBytes);
         dstBM.setPixels(addr);
 
-        for (unsigned y = 0; y < height; y++) {
-            for (unsigned x = 0; x < width; x++) {
+        for (int y = 0; y < height; y++) {
+            for (int x = 0; x < width; x++) {
                 proc(&dstBM, x, y, srcBM);
             }
         }
@@ -1251,7 +1294,7 @@ void SkBitmap::unflatten(SkFlattenableReadBuffer& buffer) {
             } else {
                 buffer.skip(size);
             }
-            ctable->safeUnref();
+            SkSafeUnref(ctable);
             break;
         }
         case SERIALIZE_PIXELTYPE_NONE: