Small cleanup on BMPImageReader.cpp

Source/platform/image-decoders/bmp/BMPImageReader.cpp

 /*
  * Copyright (c) 2008, 2009, Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
  * met:
  *
  *     * Redistributions of source code must retain the above copyright
  * notice, this list of conditions and the following disclaimer.
  *     * Redistributions in binary form must reproduce the above
@@ skipping 77 matching lines @@
     // Clue-in decodeBMP() that we need to detect the correct info header size.
     memset(&m_infoHeader, 0, sizeof(m_infoHeader));
 }
 
 bool BMPImageReader::decodeBMP(bool onlySize)
 {
     // Calculate size of info header.
     if (!m_infoHeader.biSize && !readInfoHeaderSize())
         return false;
 
+    const size_t headerEnd = m_headerOffset + m_infoHeader.biSize;
     // Read and process info header.
-    if ((m_decodedOffset < (m_headerOffset + m_infoHeader.biSize)) && !processInfoHeader())
+    if ((m_decodedOffset < headerEnd) && !processInfoHeader())
         return false;
 
     // processInfoHeader() set the size, so if that's all we needed, we're done.
     if (onlySize)
         return true;
 
     // Read and process the bitmasks, if needed.
     if (m_needToProcessBitmasks && !processBitmasks())
         return false;
 
@@ skipping 58 matching lines @@
     ASSERT(m_decodedOffset == m_headerOffset);
     if ((m_decodedOffset > m_data->size()) || ((m_data->size() - m_decodedOffset) < 4))
         return false;
     m_infoHeader.biSize = readUint32(0);
     // Don't increment m_decodedOffset here, it just makes the code in
     // processInfoHeader() more confusing.
 
     // Don't allow the header to overflow (which would be harmless here, but
     // problematic or at least confusing in other places), or to overrun the
     // image data.
-    if (((m_headerOffset + m_infoHeader.biSize) < m_headerOffset) || (m_imgDataOffset && (m_imgDataOffset < (m_headerOffset + m_infoHeader.biSize))))
+    const size_t headerEnd = m_headerOffset + m_infoHeader.biSize;
+    if ((headerEnd < m_headerOffset) || (m_imgDataOffset && (m_imgDataOffset < headerEnd)))
         return m_parent->setFailed();
 
     // See if this is a header size we understand:
     // OS/2 1.x: 12
     if (m_infoHeader.biSize == 12)
         m_isOS21x = true;
     // Windows V3: 40
     else if ((m_infoHeader.biSize == 40) || isWindowsV4Plus())
         ;
     // OS/2 2.x: any multiple of 4 between 16 and 64, inclusive, or 42 or 46
@@ skipping 226 matching lines @@
         // 16 bits:    MSB <-                     xRRRRRGG GGGBBBBB -> LSB
         // 24/32 bits: MSB <- [AAAAAAAA] RRRRRRRR GGGGGGGG BBBBBBBB -> LSB
         const int numBits = (m_infoHeader.biBitCount == 16) ? 5 : 8;
         for (int i = 0; i <= 2; ++i)
             m_bitMasks[i] = ((static_cast<uint32_t>(1) << (numBits * (3 - i))) - 1) ^ ((static_cast<uint32_t>(1) << (numBits * (2 - i))) - 1);
     } else if (!isWindowsV4Plus()) {
         // For Windows V4+ BITFIELDS mode bitmaps, this was already done when
         // we read the info header.
 
         // Fail if we don't have enough file space for the bitmasks.
-        static const size_t SIZEOF_BITMASKS = 12;
-        if (((m_headerOffset + m_infoHeader.biSize + SIZEOF_BITMASKS) < (m_headerOffset + m_infoHeader.biSize)) || (m_imgDataOffset && (m_imgDataOffset < (m_headerOffset + m_infoHeader.biSize + SIZEOF_BITMASKS))))
+        const size_t bitmasksSize = 12;

[Peter Kasting, 2015/02/04 04:59:02]

Tiny nit: Swap this line with the next

[changseok, 2015/02/04 05:34:46]

Done.

+        const size_t headerEnd = m_headerOffset + m_infoHeader.biSize;
+        const size_t bitmasksEnd = headerEnd + bitmasksSize;
+        if ((bitmasksEnd < headerEnd) || (m_imgDataOffset && (m_imgDataOffset < bitmasksEnd)))
             return m_parent->setFailed();
 
         // Read bitmasks.
-        if ((m_data->size() - m_decodedOffset) < SIZEOF_BITMASKS)
+        if ((m_data->size() - m_decodedOffset) < bitmasksSize)
             return false;
         m_bitMasks[0] = readUint32(0);
         m_bitMasks[1] = readUint32(4);
         m_bitMasks[2] = readUint32(8);
 
-        m_decodedOffset += SIZEOF_BITMASKS;
+        m_decodedOffset += bitmasksSize;
     }
 
     // Alpha is a poorly-documented and inconsistently-used feature.
     //
     // Windows V4+ has an alpha bitmask in the info header. Unlike the R/G/B
     // bitmasks, the MSDN docs don't indicate that it is only valid for the
     // BITFIELDS compression format, so we respect it at all times.
     //
     // To complicate things, Windows V3 BMPs, which lack this mask, can specify
     // 32bpp format, which to any sane reader would imply an 8-bit alpha
@@ skipping 75 matching lines @@
 
         // Calculate LUT address.
         m_lookupTableAddresses[i] = numBits ? (nBitTo8BitlookupTable + (1 << numBits) - 2) : 0;
     }
 
     return true;
 }
 
 bool BMPImageReader::processColorTable()
 {
-    size_t tableSizeInBytes = m_infoHeader.biClrUsed * (m_isOS21x ? 3 : 4);
+    const size_t tableSizeInBytes = m_infoHeader.biClrUsed * (m_isOS21x ? 3 : 4);

[Peter Kasting, 2015/02/04 04:59:01]

Tiny nit: Swap this line with the next

[changseok, 2015/02/04 05:34:46]

Done.

+    const size_t headerEnd = m_headerOffset + m_infoHeader.biSize;
+    const size_t tableEnd = headerEnd + tableSizeInBytes;
 

[Peter Kasting, 2015/02/04 04:59:01]

Nit: Eliminate this newline and move the comment below to above the lines above

[changseok, 2015/02/04 05:34:46]

Done.

     // Fail if we don't have enough file space for the color table.
-    if (((m_headerOffset + m_infoHeader.biSize + tableSizeInBytes) < (m_headerOffset + m_infoHeader.biSize)) || (m_imgDataOffset && (m_imgDataOffset < (m_headerOffset + m_infoHeader.biSize + tableSizeInBytes))))
+    if ((tableEnd < headerEnd) || (m_imgDataOffset && (m_imgDataOffset < tableEnd)))
         return m_parent->setFailed();
 
     // Read color table.
     if ((m_decodedOffset > m_data->size()) || ((m_data->size() - m_decodedOffset) < tableSizeInBytes))
         return false;
     m_colorTable.resize(m_infoHeader.biClrUsed);
     for (size_t i = 0; i < m_infoHeader.biClrUsed; ++i) {
         m_colorTable[i].rgbBlue = m_data->data()[m_decodedOffset++];
         m_colorTable[i].rgbGreen = m_data->data()[m_decodedOffset++];
         m_colorTable[i].rgbRed = m_data->data()[m_decodedOffset++];
@@ skipping 249 matching lines @@
     // Finished decoding whole image.
     return Success;
 }
 
 void BMPImageReader::moveBufferToNextRow()
 {
     m_coord.move(-m_coord.x(), m_isTopDown ? 1 : -1);
 }
 
 } // namespace blink