From b32a949d667600cb33e80e1bb8c97a23d96f27d9 Mon Sep 17 00:00:00 2001 From: Nicolas Pena Date: Thu, 22 Jun 2017 14:23:52 -0400 Subject: Continue BMP decoder cleanup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This CL replaces raw pointers with vector and unique_ptr. It also fixes other nits. Change-Id: I45c99c9aa658681ec3f0b48fc4f407b278b250f5 Reviewed-on: https://pdfium-review.googlesource.com/6830 Commit-Queue: Nicolás Peña Reviewed-by: Lei Zhang --- core/fxcodec/codec/ccodec_bmpmodule.cpp | 4 +- core/fxcodec/codec/ccodec_bmpmodule.h | 2 +- core/fxcodec/codec/fx_codec_progress.cpp | 8 +- core/fxcodec/lbmp/fx_bmp.cpp | 195 +++++++++++++++---------------- core/fxcodec/lbmp/fx_bmp.h | 16 +-- 5 files changed, 110 insertions(+), 115 deletions(-) diff --git a/core/fxcodec/codec/ccodec_bmpmodule.cpp b/core/fxcodec/codec/ccodec_bmpmodule.cpp index 1c8112498c..a64e6783e1 100644 --- a/core/fxcodec/codec/ccodec_bmpmodule.cpp +++ b/core/fxcodec/codec/ccodec_bmpmodule.cpp @@ -39,7 +39,7 @@ int32_t CCodec_BmpModule::ReadHeader(Context* pContext, bool* tb_flag, int32_t* components, int32_t* pal_num, - uint32_t** pal_pp, + std::vector* palette, CFX_DIBAttribute* pAttribute) { auto* ctx = static_cast(pContext); if (setjmp(ctx->m_Bmp.jmpbuf)) @@ -54,7 +54,7 @@ int32_t CCodec_BmpModule::ReadHeader(Context* pContext, *tb_flag = ctx->m_Bmp.imgTB_flag; *components = ctx->m_Bmp.components; *pal_num = ctx->m_Bmp.pal_num; - *pal_pp = ctx->m_Bmp.pal_ptr; + *palette = ctx->m_Bmp.palette; if (pAttribute) { pAttribute->m_wDPIUnit = FXCODEC_RESUNIT_METER; pAttribute->m_nXDPI = ctx->m_Bmp.dpi_x; diff --git a/core/fxcodec/codec/ccodec_bmpmodule.h b/core/fxcodec/codec/ccodec_bmpmodule.h index 4009150c59..e9ad7c3001 100644 --- a/core/fxcodec/codec/ccodec_bmpmodule.h +++ b/core/fxcodec/codec/ccodec_bmpmodule.h @@ -41,7 +41,7 @@ class CCodec_BmpModule { bool* tb_flag, int32_t* components, int32_t* pal_num, - uint32_t** pal_pp, + std::vector* palette, CFX_DIBAttribute* pAttribute); int32_t LoadImage(Context* pContext); }; diff --git a/core/fxcodec/codec/fx_codec_progress.cpp b/core/fxcodec/codec/fx_codec_progress.cpp index 3c377771ea..9f74ac3624 100644 --- a/core/fxcodec/codec/fx_codec_progress.cpp +++ b/core/fxcodec/codec/fx_codec_progress.cpp @@ -1027,10 +1027,10 @@ bool CCodec_ProgressiveDecoder::DetectImageType(FXCODEC_IMAGE_TYPE imageType, } m_offSet += size; pBmpModule->Input(m_pBmpContext.get(), m_pSrcBuf, size); - uint32_t* pPalette = nullptr; + std::vector palette; int32_t readResult = pBmpModule->ReadHeader( m_pBmpContext.get(), &m_SrcWidth, &m_SrcHeight, &m_BmpIsTopBottom, - &m_SrcComponents, &m_SrcPaletteNumber, &pPalette, pAttribute); + &m_SrcComponents, &m_SrcPaletteNumber, &palette, pAttribute); while (readResult == 2) { FXCODEC_STATUS error_status = FXCODEC_STATUS_ERR_FORMAT; if (!BmpReadMoreData(pBmpModule, error_status)) { @@ -1039,7 +1039,7 @@ bool CCodec_ProgressiveDecoder::DetectImageType(FXCODEC_IMAGE_TYPE imageType, } readResult = pBmpModule->ReadHeader( m_pBmpContext.get(), &m_SrcWidth, &m_SrcHeight, &m_BmpIsTopBottom, - &m_SrcComponents, &m_SrcPaletteNumber, &pPalette, pAttribute); + &m_SrcComponents, &m_SrcPaletteNumber, &palette, pAttribute); } if (readResult == 1) { m_SrcBPC = 8; @@ -1047,7 +1047,7 @@ bool CCodec_ProgressiveDecoder::DetectImageType(FXCODEC_IMAGE_TYPE imageType, FX_Free(m_pSrcPalette); if (m_SrcPaletteNumber) { m_pSrcPalette = FX_Alloc(FX_ARGB, m_SrcPaletteNumber); - memcpy(m_pSrcPalette, pPalette, + memcpy(m_pSrcPalette, palette.data(), m_SrcPaletteNumber * sizeof(uint32_t)); } else { m_pSrcPalette = nullptr; diff --git a/core/fxcodec/lbmp/fx_bmp.cpp b/core/fxcodec/lbmp/fx_bmp.cpp index a4b97efbac..989b3a4f84 100644 --- a/core/fxcodec/lbmp/fx_bmp.cpp +++ b/core/fxcodec/lbmp/fx_bmp.cpp @@ -9,16 +9,17 @@ #include #include +#include "core/fxcrt/fx_system.h" +#include "third_party/base/ptr_util.h" + +static_assert(sizeof(BmpFileHeader) == 14, + "BmpFileHeader should have a size of 14"); + namespace { const size_t kBmpCoreHeaderSize = 12; const size_t kBmpInfoHeaderSize = 40; -// TODO(thestig): Replace with FXDWORD_GET_LSBFIRST? -uint32_t GetDWord_LSBFirst(uint8_t* p) { - return p[0] | (p[1] << 8) | (p[2] << 16) | (p[3] << 24); -} - uint8_t HalfRoundUp(uint8_t value) { uint16_t value16 = value; return static_cast((value16 + 1) / 2); @@ -33,8 +34,8 @@ uint16_t GetWord_LSBFirst(uint8_t* p) { BMPDecompressor::BMPDecompressor() : err_ptr(nullptr), context_ptr(nullptr), - bmp_header_ptr(FX_Alloc(BmpFileHeader, 1)), - bmp_infoheader_ptr(nullptr), + next_in(nullptr), + header_offset(0), width(0), height(0), compress_flag(0), @@ -46,7 +47,6 @@ BMPDecompressor::BMPDecompressor() imgTB_flag(false), pal_num(0), pal_type(0), - pal_ptr(nullptr), data_size(0), img_data_offset(0), img_ifh_size(0), @@ -57,15 +57,11 @@ BMPDecompressor::BMPDecompressor() mask_red(0), mask_green(0), mask_blue(0), - next_in(nullptr), avail_in(0), skip_size(0), decode_status(BMP_D_STATUS_HEADER) {} -BMPDecompressor::~BMPDecompressor() { - FX_Free(pal_ptr); - FX_Free(bmp_header_ptr); -} +BMPDecompressor::~BMPDecompressor() {} void BMPDecompressor::Error(const char* err_msg) { strncpy(err_ptr, err_msg, BMP_MAX_ERROR_SIZE - 1); @@ -86,17 +82,19 @@ bool BMPDecompressor::GetDataPosition(uint32_t rcd_pos) { int32_t BMPDecompressor::ReadHeader() { uint32_t skip_size_org = skip_size; if (decode_status == BMP_D_STATUS_HEADER) { - ASSERT(sizeof(BmpFileHeader) == 14); - BmpFileHeader* bmp_header_ptr = nullptr; - if (!ReadData((uint8_t**)&bmp_header_ptr, 14)) + BmpFileHeader* pBmp_header = nullptr; + if (!ReadData(reinterpret_cast(&pBmp_header), + sizeof(BmpFileHeader))) { return 2; + } - bmp_header_ptr->bfType = - GetWord_LSBFirst((uint8_t*)&bmp_header_ptr->bfType); - bmp_header_ptr->bfOffBits = - GetDWord_LSBFirst((uint8_t*)&bmp_header_ptr->bfOffBits); - data_size = GetDWord_LSBFirst((uint8_t*)&bmp_header_ptr->bfSize); - if (bmp_header_ptr->bfType != BMP_SIGNATURE) { + pBmp_header->bfType = + GetWord_LSBFirst(reinterpret_cast(&pBmp_header->bfType)); + pBmp_header->bfOffBits = FXDWORD_GET_LSBFIRST( + reinterpret_cast(&pBmp_header->bfOffBits)); + data_size = + FXDWORD_GET_LSBFIRST(reinterpret_cast(&pBmp_header->bfSize)); + if (pBmp_header->bfType != BMP_SIGNATURE) { Error("Not A Bmp Image"); NOTREACHED(); } @@ -104,7 +102,8 @@ int32_t BMPDecompressor::ReadHeader() { skip_size = skip_size_org; return 2; } - img_ifh_size = GetDWord_LSBFirst(next_in + skip_size); + img_ifh_size = + FXDWORD_GET_LSBFIRST(static_cast(next_in + skip_size)); pal_type = 0; static_assert(sizeof(BmpCoreHeader) == kBmpCoreHeaderSize, "BmpCoreHeader has wrong size"); @@ -113,84 +112,71 @@ int32_t BMPDecompressor::ReadHeader() { switch (img_ifh_size) { case kBmpCoreHeaderSize: { pal_type = 1; - BmpCoreHeaderPtr bmp_core_header_ptr = nullptr; - if (!ReadData(reinterpret_cast(&bmp_core_header_ptr), + BmpCoreHeader* pBmp_core_header = nullptr; + if (!ReadData(reinterpret_cast(&pBmp_core_header), img_ifh_size)) { skip_size = skip_size_org; return 2; } width = GetWord_LSBFirst( - reinterpret_cast(&bmp_core_header_ptr->bcWidth)); + reinterpret_cast(&pBmp_core_header->bcWidth)); height = GetWord_LSBFirst( - reinterpret_cast(&bmp_core_header_ptr->bcHeight)); + reinterpret_cast(&pBmp_core_header->bcHeight)); bitCounts = GetWord_LSBFirst( - reinterpret_cast(&bmp_core_header_ptr->bcBitCount)); + reinterpret_cast(&pBmp_core_header->bcBitCount)); compress_flag = BMP_RGB; imgTB_flag = false; } break; case kBmpInfoHeaderSize: { - BmpInfoHeaderPtr bmp_info_header_ptr = nullptr; - if (!ReadData((uint8_t**)&bmp_info_header_ptr, img_ifh_size)) { + BmpInfoHeader* pBmp_info_header = nullptr; + if (!ReadData(reinterpret_cast(&pBmp_info_header), + img_ifh_size)) { skip_size = skip_size_org; return 2; } - width = GetDWord_LSBFirst((uint8_t*)&bmp_info_header_ptr->biWidth); - int32_t signed_height = - GetDWord_LSBFirst((uint8_t*)&bmp_info_header_ptr->biHeight); - bitCounts = - GetWord_LSBFirst((uint8_t*)&bmp_info_header_ptr->biBitCount); - compress_flag = - GetDWord_LSBFirst((uint8_t*)&bmp_info_header_ptr->biCompression); - color_used = - GetDWord_LSBFirst((uint8_t*)&bmp_info_header_ptr->biClrUsed); - dpi_x = (int32_t)GetDWord_LSBFirst( - (uint8_t*)&bmp_info_header_ptr->biXPelsPerMeter); - dpi_y = (int32_t)GetDWord_LSBFirst( - (uint8_t*)&bmp_info_header_ptr->biYPelsPerMeter); - if (signed_height < 0) { - if (signed_height == std::numeric_limits::min()) { - Error("Unsupported height"); - NOTREACHED(); - } - height = -signed_height; - imgTB_flag = true; - } else { - height = signed_height; - } + width = FXDWORD_GET_LSBFIRST( + reinterpret_cast(&pBmp_info_header->biWidth)); + int32_t signed_height = FXDWORD_GET_LSBFIRST( + reinterpret_cast(&pBmp_info_header->biHeight)); + bitCounts = GetWord_LSBFirst( + reinterpret_cast(&pBmp_info_header->biBitCount)); + compress_flag = FXDWORD_GET_LSBFIRST( + reinterpret_cast(&pBmp_info_header->biCompression)); + color_used = FXDWORD_GET_LSBFIRST( + reinterpret_cast(&pBmp_info_header->biClrUsed)); + dpi_x = static_cast(FXDWORD_GET_LSBFIRST( + reinterpret_cast(&pBmp_info_header->biXPelsPerMeter))); + dpi_y = static_cast(FXDWORD_GET_LSBFIRST( + reinterpret_cast(&pBmp_info_header->biYPelsPerMeter))); + SetHeight(signed_height); } break; default: { if (img_ifh_size > std::min(kBmpInfoHeaderSize, sizeof(BmpInfoHeader))) { - BmpInfoHeaderPtr bmp_info_header_ptr = nullptr; - if (!ReadData((uint8_t**)&bmp_info_header_ptr, img_ifh_size)) { + BmpInfoHeader* pBmp_info_header = nullptr; + if (!ReadData(reinterpret_cast(&pBmp_info_header), + img_ifh_size)) { skip_size = skip_size_org; return 2; } uint16_t biPlanes; - width = GetDWord_LSBFirst((uint8_t*)&bmp_info_header_ptr->biWidth); - int32_t signed_height = - GetDWord_LSBFirst((uint8_t*)&bmp_info_header_ptr->biHeight); - bitCounts = - GetWord_LSBFirst((uint8_t*)&bmp_info_header_ptr->biBitCount); - compress_flag = - GetDWord_LSBFirst((uint8_t*)&bmp_info_header_ptr->biCompression); - color_used = - GetDWord_LSBFirst((uint8_t*)&bmp_info_header_ptr->biClrUsed); - biPlanes = GetWord_LSBFirst((uint8_t*)&bmp_info_header_ptr->biPlanes); - dpi_x = GetDWord_LSBFirst( - (uint8_t*)&bmp_info_header_ptr->biXPelsPerMeter); - dpi_y = GetDWord_LSBFirst( - (uint8_t*)&bmp_info_header_ptr->biYPelsPerMeter); - if (signed_height < 0) { - if (signed_height == std::numeric_limits::min()) { - Error("Unsupported height"); - NOTREACHED(); - } - height = -signed_height; - imgTB_flag = true; - } else { - height = signed_height; - } + width = FXDWORD_GET_LSBFIRST( + reinterpret_cast(&pBmp_info_header->biWidth)); + int32_t signed_height = FXDWORD_GET_LSBFIRST( + reinterpret_cast(&pBmp_info_header->biHeight)); + bitCounts = GetWord_LSBFirst( + reinterpret_cast(&pBmp_info_header->biBitCount)); + compress_flag = FXDWORD_GET_LSBFIRST( + reinterpret_cast(&pBmp_info_header->biCompression)); + color_used = FXDWORD_GET_LSBFIRST( + reinterpret_cast(&pBmp_info_header->biClrUsed)); + biPlanes = GetWord_LSBFirst( + reinterpret_cast(&pBmp_info_header->biPlanes)); + dpi_x = FXDWORD_GET_LSBFIRST( + reinterpret_cast(&pBmp_info_header->biXPelsPerMeter)); + dpi_y = FXDWORD_GET_LSBFIRST( + reinterpret_cast(&pBmp_info_header->biYPelsPerMeter)); + SetHeight(signed_height); if (compress_flag == BMP_RGB && biPlanes == 1 && color_used == 0) break; } @@ -208,7 +194,7 @@ int32_t BMPDecompressor::ReadHeader() { case 8: case 16: case 24: { - if (color_used > ((uint32_t)1) << bitCounts) { + if (color_used > 1U << bitCounts) { Error("The Bmp File Is Corrupt"); NOTREACHED(); } @@ -255,21 +241,20 @@ int32_t BMPDecompressor::ReadHeader() { NOTREACHED(); } uint32_t* mask; - if (ReadData((uint8_t**)&mask, 3 * sizeof(uint32_t)) == nullptr) { + if (ReadData(reinterpret_cast(&mask), 3 * sizeof(uint32_t)) == + nullptr) { skip_size = skip_size_org; return 2; } - mask_red = GetDWord_LSBFirst((uint8_t*)&mask[0]); - mask_green = GetDWord_LSBFirst((uint8_t*)&mask[1]); - mask_blue = GetDWord_LSBFirst((uint8_t*)&mask[2]); + mask_red = FXDWORD_GET_LSBFIRST(reinterpret_cast(&mask[0])); + mask_green = FXDWORD_GET_LSBFIRST(reinterpret_cast(&mask[1])); + mask_blue = FXDWORD_GET_LSBFIRST(reinterpret_cast(&mask[2])); if (mask_red & mask_green || mask_red & mask_blue || mask_green & mask_blue) { Error("The Bitfield Bmp File Is Corrupt"); NOTREACHED(); } - if (bmp_header_ptr->bfOffBits < 26 + img_ifh_size) { - bmp_header_ptr->bfOffBits = 26 + img_ifh_size; - } + header_offset = std::max(header_offset, 26 + img_ifh_size); SaveDecodingStatus(BMP_D_STATUS_DATA_PRE); return 1; } else if (bitCounts == 16) { @@ -280,37 +265,32 @@ int32_t BMPDecompressor::ReadHeader() { pal_num = 0; if (bitCounts < 16) { pal_num = 1 << bitCounts; - if (color_used != 0) { + if (color_used != 0) pal_num = color_used; - } uint8_t* src_pal_ptr = nullptr; uint32_t src_pal_size = pal_num * (pal_type ? 3 : 4); - if (ReadData((uint8_t**)&src_pal_ptr, src_pal_size) == nullptr) { + if (ReadData(&src_pal_ptr, src_pal_size) == nullptr) { skip_size = skip_size_org; return 2; } - FX_Free(pal_ptr); - pal_ptr = FX_Alloc(uint32_t, pal_num); + palette.resize(pal_num); int32_t src_pal_index = 0; if (pal_type == BMP_PAL_OLD) { while (src_pal_index < pal_num) { - pal_ptr[src_pal_index++] = BMP_PAL_ENCODE( + palette[src_pal_index++] = BMP_PAL_ENCODE( 0x00, src_pal_ptr[2], src_pal_ptr[1], src_pal_ptr[0]); src_pal_ptr += 3; } } else { while (src_pal_index < pal_num) { - pal_ptr[src_pal_index++] = BMP_PAL_ENCODE( + palette[src_pal_index++] = BMP_PAL_ENCODE( src_pal_ptr[3], src_pal_ptr[2], src_pal_ptr[1], src_pal_ptr[0]); src_pal_ptr += 4; } } } - if (bmp_header_ptr->bfOffBits < - 14 + img_ifh_size + pal_num * (pal_type ? 3 : 4)) { - bmp_header_ptr->bfOffBits = - 14 + img_ifh_size + pal_num * (pal_type ? 3 : 4); - } + header_offset = std::max(header_offset, + 14 + img_ifh_size + pal_num * (pal_type ? 3 : 4)); SaveDecodingStatus(BMP_D_STATUS_DATA_PRE); } return 1; @@ -331,7 +311,7 @@ bool BMPDecompressor::ValidateFlag() const { int32_t BMPDecompressor::DecodeImage() { if (decode_status == BMP_D_STATUS_DATA_PRE) { avail_in = 0; - if (!GetDataPosition(bmp_header_ptr->bfOffBits)) { + if (!GetDataPosition(header_offset)) { decode_status = BMP_D_STATUS_TAIL; Error("The Bmp File Is Corrupt, Unexpected Stream Offset"); NOTREACHED(); @@ -406,7 +386,7 @@ int32_t BMPDecompressor::DecodeRGB() { green_bits -= 8; red_bits -= 8; for (uint32_t col = 0; col < width; ++col) { - *buf = GetWord_LSBFirst((uint8_t*)buf); + *buf = GetWord_LSBFirst(reinterpret_cast(buf)); out_row_buffer[idx++] = static_cast((*buf & mask_blue) << blue_bits); out_row_buffer[idx++] = @@ -681,3 +661,16 @@ uint32_t BMPDecompressor::GetAvailInput(uint8_t** avail_buf) { } return avail_in; } + +void BMPDecompressor::SetHeight(int32_t signed_height) { + if (signed_height >= 0) { + height = signed_height; + return; + } + if (signed_height == std::numeric_limits::min()) { + Error("Unsupported height"); + NOTREACHED(); + } + height = -signed_height; + imgTB_flag = true; +} diff --git a/core/fxcodec/lbmp/fx_bmp.h b/core/fxcodec/lbmp/fx_bmp.h index e30402070b..175d9bede5 100644 --- a/core/fxcodec/lbmp/fx_bmp.h +++ b/core/fxcodec/lbmp/fx_bmp.h @@ -8,6 +8,8 @@ #define CORE_FXCODEC_LBMP_FX_BMP_H_ #include + +#include #include #include "core/fxcodec/codec/ccodec_bmpmodule.h" @@ -44,14 +46,14 @@ typedef struct tagBmpFileHeader { uint16_t bfReserved1; uint16_t bfReserved2; uint32_t bfOffBits; -} BmpFileHeader, *BmpFileHeaderPtr; +} BmpFileHeader; typedef struct tagBmpCoreHeader { uint32_t bcSize; uint16_t bcWidth; uint16_t bcHeight; uint16_t bcPlanes; uint16_t bcBitCount; -} BmpCoreHeader, *BmpCoreHeaderPtr; +} BmpCoreHeader; typedef struct tagBmpInfoHeader { uint32_t biSize; int32_t biWidth; @@ -64,7 +66,7 @@ typedef struct tagBmpInfoHeader { int32_t biYPelsPerMeter; uint32_t biClrUsed; uint32_t biClrImportant; -} BmpInfoHeader, *BmpInfoHeaderPtr; +} BmpInfoHeader; #pragma pack() class BMPDecompressor { @@ -83,10 +85,11 @@ class BMPDecompressor { void* context_ptr; - BmpFileHeaderPtr bmp_header_ptr; - BmpInfoHeaderPtr bmp_infoheader_ptr; std::vector out_row_buffer; + std::vector palette; + uint8_t* next_in; + uint32_t header_offset; uint32_t width; uint32_t height; uint32_t compress_flag; @@ -98,7 +101,6 @@ class BMPDecompressor { bool imgTB_flag; int32_t pal_num; int32_t pal_type; - uint32_t* pal_ptr; uint32_t data_size; uint32_t img_data_offset; uint32_t img_ifh_size; @@ -110,7 +112,6 @@ class BMPDecompressor { uint32_t mask_green; uint32_t mask_blue; - uint8_t* next_in; uint32_t avail_in; uint32_t skip_size; int32_t decode_status; @@ -125,6 +126,7 @@ class BMPDecompressor { void SaveDecodingStatus(int32_t status); bool ValidateColorIndex(uint8_t val); bool ValidateFlag() const; + void SetHeight(int32_t signed_height); }; class CBmpContext : public CCodec_BmpModule::Context { -- cgit v1.2.3