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 78 matching lines @@
     memset(&m_infoHeader, 0, sizeof(m_infoHeader));
 }
 
 bool BMPImageReader::decodeBMP(bool onlySize)
 {
     // Calculate size of info header.
     if (!m_infoHeader.biSize && !readInfoHeaderSize())
         return false;
 
     // Read and process info header.
-    if ((m_decodedOffset < (m_headerOffset + m_infoHeader.biSize)) && !processInfoHeader())
+    if ((m_decodedOffset < sizeOfHeaderAndInfoHeader()) && !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 headerAndInfoHeaderSize = sizeOfHeaderAndInfoHeader();

[Peter Kasting, 2015/02/03 23:56:43]

Nit: Call this |headerEnd|.

[changseok, 2015/02/04 04:10:32]

Done.

+    if ((headerAndInfoHeaderSize < m_headerOffset) || (m_imgDataOffset && (m_imgDataOffset < headerAndInfoHeaderSize)))
         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;

[Peter Kasting, 2015/02/03 23:56:43]

Nit: While here, change to:

        const size_t bitmasksSize = 12;

[changseok, 2015/02/04 04:10:32]

Done.

-        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 headerAndInfoHeaderSize = sizeOfHeaderAndInfoHeader();
+        const size_t headerInfoHeaderAndBitmasksSize = headerAndInfoHeaderSize + SIZEOF_BITMASKS;

[Peter Kasting, 2015/02/03 23:56:43]

Nit: Call these |headerEnd| and |bitmasksEnd|.

[changseok, 2015/02/04 04:10:32]

Done.

+        if ((headerInfoHeaderAndBitmasksSize < headerAndInfoHeaderSize) || (m_imgDataOffset && (m_imgDataOffset < headerInfoHeaderAndBitmasksSize)))
             return m_parent->setFailed();
 
         // Read bitmasks.
         if ((m_data->size() - m_decodedOffset) < SIZEOF_BITMASKS)
             return false;
         m_bitMasks[0] = readUint32(0);
         m_bitMasks[1] = readUint32(4);
         m_bitMasks[2] = readUint32(8);
 
         m_decodedOffset += SIZEOF_BITMASKS;
@@ skipping 86 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);
+    size_t headerAndInfoHeaderSize = sizeOfHeaderAndInfoHeader();
+    size_t headerInfoHeaderAndTableSize = headerAndInfoHeaderSize + tableSizeInBytes;

[Peter Kasting, 2015/02/03 23:56:43]

Nit: Make all these const.  Call the new variables |headerEnd| and |tableEnd|.

[changseok, 2015/02/04 04:10:32]

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 ((headerInfoHeaderAndTableSize < headerAndInfoHeaderSize) || (m_imgDataOffset && (m_imgDataOffset < headerInfoHeaderAndTableSize)))
         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