Qualify the return value of SkImageDecoder::decode

src/images/SkImageDecoder_libico.cpp

 /*
  * Copyright 2006 The Android Open Source Project
  *
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file.
  */
 
 #include "SkColorPriv.h"
 #include "SkImageDecoder.h"
 #include "SkStream.h"
 #include "SkStreamPriv.h"
 #include "SkTypes.h"
 
 class SkICOImageDecoder : public SkImageDecoder {
 public:
     SkICOImageDecoder();
 
     virtual Format getFormat() const SK_OVERRIDE {
         return kICO_Format;
     }
 
 protected:
-    virtual bool onDecode(SkStream* stream, SkBitmap* bm, Mode) SK_OVERRIDE;
+    virtual Result onDecode(SkStream* stream, SkBitmap* bm, Mode) SK_OVERRIDE;
 
 private:
     typedef SkImageDecoder INHERITED;
 };
 
 /////////////////////////////////////////////////////////////////////////////////////////
 
 //read bytes starting from the begin-th index in the buffer
 //read in Intel order, and return an integer
 
@@ skipping 31 matching lines @@
     //  In the case of a 4 bit image with an odd width, we need to add some
     //  so we can go off the end of the drawn bitmap.
     //  Add 4 to ensure that it is still a multiple of 4.
     if (4 == bitCount && (w & 0x1)) {
         return (w + 1) << 2;
     }
     //  Otherwise return 0, which will allow it to be calculated automatically.
     return 0;
 }
 
-bool SkICOImageDecoder::onDecode(SkStream* stream, SkBitmap* bm, Mode mode)
+SkImageDecoder::Result SkICOImageDecoder::onDecode(SkStream* stream, SkBitmap* bm, Mode mode)
 {
     SkAutoMalloc autoMal;
     const size_t length = SkCopyStreamToStorage(&autoMal, stream);
     if (0 == length) {
-        return false;
+        return kFailure;
     }
 
     unsigned char* buf = (unsigned char*)autoMal.get();
 
     //these should always be the same - should i use for error checking? - what about files that have some
     //incorrect values, but still decode properly?
     int reserved = read2Bytes(buf, 0);    // 0
     int type = read2Bytes(buf, 2);        // 1
-    if (reserved != 0 || type != 1)
-        return false;
+    if (reserved != 0 || type != 1) {
+        return kFailure;
+    }
+
     int count = read2Bytes(buf, 4);
 
     //need to at least have enough space to hold the initial table of info
-    if (length < (size_t)(6 + count*16))
-        return false;
+    if (length < (size_t)(6 + count*16)) {
+        return kFailure;
+    }
 
 #ifdef SK_SUPPORT_LEGACY_IMAGEDECODER_CHOOSER
     int choice;
     Chooser* chooser = this->getChooser();
     //FIXME:if no chooser, consider providing the largest color image
     //what are the odds that the largest image would be monochrome?
     if (NULL == chooser) {
         choice = 0;
     } else {
         chooser->begin(count);
@@ skipping 24 matching lines @@
                 default:
                     SkDEBUGF(("Image with %ibpp not supported\n", bitCount));
                     continue;
             }
             chooser->inspect(i, c, width, height);
         }
         choice = chooser->choose();
     }
 
     //you never know what the chooser is going to supply
-    if (choice >= count || choice < 0)
-        return false;
+    if (choice >= count || choice < 0) {
+        return kFailure;
+    }
 #else
     const int choice = 0;   // TODO: fold this value into the expressions below
 #endif
 
     //skip ahead to the correct header
     //commented out lines are not used, but if i switch to other read method, need to know how many to skip
     //otherwise, they could be used for error checking
     int w = readByte(buf, 6 + choice*16);
     int h = readByte(buf, 7 + choice*16);
     int colorCount = readByte(buf, 8 + choice*16);
     //int reservedToo = readByte(buf, 9 + choice*16);   //0
     //int planes = read2Bytes(buf, 10 + choice*16);       //1 - but often 0
     //int fakeBitCount = read2Bytes(buf, 12 + choice*16); //should be real - usually 0
     const size_t size = read4Bytes(buf, 14 + choice*16);           //matters?
     const size_t offset = read4Bytes(buf, 18 + choice*16);
     // promote the sum to 64-bits to avoid overflow
     if (((uint64_t)offset + size) > length) {
-        return false;
+        return kFailure;
     }
 
     // Check to see if this is a PNG image inside the ICO
     {
         SkMemoryStream subStream(buf + offset, size, false);
         SkAutoTDelete<SkImageDecoder> otherDecoder(SkImageDecoder::Factory(&subStream));
         if (otherDecoder.get() != NULL) {
             // Disallow nesting ICO files within one another
+            // FIXME: Can ICO files contain other formats besides PNG?
             if (otherDecoder->getFormat() == SkImageDecoder::kICO_Format) {
-                return false;
+                return kFailure;
             }
             // Set fields on the other decoder to be the same as this one.
             this->copyFieldsToOther(otherDecoder.get());
-            if(otherDecoder->decode(&subStream, bm, this->getDefaultPref(), mode)) {
-                return true;
+            const Result result = otherDecoder->decode(&subStream, bm, this->getDefaultPref(),
+                                                       mode);
+            // FIXME: Should we just return result here? Is it possible that data that looked like
+            // a subimage was not, but was actually a valid ICO?
+            if (result != kFailure) {
+                return result;
             }
         }
     }
 
     //int infoSize = read4Bytes(buf, offset);             //40
     //int width = read4Bytes(buf, offset+4);              //should == w
     //int height = read4Bytes(buf, offset+8);             //should == 2*h
     //int planesToo = read2Bytes(buf, offset+12);         //should == 1 (does it?)
     int bitCount = read2Bytes(buf, offset+14);
 
@@ skipping 17 matching lines @@
         case 24:
             placePixel = &editPixelBit24;
             colorCount = 0;
             break;
         case 32:
             placePixel = &editPixelBit32;
             colorCount = 0;
             break;
         default:
             SkDEBUGF(("Decoding %ibpp is unimplemented\n", bitCount));
-            return false;
+            return kFailure;
     }
 
     //these should all be zero, but perhaps are not - need to check
     //int compression = read4Bytes(buf, offset+16);       //0
     //int imageSize = read4Bytes(buf, offset+20);         //0 - sometimes has a value
     //int xPixels = read4Bytes(buf, offset+24);           //0
     //int yPixels = read4Bytes(buf, offset+28);           //0
     //int colorsUsed = read4Bytes(buf, offset+32)         //0 - might have an actual value though
     //int colorsImportant = read4Bytes(buf, offset+36);   //0
 
@@ skipping 30 matching lines @@
     //if we allow different Configs, everything is the same til here
     //change the config, and use different address getter, and place index vs color, and add the color table
     //FIXME: what is the tradeoff in size?
     //if the andbitmap (mask) is all zeroes, then we can easily do an index bitmap
     //however, with small images with large colortables, maybe it's better to still do argb_8888
 
     bm->setInfo(SkImageInfo::MakeN32Premul(w, h), calculateRowBytesFor8888(w, bitCount));
 
     if (SkImageDecoder::kDecodeBounds_Mode == mode) {
         delete[] colors;
-        return true;
+        return kSuccess;
     }
 
     if (!this->allocPixelRef(bm, NULL))
     {
         delete[] colors;
-        return false;
+        return kFailure;
     }
 
     SkAutoLockPixels alp(*bm);
 
     for (int y = 0; y < h; y++)
     {
         for (int x = 0; x < w; x++)
         {
             //U32* address = bm->getAddr32(x, y);
 
             //check the alpha bit first, but pass it along to the function to figure out how to deal with it
             int andPixelNo = andLineWidth*(h-y-1)+x;
             //only need to get a new alphaByte when x %8 == 0
             //but that introduces an if and a mod - probably much slower
             //that's ok, it's just a read of an array, not a stream
             int alphaByte = readByte(buf, andOffset + (andPixelNo >> 3));
             int shift = 7 - (andPixelNo & 0x7);
             int m = 1 << shift;
 
             int pixelNo = lineWidth*(h-y-1)+x;
             placePixel(pixelNo, buf, xorOffset, x, y, w, bm, alphaByte, m, shift, colors);
 
         }
     }
 
     delete [] colors;
     //ensure we haven't read off the end?
     //of course this doesn't help us if the andOffset was a lie...
     //return andOffset + (andLineWidth >> 3) <= length;
-    return true;
+    return kSuccess;
 }   //onDecode
 
 //function to place the pixel, determined by the bitCount
 static void editPixelBit1(const int pixelNo, const unsigned char* buf,
             const int xorOffset, int& x, int y, const int w,
             SkBitmap* bm, int alphaByte, int m, int shift, SkPMColor* colors)
 {
     // note that this should be the same as/similar to the AND bitmap
     SkPMColor* address = bm->getAddr32(x,y);
     int byte = readByte(buf, xorOffset + (pixelNo >> 3));
@@ skipping 108 matching lines @@
 static SkImageDecoder_DecodeReg gReg(sk_libico_dfactory);
 
 static SkImageDecoder::Format get_format_ico(SkStreamRewindable* stream) {
     if (is_ico(stream)) {
         return SkImageDecoder::kICO_Format;
     }
     return SkImageDecoder::kUnknown_Format;
 }
 
 static SkImageDecoder_FormatReg gFormatReg(get_format_ico);