Tidy FXSYS_DWORD_GET_MSBFIRST() and friends.
We'd thought about converting these to functions taking (const uint8_t*)
arguments, but it might be nice to apply these to anything supporting a
operator[] (such as span<>), which would mean either templates, or just
leaving them as macros.
-- Rename WORD/DWORD to UINT16/UINT32 to avoid those "word" words.
-- Parenthesize to allow e.g. GET(ptr + 10)
-- Perform unsigned shifts only (uint16_t silently promotes to int).
-- De-duplicate implementation from test file.
-- Use in cfx_cttgsubtable.cpp
Change-Id: I7dde49860ccd7cd6431eeea4647c1533a07bc24a
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/82975
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Tom Sepez <tsepez@chromium.org>
diff --git a/core/fpdfapi/font/cfx_cttgsubtable.cpp b/core/fpdfapi/font/cfx_cttgsubtable.cpp
index cb96bf4..bad7f71 100644
--- a/core/fpdfapi/font/cfx_cttgsubtable.cpp
+++ b/core/fpdfapi/font/cfx_cttgsubtable.cpp
@@ -8,6 +8,7 @@
#include <utility>
+#include "core/fxcrt/fx_system.h"
#include "core/fxcrt/stl_util.h"
#include "core/fxge/cfx_fontmapper.h"
@@ -49,11 +50,12 @@
CFX_CTTGSUBTable::~CFX_CTTGSUBTable() = default;
bool CFX_CTTGSUBTable::LoadGSUBTable(FT_Bytes gsub) {
- if ((gsub[0] << 24u | gsub[1] << 16u | gsub[2] << 8u | gsub[3]) != 0x00010000)
+ if (FXSYS_UINT32_GET_MSBFIRST(gsub) != 0x00010000)
return false;
- return Parse(&gsub[gsub[4] << 8 | gsub[5]], &gsub[gsub[6] << 8 | gsub[7]],
- &gsub[gsub[8] << 8 | gsub[9]]);
+ return Parse(&gsub[FXSYS_UINT16_GET_MSBFIRST(gsub + 4)],
+ &gsub[FXSYS_UINT16_GET_MSBFIRST(gsub + 6)],
+ &gsub[FXSYS_UINT16_GET_MSBFIRST(gsub + 8)]);
}
uint32_t CFX_CTTGSUBTable::GetVerticalGlyph(uint32_t glyphnum) const {
diff --git a/core/fpdfapi/parser/cpdf_security_handler.cpp b/core/fpdfapi/parser/cpdf_security_handler.cpp
index 2803f15..b54b707 100644
--- a/core/fpdfapi/parser/cpdf_security_handler.cpp
+++ b/core/fpdfapi/parser/cpdf_security_handler.cpp
@@ -385,7 +385,7 @@
if (buf[9] != 'a' || buf[10] != 'd' || buf[11] != 'b')
return false;
- if (FXSYS_DWORD_GET_LSBFIRST(buf) != m_Permissions)
+ if (FXSYS_UINT32_GET_LSBFIRST(buf) != m_Permissions)
return false;
// Relax this check as there appear to be some non-conforming documents
diff --git a/core/fxcodec/bmp/cfx_bmpdecompressor.cpp b/core/fxcodec/bmp/cfx_bmpdecompressor.cpp
index e2f2fc6..eaa8f67 100644
--- a/core/fxcodec/bmp/cfx_bmpdecompressor.cpp
+++ b/core/fxcodec/bmp/cfx_bmpdecompressor.cpp
@@ -93,11 +93,11 @@
}
bmp_header.bfType =
- FXSYS_WORD_GET_LSBFIRST(reinterpret_cast<uint8_t*>(&bmp_header.bfType));
- bmp_header.bfOffBits = FXSYS_DWORD_GET_LSBFIRST(
+ FXSYS_UINT16_GET_LSBFIRST(reinterpret_cast<uint8_t*>(&bmp_header.bfType));
+ bmp_header.bfOffBits = FXSYS_UINT32_GET_LSBFIRST(
reinterpret_cast<uint8_t*>(&bmp_header.bfOffBits));
data_size_ =
- FXSYS_DWORD_GET_LSBFIRST(reinterpret_cast<uint8_t*>(&bmp_header.bfSize));
+ FXSYS_UINT32_GET_LSBFIRST(reinterpret_cast<uint8_t*>(&bmp_header.bfSize));
if (bmp_header.bfType != kBmpSignature)
return BmpDecoder::Status::kFail;
@@ -110,7 +110,7 @@
return BmpDecoder::Status::kFail;
img_ifh_size_ =
- FXSYS_DWORD_GET_LSBFIRST(reinterpret_cast<uint8_t*>(&img_ifh_size_));
+ FXSYS_UINT32_GET_LSBFIRST(reinterpret_cast<uint8_t*>(&img_ifh_size_));
pal_type_ = 0;
BmpDecoder::Status status = ReadBmpHeaderIfh();
if (status != BmpDecoder::Status::kSuccess)
@@ -128,11 +128,11 @@
return BmpDecoder::Status::kContinue;
}
- width_ = FXSYS_WORD_GET_LSBFIRST(
+ width_ = FXSYS_UINT16_GET_LSBFIRST(
reinterpret_cast<uint8_t*>(&bmp_core_header.bcWidth));
- height_ = FXSYS_WORD_GET_LSBFIRST(
+ height_ = FXSYS_UINT16_GET_LSBFIRST(
reinterpret_cast<uint8_t*>(&bmp_core_header.bcHeight));
- bit_counts_ = FXSYS_WORD_GET_LSBFIRST(
+ bit_counts_ = FXSYS_UINT16_GET_LSBFIRST(
reinterpret_cast<uint8_t*>(&bmp_core_header.bcBitCount));
compress_flag_ = kBmpRgb;
img_tb_flag_ = false;
@@ -146,19 +146,19 @@
return BmpDecoder::Status::kContinue;
}
- width_ = FXSYS_DWORD_GET_LSBFIRST(
+ width_ = FXSYS_UINT32_GET_LSBFIRST(
reinterpret_cast<uint8_t*>(&bmp_info_header.biWidth));
- int32_t signed_height = FXSYS_DWORD_GET_LSBFIRST(
+ int32_t signed_height = FXSYS_UINT32_GET_LSBFIRST(
reinterpret_cast<uint8_t*>(&bmp_info_header.biHeight));
- bit_counts_ = FXSYS_WORD_GET_LSBFIRST(
+ bit_counts_ = FXSYS_UINT16_GET_LSBFIRST(
reinterpret_cast<uint8_t*>(&bmp_info_header.biBitCount));
- compress_flag_ = FXSYS_DWORD_GET_LSBFIRST(
+ compress_flag_ = FXSYS_UINT32_GET_LSBFIRST(
reinterpret_cast<uint8_t*>(&bmp_info_header.biCompression));
- color_used_ = FXSYS_DWORD_GET_LSBFIRST(
+ color_used_ = FXSYS_UINT32_GET_LSBFIRST(
reinterpret_cast<uint8_t*>(&bmp_info_header.biClrUsed));
- dpi_x_ = static_cast<int32_t>(FXSYS_DWORD_GET_LSBFIRST(
+ dpi_x_ = static_cast<int32_t>(FXSYS_UINT32_GET_LSBFIRST(
reinterpret_cast<uint8_t*>(&bmp_info_header.biXPelsPerMeter)));
- dpi_y_ = static_cast<int32_t>(FXSYS_DWORD_GET_LSBFIRST(
+ dpi_y_ = static_cast<int32_t>(FXSYS_UINT32_GET_LSBFIRST(
reinterpret_cast<uint8_t*>(&bmp_info_header.biYPelsPerMeter)));
if (!SetHeight(signed_height))
return BmpDecoder::Status::kFail;
@@ -183,21 +183,21 @@
return BmpDecoder::Status::kContinue;
uint16_t bi_planes;
- width_ = FXSYS_DWORD_GET_LSBFIRST(
+ width_ = FXSYS_UINT32_GET_LSBFIRST(
reinterpret_cast<uint8_t*>(&bmp_info_header.biWidth));
- int32_t signed_height = FXSYS_DWORD_GET_LSBFIRST(
+ int32_t signed_height = FXSYS_UINT32_GET_LSBFIRST(
reinterpret_cast<uint8_t*>(&bmp_info_header.biHeight));
- bit_counts_ = FXSYS_WORD_GET_LSBFIRST(
+ bit_counts_ = FXSYS_UINT16_GET_LSBFIRST(
reinterpret_cast<uint8_t*>(&bmp_info_header.biBitCount));
- compress_flag_ = FXSYS_DWORD_GET_LSBFIRST(
+ compress_flag_ = FXSYS_UINT32_GET_LSBFIRST(
reinterpret_cast<uint8_t*>(&bmp_info_header.biCompression));
- color_used_ = FXSYS_DWORD_GET_LSBFIRST(
+ color_used_ = FXSYS_UINT32_GET_LSBFIRST(
reinterpret_cast<uint8_t*>(&bmp_info_header.biClrUsed));
- bi_planes = FXSYS_WORD_GET_LSBFIRST(
+ bi_planes = FXSYS_UINT16_GET_LSBFIRST(
reinterpret_cast<uint8_t*>(&bmp_info_header.biPlanes));
- dpi_x_ = FXSYS_DWORD_GET_LSBFIRST(
+ dpi_x_ = FXSYS_UINT32_GET_LSBFIRST(
reinterpret_cast<uint8_t*>(&bmp_info_header.biXPelsPerMeter));
- dpi_y_ = FXSYS_DWORD_GET_LSBFIRST(
+ dpi_y_ = FXSYS_UINT32_GET_LSBFIRST(
reinterpret_cast<uint8_t*>(&bmp_info_header.biYPelsPerMeter));
if (!SetHeight(signed_height))
return BmpDecoder::Status::kFail;
@@ -273,9 +273,10 @@
if (!ReadData(reinterpret_cast<uint8_t*>(masks), sizeof(masks)))
return BmpDecoder::Status::kContinue;
- mask_red_ = FXSYS_DWORD_GET_LSBFIRST(reinterpret_cast<uint8_t*>(&masks[0]));
- mask_green_ = FXSYS_DWORD_GET_LSBFIRST(reinterpret_cast<uint8_t*>(&masks[1]));
- mask_blue_ = FXSYS_DWORD_GET_LSBFIRST(reinterpret_cast<uint8_t*>(&masks[2]));
+ mask_red_ = FXSYS_UINT32_GET_LSBFIRST(reinterpret_cast<uint8_t*>(&masks[0]));
+ mask_green_ =
+ FXSYS_UINT32_GET_LSBFIRST(reinterpret_cast<uint8_t*>(&masks[1]));
+ mask_blue_ = FXSYS_UINT32_GET_LSBFIRST(reinterpret_cast<uint8_t*>(&masks[2]));
if (mask_red_ & mask_green_ || mask_red_ & mask_blue_ ||
mask_green_ & mask_blue_) {
return BmpDecoder::Status::kFail;
@@ -411,7 +412,7 @@
green_bits -= 8;
red_bits -= 8;
for (uint32_t col = 0; col < width_; ++col) {
- *buf = FXSYS_WORD_GET_LSBFIRST(reinterpret_cast<uint8_t*>(buf));
+ *buf = FXSYS_UINT16_GET_LSBFIRST(reinterpret_cast<uint8_t*>(buf));
out_row_buffer_[idx++] =
static_cast<uint8_t>((*buf & mask_blue_) << blue_bits);
out_row_buffer_[idx++] =
diff --git a/core/fxcodec/gif/cfx_gifcontext.cpp b/core/fxcodec/gif/cfx_gifcontext.cpp
index 30e4038..cde9bec 100644
--- a/core/fxcodec/gif/cfx_gifcontext.cpp
+++ b/core/fxcodec/gif/cfx_gifcontext.cpp
@@ -387,9 +387,9 @@
}
width_ = static_cast<int>(
- FXSYS_WORD_GET_LSBFIRST(reinterpret_cast<uint8_t*>(&lsd.width)));
+ FXSYS_UINT16_GET_LSBFIRST(reinterpret_cast<uint8_t*>(&lsd.width)));
height_ = static_cast<int>(
- FXSYS_WORD_GET_LSBFIRST(reinterpret_cast<uint8_t*>(&lsd.height)));
+ FXSYS_UINT16_GET_LSBFIRST(reinterpret_cast<uint8_t*>(&lsd.height)));
return GifDecoder::Status::kSuccess;
}
@@ -431,7 +431,7 @@
std::make_unique<CFX_GifGraphicControlExtension>();
graphic_control_extension_->block_size = gif_gce.block_size;
graphic_control_extension_->gce_flags = gif_gce.gce_flags;
- graphic_control_extension_->delay_time = FXSYS_WORD_GET_LSBFIRST(
+ graphic_control_extension_->delay_time = FXSYS_UINT16_GET_LSBFIRST(
reinterpret_cast<uint8_t*>(&gif_gce.delay_time));
graphic_control_extension_->trans_index = gif_gce.trans_index;
break;
@@ -461,13 +461,13 @@
auto gif_image = std::make_unique<CFX_GifImage>();
gif_image->image_info.left =
- FXSYS_WORD_GET_LSBFIRST(reinterpret_cast<uint8_t*>(&img_info.left));
+ FXSYS_UINT16_GET_LSBFIRST(reinterpret_cast<uint8_t*>(&img_info.left));
gif_image->image_info.top =
- FXSYS_WORD_GET_LSBFIRST(reinterpret_cast<uint8_t*>(&img_info.top));
+ FXSYS_UINT16_GET_LSBFIRST(reinterpret_cast<uint8_t*>(&img_info.top));
gif_image->image_info.width =
- FXSYS_WORD_GET_LSBFIRST(reinterpret_cast<uint8_t*>(&img_info.width));
+ FXSYS_UINT16_GET_LSBFIRST(reinterpret_cast<uint8_t*>(&img_info.width));
gif_image->image_info.height =
- FXSYS_WORD_GET_LSBFIRST(reinterpret_cast<uint8_t*>(&img_info.height));
+ FXSYS_UINT16_GET_LSBFIRST(reinterpret_cast<uint8_t*>(&img_info.height));
gif_image->image_info.local_flags = img_info.local_flags;
if (gif_image->image_info.left + gif_image->image_info.width > width_ ||
gif_image->image_info.top + gif_image->image_info.height > height_)
diff --git a/core/fxcrt/byteorder_unittest.cpp b/core/fxcrt/byteorder_unittest.cpp
index 7b86acf..1133a8a 100644
--- a/core/fxcrt/byteorder_unittest.cpp
+++ b/core/fxcrt/byteorder_unittest.cpp
@@ -4,25 +4,11 @@
#include "core/fxcrt/byteorder.h"
+#include "core/fxcrt/fx_system.h"
#include "testing/gtest/include/gtest/gtest.h"
namespace {
-// Original code to use as a reference implementation.
-
-#define FXSYS_WORD_GET_LSBFIRST(p) \
- (static_cast<uint16_t>((static_cast<uint16_t>(p[1]) << 8) | \
- (static_cast<uint16_t>(p[0]))))
-#define FXSYS_WORD_GET_MSBFIRST(p) \
- (static_cast<uint16_t>((static_cast<uint16_t>(p[0]) << 8) | \
- (static_cast<uint16_t>(p[1]))))
-#define FXSYS_DWORD_GET_LSBFIRST(p) \
- ((static_cast<uint32_t>(p[3]) << 24) | (static_cast<uint32_t>(p[2]) << 16) | \
- (static_cast<uint32_t>(p[1]) << 8) | (static_cast<uint32_t>(p[0])))
-#define FXSYS_DWORD_GET_MSBFIRST(p) \
- ((static_cast<uint32_t>(p[0]) << 24) | (static_cast<uint32_t>(p[1]) << 16) | \
- (static_cast<uint32_t>(p[2]) << 8) | (static_cast<uint32_t>(p[3])))
-
constexpr uint32_t kTestValues32[] = {
0x0, 0x1, 0x2, 0x3, 0x4, 0xfe,
0xff, 0x100, 0x101, 0xffff, 0x10000, 0x123456,
@@ -37,7 +23,7 @@
for (uint32_t v = 0; v < 0x10000; ++v) {
const uint16_t v16 = v;
uint16_t expected =
- FXSYS_WORD_GET_LSBFIRST(reinterpret_cast<const uint8_t*>(&v16));
+ FXSYS_UINT16_GET_LSBFIRST(reinterpret_cast<const uint8_t*>(&v16));
EXPECT_EQ(expected, ByteSwapToLE16(v16)) << v;
}
}
@@ -45,7 +31,7 @@
TEST(ByteOrder, ByteSwapToLE32) {
for (uint32_t v : kTestValues32) {
uint32_t expected =
- FXSYS_DWORD_GET_LSBFIRST(reinterpret_cast<const uint8_t*>(&v));
+ FXSYS_UINT32_GET_LSBFIRST(reinterpret_cast<const uint8_t*>(&v));
EXPECT_EQ(expected, ByteSwapToLE32(v)) << v;
}
}
@@ -55,7 +41,7 @@
for (uint32_t v = 0; v < 0x10000; ++v) {
const uint16_t v16 = v;
uint16_t expected =
- FXSYS_WORD_GET_MSBFIRST(reinterpret_cast<const uint8_t*>(&v16));
+ FXSYS_UINT16_GET_MSBFIRST(reinterpret_cast<const uint8_t*>(&v16));
EXPECT_EQ(expected, ByteSwapToBE16(v16)) << v;
}
}
@@ -63,7 +49,7 @@
TEST(ByteOrder, ByteSwapToBE32) {
for (uint32_t v : kTestValues32) {
uint32_t expected =
- FXSYS_DWORD_GET_MSBFIRST(reinterpret_cast<const uint8_t*>(&v));
+ FXSYS_UINT32_GET_MSBFIRST(reinterpret_cast<const uint8_t*>(&v));
EXPECT_EQ(expected, ByteSwapToBE32(v)) << v;
}
}
diff --git a/core/fxcrt/fx_system.h b/core/fxcrt/fx_system.h
index c17df4d..29b3c56 100644
--- a/core/fxcrt/fx_system.h
+++ b/core/fxcrt/fx_system.h
@@ -127,18 +127,21 @@
uint32_t FXSYS_GetLastError();
#endif // defined(OS_WIN)
-#define FXSYS_WORD_GET_LSBFIRST(p) \
- (static_cast<uint16_t>((static_cast<uint16_t>(p[1]) << 8) | \
- (static_cast<uint16_t>(p[0]))))
-#define FXSYS_WORD_GET_MSBFIRST(p) \
- (static_cast<uint16_t>((static_cast<uint16_t>(p[0]) << 8) | \
- (static_cast<uint16_t>(p[1]))))
-#define FXSYS_DWORD_GET_LSBFIRST(p) \
- ((static_cast<uint32_t>(p[3]) << 24) | (static_cast<uint32_t>(p[2]) << 16) | \
- (static_cast<uint32_t>(p[1]) << 8) | (static_cast<uint32_t>(p[0])))
-#define FXSYS_DWORD_GET_MSBFIRST(p) \
- ((static_cast<uint32_t>(p[0]) << 24) | (static_cast<uint32_t>(p[1]) << 16) | \
- (static_cast<uint32_t>(p[2]) << 8) | (static_cast<uint32_t>(p[3])))
+#define FXSYS_UINT16_GET_LSBFIRST(p) \
+ (static_cast<uint16_t>((static_cast<uint32_t>((p)[1]) << 8) | \
+ (static_cast<uint32_t>((p)[0]))))
+#define FXSYS_UINT16_GET_MSBFIRST(p) \
+ (static_cast<uint16_t>((static_cast<uint32_t>((p)[0]) << 8) | \
+ (static_cast<uint32_t>((p)[1]))))
+#define FXSYS_UINT32_GET_LSBFIRST(p) \
+ ((static_cast<uint32_t>((p)[3]) << 24) | \
+ (static_cast<uint32_t>((p)[2]) << 16) | \
+ (static_cast<uint32_t>((p)[1]) << 8) | (static_cast<uint32_t>((p)[0])))
+#define FXSYS_UINT32_GET_MSBFIRST(p) \
+ ((static_cast<uint32_t>((p)[0]) << 24) | \
+ (static_cast<uint32_t>((p)[1]) << 16) | \
+ (static_cast<uint32_t>((p)[2]) << 8) | (static_cast<uint32_t>((p)[3])))
+
int32_t FXSYS_atoi(const char* str);
uint32_t FXSYS_atoui(const char* str);
int32_t FXSYS_wtoi(const wchar_t* str);
@@ -147,6 +150,7 @@
int FXSYS_roundf(float f);
int FXSYS_round(double d);
float FXSYS_sqrt2(float a, float b);
+
#ifdef __cplusplus
} // extern C
#endif // __cplusplus
diff --git a/core/fxge/win32/cwin32_platform.cpp b/core/fxge/win32/cwin32_platform.cpp
index b609f1f..743133c 100644
--- a/core/fxge/win32/cwin32_platform.cpp
+++ b/core/fxge/win32/cwin32_platform.cpp
@@ -394,7 +394,7 @@
uint32_t table,
pdfium::span<uint8_t> buffer) {
HFONT hOldFont = (HFONT)::SelectObject(m_hDC, (HFONT)hFont);
- table = FXSYS_DWORD_GET_MSBFIRST(reinterpret_cast<uint8_t*>(&table));
+ table = FXSYS_UINT32_GET_MSBFIRST(reinterpret_cast<uint8_t*>(&table));
uint32_t size = ::GetFontData(m_hDC, table, 0, buffer.data(), buffer.size());
::SelectObject(m_hDC, hOldFont);
if (size == GDI_ERROR) {
