Fix nits in CFX_BmpDecompressor.
- Initialize members in the header.
- Delete unused member, pack booleans tighter.
- Use UnownedPtr for a pointer member.
- Put break statements in the right scope.
- Rename some variables to the right style.
- Change ReadHeader() to have early returns.
Change-Id: I6c94997167c61d81b15bde531ad64dc9001d022b
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/56010
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Commit-Queue: Lei Zhang <thestig@chromium.org>
diff --git a/core/fxcodec/bmp/cfx_bmpdecompressor.cpp b/core/fxcodec/bmp/cfx_bmpdecompressor.cpp
index 000a9e2..9d1010a 100644
--- a/core/fxcodec/bmp/cfx_bmpdecompressor.cpp
+++ b/core/fxcodec/bmp/cfx_bmpdecompressor.cpp
@@ -19,8 +19,8 @@
namespace {
-const size_t kBmpCoreHeaderSize = 12;
-const size_t kBmpInfoHeaderSize = 40;
+constexpr size_t kBmpCoreHeaderSize = 12;
+constexpr size_t kBmpInfoHeaderSize = 40;
static_assert(sizeof(BmpCoreHeader) == kBmpCoreHeaderSize,
"BmpCoreHeader has wrong size");
@@ -34,33 +34,9 @@
} // namespace
-CFX_BmpDecompressor::CFX_BmpDecompressor()
- : context_ptr_(nullptr),
- header_offset_(0),
- width_(0),
- height_(0),
- compress_flag_(0),
- components_(0),
- src_row_bytes_(0),
- out_row_bytes_(0),
- bit_counts_(0),
- color_used_(0),
- imgTB_flag_(false),
- pal_num_(0),
- pal_type_(0),
- data_size_(0),
- img_data_offset_(0),
- img_ifh_size_(0),
- row_num_(0),
- col_num_(0),
- dpi_x_(0),
- dpi_y_(0),
- mask_red_(0),
- mask_green_(0),
- mask_blue_(0),
- decode_status_(BMP_D_STATUS_HEADER) {}
+CFX_BmpDecompressor::CFX_BmpDecompressor() = default;
-CFX_BmpDecompressor::~CFX_BmpDecompressor() {}
+CFX_BmpDecompressor::~CFX_BmpDecompressor() = default;
void CFX_BmpDecompressor::Error() {
longjmp(jmpbuf_, 1);
@@ -68,11 +44,11 @@
void CFX_BmpDecompressor::ReadScanline(uint32_t row_num,
const std::vector<uint8_t>& row_buf) {
- context_ptr_->m_pDelegate->BmpReadScanline(row_num, row_buf);
+ context_->m_pDelegate->BmpReadScanline(row_num, row_buf);
}
bool CFX_BmpDecompressor::GetDataPosition(uint32_t rcd_pos) {
- return context_ptr_->m_pDelegate->BmpInputImagePositionBuf(rcd_pos);
+ return context_->m_pDelegate->BmpInputImagePositionBuf(rcd_pos);
}
int32_t CFX_BmpDecompressor::ReadHeader() {
@@ -118,8 +94,9 @@
bit_counts_ = FXWORD_GET_LSBFIRST(
reinterpret_cast<uint8_t*>(&bmp_core_header.bcBitCount));
compress_flag_ = BMP_RGB;
- imgTB_flag_ = false;
- } break;
+ img_tb_flag_ = false;
+ break;
+ }
case kBmpInfoHeaderSize: {
BmpInfoHeader bmp_info_header;
if (!ReadData(reinterpret_cast<uint8_t*>(&bmp_info_header),
@@ -142,7 +119,8 @@
dpi_y_ = static_cast<int32_t>(FXDWORD_GET_LSBFIRST(
reinterpret_cast<uint8_t*>(&bmp_info_header.biYPelsPerMeter)));
SetHeight(signed_height);
- } break;
+ break;
+ }
default: {
if (img_ifh_size_ <= sizeof(BmpInfoHeader)) {
Error();
@@ -165,7 +143,7 @@
if (!input_buffer_->Seek(new_pos.ValueOrDie()))
return 2;
- uint16_t biPlanes;
+ uint16_t bi_planes;
width_ = FXDWORD_GET_LSBFIRST(
reinterpret_cast<uint8_t*>(&bmp_info_header.biWidth));
int32_t signed_height = FXDWORD_GET_LSBFIRST(
@@ -176,17 +154,18 @@
reinterpret_cast<uint8_t*>(&bmp_info_header.biCompression));
color_used_ = FXDWORD_GET_LSBFIRST(
reinterpret_cast<uint8_t*>(&bmp_info_header.biClrUsed));
- biPlanes = FXWORD_GET_LSBFIRST(
+ bi_planes = FXWORD_GET_LSBFIRST(
reinterpret_cast<uint8_t*>(&bmp_info_header.biPlanes));
dpi_x_ = FXDWORD_GET_LSBFIRST(
reinterpret_cast<uint8_t*>(&bmp_info_header.biXPelsPerMeter));
dpi_y_ = FXDWORD_GET_LSBFIRST(
reinterpret_cast<uint8_t*>(&bmp_info_header.biYPelsPerMeter));
SetHeight(signed_height);
- if (compress_flag_ != BMP_RGB || biPlanes != 1 || color_used_ != 0) {
+ if (compress_flag_ != BMP_RGB || bi_planes != 1 || color_used_ != 0) {
Error();
NOTREACHED();
}
+ break;
}
}
@@ -242,66 +221,68 @@
out_row_buffer_.resize(out_row_bytes_);
SaveDecodingStatus(BMP_D_STATUS_PAL);
}
- if (decode_status_ == BMP_D_STATUS_PAL) {
- if (compress_flag_ == BMP_BITFIELDS) {
- if (bit_counts_ != 16 && bit_counts_ != 32) {
- Error();
- NOTREACHED();
- }
+ if (decode_status_ != BMP_D_STATUS_PAL)
+ return 1;
- uint32_t masks[3];
- if (!ReadData(reinterpret_cast<uint8_t*>(masks), sizeof(masks)))
- return 2;
-
- mask_red_ = FXDWORD_GET_LSBFIRST(reinterpret_cast<uint8_t*>(&masks[0]));
- mask_green_ = FXDWORD_GET_LSBFIRST(reinterpret_cast<uint8_t*>(&masks[1]));
- mask_blue_ = FXDWORD_GET_LSBFIRST(reinterpret_cast<uint8_t*>(&masks[2]));
- if (mask_red_ & mask_green_ || mask_red_ & mask_blue_ ||
- mask_green_ & mask_blue_) {
- Error();
- NOTREACHED();
- }
- header_offset_ = std::max(header_offset_, 26 + img_ifh_size_);
- SaveDecodingStatus(BMP_D_STATUS_DATA_PRE);
- return 1;
- } else if (bit_counts_ == 16) {
- mask_red_ = 0x7C00;
- mask_green_ = 0x03E0;
- mask_blue_ = 0x001F;
+ if (compress_flag_ == BMP_BITFIELDS) {
+ if (bit_counts_ != 16 && bit_counts_ != 32) {
+ Error();
+ NOTREACHED();
}
- pal_num_ = 0;
- if (bit_counts_ < 16) {
- pal_num_ = 1 << bit_counts_;
- if (color_used_ != 0)
- pal_num_ = color_used_;
- uint32_t src_pal_size = pal_num_ * (pal_type_ ? 3 : 4);
- std::vector<uint8_t> src_pal(src_pal_size);
- uint8_t* src_pal_data = src_pal.data();
- if (!ReadData(src_pal_data, src_pal_size)) {
- return 2;
- }
- palette_.resize(pal_num_);
- int32_t src_pal_index = 0;
- if (pal_type_ == BMP_PAL_OLD) {
- while (src_pal_index < pal_num_) {
- palette_[src_pal_index++] = BMP_PAL_ENCODE(
- 0x00, src_pal_data[2], src_pal_data[1], src_pal_data[0]);
- src_pal_data += 3;
- }
- } else {
- while (src_pal_index < pal_num_) {
- palette_[src_pal_index++] =
- BMP_PAL_ENCODE(src_pal_data[3], src_pal_data[2], src_pal_data[1],
- src_pal_data[0]);
- src_pal_data += 4;
- }
- }
+ uint32_t masks[3];
+ if (!ReadData(reinterpret_cast<uint8_t*>(masks), sizeof(masks)))
+ return 2;
+
+ mask_red_ = FXDWORD_GET_LSBFIRST(reinterpret_cast<uint8_t*>(&masks[0]));
+ mask_green_ = FXDWORD_GET_LSBFIRST(reinterpret_cast<uint8_t*>(&masks[1]));
+ mask_blue_ = FXDWORD_GET_LSBFIRST(reinterpret_cast<uint8_t*>(&masks[2]));
+ if (mask_red_ & mask_green_ || mask_red_ & mask_blue_ ||
+ mask_green_ & mask_blue_) {
+ Error();
+ NOTREACHED();
}
- header_offset_ = std::max(
- header_offset_, 14 + img_ifh_size_ + pal_num_ * (pal_type_ ? 3 : 4));
+ header_offset_ = std::max(header_offset_, 26 + img_ifh_size_);
SaveDecodingStatus(BMP_D_STATUS_DATA_PRE);
+ return 1;
}
+
+ if (bit_counts_ == 16) {
+ mask_red_ = 0x7C00;
+ mask_green_ = 0x03E0;
+ mask_blue_ = 0x001F;
+ }
+ pal_num_ = 0;
+ if (bit_counts_ < 16) {
+ pal_num_ = 1 << bit_counts_;
+ if (color_used_ != 0)
+ pal_num_ = color_used_;
+ uint32_t src_pal_size = pal_num_ * (pal_type_ ? 3 : 4);
+ std::vector<uint8_t> src_pal(src_pal_size);
+ uint8_t* src_pal_data = src_pal.data();
+ if (!ReadData(src_pal_data, src_pal_size)) {
+ return 2;
+ }
+
+ palette_.resize(pal_num_);
+ int32_t src_pal_index = 0;
+ if (pal_type_ == BMP_PAL_OLD) {
+ while (src_pal_index < pal_num_) {
+ palette_[src_pal_index++] = BMP_PAL_ENCODE(
+ 0x00, src_pal_data[2], src_pal_data[1], src_pal_data[0]);
+ src_pal_data += 3;
+ }
+ } else {
+ while (src_pal_index < pal_num_) {
+ palette_[src_pal_index++] = BMP_PAL_ENCODE(
+ src_pal_data[3], src_pal_data[2], src_pal_data[1], src_pal_data[0]);
+ src_pal_data += 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;
}
@@ -369,14 +350,16 @@
for (uint32_t col = 0; col < width_; ++col)
out_row_buffer_[idx++] =
dest_buf[col >> 3] & (0x80 >> (col % 8)) ? 0x01 : 0x00;
- } break;
+ break;
+ }
case 4: {
for (uint32_t col = 0; col < width_; ++col) {
out_row_buffer_[idx++] = (col & 0x01)
? (dest_buf[col >> 1] & 0x0F)
: ((dest_buf[col >> 1] & 0xF0) >> 4);
}
- } break;
+ break;
+ }
case 16: {
uint16_t* buf = reinterpret_cast<uint16_t*>(dest_buf.data());
uint8_t blue_bits = 0;
@@ -406,7 +389,8 @@
out_row_buffer_[idx++] =
static_cast<uint8_t>((*buf++ & mask_red_) >> red_bits);
}
- } break;
+ break;
+ }
case 8:
case 24:
case 32:
@@ -420,7 +404,7 @@
if (!ValidateColorIndex(byte))
return 0;
}
- ReadScanline(imgTB_flag_ ? row_num_++ : (height_ - 1 - row_num_++),
+ ReadScanline(img_tb_flag_ ? row_num_++ : (height_ - 1 - row_num_++),
out_row_buffer_);
}
SaveDecodingStatus(BMP_D_STATUS_TAIL);
@@ -447,7 +431,7 @@
NOTREACHED();
}
- ReadScanline(imgTB_flag_ ? row_num_++ : (height_ - 1 - row_num_++),
+ ReadScanline(img_tb_flag_ ? row_num_++ : (height_ - 1 - row_num_++),
out_row_buffer_);
col_num_ = 0;
std::fill(out_row_buffer_.begin(), out_row_buffer_.end(), 0);
@@ -457,7 +441,7 @@
case RLE_EOI: {
if (row_num_ < height_) {
ReadScanline(
- imgTB_flag_ ? row_num_++ : (height_ - 1 - row_num_++),
+ img_tb_flag_ ? row_num_++ : (height_ - 1 - row_num_++),
out_row_buffer_);
}
SaveDecodingStatus(BMP_D_STATUS_TAIL);
@@ -478,10 +462,11 @@
while (row_num_ < bmp_row_num__next) {
std::fill(out_row_buffer_.begin(), out_row_buffer_.end(), 0);
ReadScanline(
- imgTB_flag_ ? row_num_++ : (height_ - 1 - row_num_++),
+ img_tb_flag_ ? row_num_++ : (height_ - 1 - row_num_++),
out_row_buffer_);
}
- } break;
+ break;
+ }
default: {
int32_t avail_size = out_row_bytes_ - col_num_;
if (!avail_size || static_cast<int32_t>(first_part) > avail_size) {
@@ -505,7 +490,8 @@
col_num_ += first_part;
}
}
- } break;
+ break;
+ }
default: {
int32_t avail_size = out_row_bytes_ - col_num_;
if (!avail_size || static_cast<int32_t>(first_part) > avail_size) {
@@ -549,7 +535,7 @@
NOTREACHED();
}
- ReadScanline(imgTB_flag_ ? row_num_++ : (height_ - 1 - row_num_++),
+ ReadScanline(img_tb_flag_ ? row_num_++ : (height_ - 1 - row_num_++),
out_row_buffer_);
col_num_ = 0;
std::fill(out_row_buffer_.begin(), out_row_buffer_.end(), 0);
@@ -559,7 +545,7 @@
case RLE_EOI: {
if (row_num_ < height_) {
ReadScanline(
- imgTB_flag_ ? row_num_++ : (height_ - 1 - row_num_++),
+ img_tb_flag_ ? row_num_++ : (height_ - 1 - row_num_++),
out_row_buffer_);
}
SaveDecodingStatus(BMP_D_STATUS_TAIL);
@@ -580,10 +566,11 @@
while (row_num_ < bmp_row_num__next) {
std::fill(out_row_buffer_.begin(), out_row_buffer_.end(), 0);
ReadScanline(
- imgTB_flag_ ? row_num_++ : (height_ - 1 - row_num_++),
+ img_tb_flag_ ? row_num_++ : (height_ - 1 - row_num_++),
out_row_buffer_);
}
- } break;
+ break;
+ }
default: {
int32_t avail_size = out_row_bytes_ - col_num_;
if (!avail_size) {
@@ -615,7 +602,8 @@
}
}
}
- } break;
+ break;
+ }
default: {
int32_t avail_size = out_row_bytes_ - col_num_;
if (!avail_size) {
@@ -682,5 +670,5 @@
NOTREACHED();
}
height_ = -signed_height;
- imgTB_flag_ = true;
+ img_tb_flag_ = true;
}
diff --git a/core/fxcodec/bmp/cfx_bmpdecompressor.h b/core/fxcodec/bmp/cfx_bmpdecompressor.h
index c8c04a3..61bda4a 100644
--- a/core/fxcodec/bmp/cfx_bmpdecompressor.h
+++ b/core/fxcodec/bmp/cfx_bmpdecompressor.h
@@ -13,6 +13,7 @@
#include "core/fxcodec/bmp/fx_bmp.h"
#include "core/fxcrt/retain_ptr.h"
+#include "core/fxcrt/unowned_ptr.h"
class CFX_BmpContext;
class CFX_CodecMemory;
@@ -29,32 +30,31 @@
FX_FILESIZE GetAvailInput() const;
jmp_buf jmpbuf_;
- CFX_BmpContext* context_ptr_;
+ UnownedPtr<CFX_BmpContext> context_;
std::vector<uint8_t> out_row_buffer_;
std::vector<uint32_t> palette_;
- uint32_t header_offset_;
- uint32_t width_;
- uint32_t height_;
- uint32_t compress_flag_;
- int32_t components_;
- size_t src_row_bytes_;
- size_t out_row_bytes_;
- uint16_t bit_counts_;
- uint32_t color_used_;
- bool imgTB_flag_;
- int32_t pal_num_;
- int32_t pal_type_;
- uint32_t data_size_;
- uint32_t img_data_offset_;
- uint32_t img_ifh_size_;
- size_t row_num_;
- size_t col_num_;
- int32_t dpi_x_;
- int32_t dpi_y_;
- uint32_t mask_red_;
- uint32_t mask_green_;
- uint32_t mask_blue_;
- int32_t decode_status_;
+ uint32_t header_offset_ = 0;
+ uint32_t width_ = 0;
+ uint32_t height_ = 0;
+ uint32_t compress_flag_ = 0;
+ int32_t components_ = 0;
+ size_t src_row_bytes_ = 0;
+ size_t out_row_bytes_ = 0;
+ bool img_tb_flag_ = false;
+ uint16_t bit_counts_ = 0;
+ uint32_t color_used_ = 0;
+ int32_t pal_num_ = 0;
+ int32_t pal_type_ = 0;
+ uint32_t data_size_ = 0;
+ uint32_t img_ifh_size_ = 0;
+ size_t row_num_ = 0;
+ size_t col_num_ = 0;
+ int32_t dpi_x_ = 0;
+ int32_t dpi_y_ = 0;
+ uint32_t mask_red_ = 0;
+ uint32_t mask_green_ = 0;
+ uint32_t mask_blue_ = 0;
+ int32_t decode_status_ = BMP_D_STATUS_HEADER;
private:
bool GetDataPosition(uint32_t cur_pos);
diff --git a/core/fxcodec/codec/bmpmodule.cpp b/core/fxcodec/codec/bmpmodule.cpp
index 47a3dc4..495285c 100644
--- a/core/fxcodec/codec/bmpmodule.cpp
+++ b/core/fxcodec/codec/bmpmodule.cpp
@@ -24,7 +24,7 @@
std::unique_ptr<CodecModuleIface::Context> BmpModule::Start(
Delegate* pDelegate) {
auto p = pdfium::MakeUnique<CFX_BmpContext>(this, pDelegate);
- p->m_Bmp.context_ptr_ = p.get();
+ p->m_Bmp.context_ = p.get();
return p;
}
@@ -48,7 +48,7 @@
*width = ctx->m_Bmp.width_;
*height = ctx->m_Bmp.height_;
- *tb_flag = ctx->m_Bmp.imgTB_flag_;
+ *tb_flag = ctx->m_Bmp.img_tb_flag_;
*components = ctx->m_Bmp.components_;
*pal_num = ctx->m_Bmp.pal_num_;
*palette = ctx->m_Bmp.palette_;
