From 1881cb7591c7154c65efaa62a8ead989113a6c23 Mon Sep 17 00:00:00 2001 From: Tom Sepez Date: Thu, 6 Sep 2018 22:07:10 +0000 Subject: Rename CFX_GifContext::ReadData() to ReadAllOrNone() The whole point of this wrapper method is to disallow partial reads, so name it accordingly. No functional change. Change-Id: Ia2e7ee756192615f399369d5b077b836438befb2 Reviewed-on: https://pdfium-review.googlesource.com/42150 Reviewed-by: Henrique Nakashima Commit-Queue: Tom Sepez --- core/fxcodec/gif/cfx_gifcontext.cpp | 41 ++++++++++++++-------------- core/fxcodec/gif/cfx_gifcontext.h | 2 +- core/fxcodec/gif/cfx_gifcontext_unittest.cpp | 28 +++++++++---------- 3 files changed, 36 insertions(+), 35 deletions(-) diff --git a/core/fxcodec/gif/cfx_gifcontext.cpp b/core/fxcodec/gif/cfx_gifcontext.cpp index 8f48cfde48..c1467ae233 100644 --- a/core/fxcodec/gif/cfx_gifcontext.cpp +++ b/core/fxcodec/gif/cfx_gifcontext.cpp @@ -78,7 +78,7 @@ CFX_GifDecodeStatus CFX_GifContext::GetFrame() { return CFX_GifDecodeStatus::Success; case GIF_D_STATUS_SIG: { uint8_t signature; - if (!ReadData(&signature, sizeof(signature))) + if (!ReadAllOrNone(&signature, sizeof(signature))) return CFX_GifDecodeStatus::Unfinished; switch (signature) { @@ -103,7 +103,7 @@ CFX_GifDecodeStatus CFX_GifContext::GetFrame() { } case GIF_D_STATUS_EXT: { uint8_t extension; - if (!ReadData(&extension, sizeof(extension))) + if (!ReadAllOrNone(&extension, sizeof(extension))) return CFX_GifDecodeStatus::Unfinished; switch (extension) { @@ -137,7 +137,7 @@ CFX_GifDecodeStatus CFX_GifContext::GetFrame() { uint8_t img_data_size; size_t read_marker = input_buffer_->GetPosition(); - if (!ReadData(&img_data_size, sizeof(img_data_size))) + if (!ReadAllOrNone(&img_data_size, sizeof(img_data_size))) return CFX_GifDecodeStatus::Unfinished; while (img_data_size != GIF_BLOCK_TERMINAL) { @@ -151,7 +151,7 @@ CFX_GifDecodeStatus CFX_GifContext::GetFrame() { // ScanForTerminalMarker() cannot be used here. SaveDecodingStatus(GIF_D_STATUS_IMG_DATA); read_marker = input_buffer_->GetPosition(); - if (!ReadData(&img_data_size, sizeof(img_data_size))) + if (!ReadAllOrNone(&img_data_size, sizeof(img_data_size))) return CFX_GifDecodeStatus::Unfinished; } @@ -236,12 +236,12 @@ CFX_GifDecodeStatus CFX_GifContext::LoadFrame(int32_t frame_num) { size_t read_marker = input_buffer_->GetPosition(); if (decode_status_ == GIF_D_STATUS_IMG_DATA) { - if (!ReadData(&img_data_size, sizeof(img_data_size))) + if (!ReadAllOrNone(&img_data_size, sizeof(img_data_size))) return CFX_GifDecodeStatus::Unfinished; if (img_data_size != GIF_BLOCK_TERMINAL) { img_data.resize(img_data_size); - if (!ReadData(img_data.data(), img_data_size)) { + if (!ReadAllOrNone(img_data.data(), img_data_size)) { input_buffer_->Seek(read_marker); return CFX_GifDecodeStatus::Unfinished; } @@ -276,12 +276,12 @@ CFX_GifDecodeStatus CFX_GifContext::LoadFrame(int32_t frame_num) { if (ret == CFX_GifDecodeStatus::Unfinished) { read_marker = input_buffer_->GetPosition(); - if (!ReadData(&img_data_size, sizeof(img_data_size))) + if (!ReadAllOrNone(&img_data_size, sizeof(img_data_size))) return CFX_GifDecodeStatus::Unfinished; if (img_data_size != GIF_BLOCK_TERMINAL) { img_data.resize(img_data_size); - if (!ReadData(img_data.data(), img_data_size)) { + if (!ReadAllOrNone(img_data.data(), img_data_size)) { input_buffer_->Seek(read_marker); return CFX_GifDecodeStatus::Unfinished; } @@ -353,7 +353,7 @@ uint32_t CFX_GifContext::GetAvailInput() const { return input_buffer_->GetSize() - input_buffer_->GetPosition(); } -bool CFX_GifContext::ReadData(uint8_t* dest, uint32_t size) { +bool CFX_GifContext::ReadAllOrNone(uint8_t* dest, uint32_t size) { if (!input_buffer_ || !dest) return false; @@ -369,7 +369,7 @@ bool CFX_GifContext::ReadData(uint8_t* dest, uint32_t size) { CFX_GifDecodeStatus CFX_GifContext::ReadGifSignature() { CFX_GifHeader header; - if (!ReadData(reinterpret_cast(&header), 6)) + if (!ReadAllOrNone(reinterpret_cast(&header), 6)) return CFX_GifDecodeStatus::Unfinished; if (strncmp(header.signature, kGifSignature87, 6) != 0 && @@ -384,7 +384,7 @@ CFX_GifDecodeStatus CFX_GifContext::ReadLogicalScreenDescriptor() { CFX_GifLocalScreenDescriptor lsd; size_t read_marker = input_buffer_->GetPosition(); - if (!ReadData(reinterpret_cast(&lsd), sizeof(lsd))) + if (!ReadAllOrNone(reinterpret_cast(&lsd), sizeof(lsd))) return CFX_GifDecodeStatus::Unfinished; if (lsd.global_flags.global_pal) { @@ -395,7 +395,8 @@ CFX_GifDecodeStatus CFX_GifContext::ReadLogicalScreenDescriptor() { uint32_t palette_size = palette_count * sizeof(CFX_GifPalette); std::vector palette(palette_count); - if (!ReadData(reinterpret_cast(palette.data()), palette_size)) { + if (!ReadAllOrNone(reinterpret_cast(palette.data()), + palette_size)) { // Roll back the read for the LSD input_buffer_->Seek(read_marker); return CFX_GifDecodeStatus::Unfinished; @@ -433,7 +434,7 @@ CFX_GifDecodeStatus CFX_GifContext::DecodeExtension() { } case GIF_D_STATUS_EXT_PTE: { CFX_GifPlainTextExtension gif_pte; - if (!ReadData(reinterpret_cast(&gif_pte), sizeof(gif_pte))) + if (!ReadAllOrNone(reinterpret_cast(&gif_pte), sizeof(gif_pte))) return CFX_GifDecodeStatus::Unfinished; graphic_control_extension_ = nullptr; @@ -445,7 +446,7 @@ CFX_GifDecodeStatus CFX_GifContext::DecodeExtension() { } case GIF_D_STATUS_EXT_GCE: { CFX_GifGraphicControlExtension gif_gce; - if (!ReadData(reinterpret_cast(&gif_gce), sizeof(gif_gce))) + if (!ReadAllOrNone(reinterpret_cast(&gif_gce), sizeof(gif_gce))) return CFX_GifDecodeStatus::Unfinished; if (!graphic_control_extension_.get()) @@ -478,7 +479,7 @@ CFX_GifDecodeStatus CFX_GifContext::DecodeImageInfo() { size_t read_marker = input_buffer_->GetPosition(); CFX_CFX_GifImageInfo img_info; - if (!ReadData(reinterpret_cast(&img_info), sizeof(img_info))) + if (!ReadAllOrNone(reinterpret_cast(&img_info), sizeof(img_info))) return CFX_GifDecodeStatus::Unfinished; auto gif_image = pdfium::MakeUnique(); @@ -500,8 +501,8 @@ CFX_GifDecodeStatus CFX_GifContext::DecodeImageInfo() { gif_image->local_pallette_exp = gif_img_info_lf->pal_bits; uint32_t loc_pal_count = unsigned(2 << gif_img_info_lf->pal_bits); std::vector loc_pal(loc_pal_count); - if (!ReadData(reinterpret_cast(loc_pal.data()), - loc_pal_count * sizeof(CFX_GifPalette))) { + if (!ReadAllOrNone(reinterpret_cast(loc_pal.data()), + loc_pal_count * sizeof(CFX_GifPalette))) { input_buffer_->Seek(read_marker); return CFX_GifDecodeStatus::Unfinished; } @@ -510,7 +511,7 @@ CFX_GifDecodeStatus CFX_GifContext::DecodeImageInfo() { } uint8_t code_size; - if (!ReadData(&code_size, sizeof(code_size))) { + if (!ReadAllOrNone(&code_size, sizeof(code_size))) { input_buffer_->Seek(read_marker); return CFX_GifDecodeStatus::Unfinished; } @@ -546,12 +547,12 @@ void CFX_GifContext::DecodingFailureAtTailCleanup(CFX_GifImage* gif_image) { bool CFX_GifContext::ScanForTerminalMarker() { uint8_t data_size; - if (!ReadData(&data_size, sizeof(data_size))) + if (!ReadAllOrNone(&data_size, sizeof(data_size))) return false; while (data_size != GIF_BLOCK_TERMINAL) { if (!input_buffer_->Seek(input_buffer_->GetPosition() + data_size) || - !ReadData(&data_size, sizeof(data_size))) { + !ReadAllOrNone(&data_size, sizeof(data_size))) { return false; } } diff --git a/core/fxcodec/gif/cfx_gifcontext.h b/core/fxcodec/gif/cfx_gifcontext.h index 9cc7da9615..5eae5163cc 100644 --- a/core/fxcodec/gif/cfx_gifcontext.h +++ b/core/fxcodec/gif/cfx_gifcontext.h @@ -64,7 +64,7 @@ class CFX_GifContext : public CCodec_GifModule::Context { uint8_t img_pass_num_; protected: - bool ReadData(uint8_t* dest, uint32_t size); + bool ReadAllOrNone(uint8_t* dest, uint32_t size); CFX_GifDecodeStatus ReadGifSignature(); CFX_GifDecodeStatus ReadLogicalScreenDescriptor(); diff --git a/core/fxcodec/gif/cfx_gifcontext_unittest.cpp b/core/fxcodec/gif/cfx_gifcontext_unittest.cpp index 25f12b2724..b9805ef399 100644 --- a/core/fxcodec/gif/cfx_gifcontext_unittest.cpp +++ b/core/fxcodec/gif/cfx_gifcontext_unittest.cpp @@ -14,7 +14,7 @@ class CFX_GifContextForTest final : public CFX_GifContext { : CFX_GifContext(gif_module, delegate) {} ~CFX_GifContextForTest() override {} - using CFX_GifContext::ReadData; + using CFX_GifContext::ReadAllOrNone; using CFX_GifContext::ReadGifSignature; using CFX_GifContext::ReadLogicalScreenDescriptor; @@ -54,7 +54,7 @@ TEST(CFX_GifContext, SetInputBuffer) { } } -TEST(CFX_GifContext, ReadData) { +TEST(CFX_GifContext, ReadAllOrNone) { std::vector dest_buffer; uint8_t src_buffer[] = {0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09}; @@ -63,33 +63,33 @@ TEST(CFX_GifContext, ReadData) { CFX_GifContextForTest context(nullptr, nullptr); context.SetInputBuffer({nullptr, 0}); - EXPECT_FALSE(context.ReadData(nullptr, 0)); - EXPECT_FALSE(context.ReadData(nullptr, 10)); + EXPECT_FALSE(context.ReadAllOrNone(nullptr, 0)); + EXPECT_FALSE(context.ReadAllOrNone(nullptr, 10)); - EXPECT_FALSE(context.ReadData(dest_buffer.data(), 0)); - EXPECT_FALSE(context.ReadData(dest_buffer.data(), 10)); + EXPECT_FALSE(context.ReadAllOrNone(dest_buffer.data(), 0)); + EXPECT_FALSE(context.ReadAllOrNone(dest_buffer.data(), 10)); context.SetInputBuffer({src_buffer, 0}); dest_buffer.resize(sizeof(src_buffer)); - EXPECT_FALSE(context.ReadData(dest_buffer.data(), sizeof(src_buffer))); + EXPECT_FALSE(context.ReadAllOrNone(dest_buffer.data(), sizeof(src_buffer))); context.SetInputBuffer({src_buffer, 1}); - EXPECT_FALSE(context.ReadData(dest_buffer.data(), sizeof(src_buffer))); + EXPECT_FALSE(context.ReadAllOrNone(dest_buffer.data(), sizeof(src_buffer))); EXPECT_EQ(0u, context.InputBuffer()->GetPosition()); - EXPECT_FALSE(context.ReadData(nullptr, sizeof(src_buffer))); - EXPECT_FALSE(context.ReadData(nullptr, 1)); - EXPECT_TRUE(context.ReadData(dest_buffer.data(), 1)); + EXPECT_FALSE(context.ReadAllOrNone(nullptr, sizeof(src_buffer))); + EXPECT_FALSE(context.ReadAllOrNone(nullptr, 1)); + EXPECT_TRUE(context.ReadAllOrNone(dest_buffer.data(), 1)); EXPECT_EQ(src_buffer[0], dest_buffer[0]); context.SetInputBuffer(src_buffer); - EXPECT_FALSE(context.ReadData(nullptr, sizeof(src_buffer))); - EXPECT_TRUE(context.ReadData(dest_buffer.data(), sizeof(src_buffer))); + EXPECT_FALSE(context.ReadAllOrNone(nullptr, sizeof(src_buffer))); + EXPECT_TRUE(context.ReadAllOrNone(dest_buffer.data(), sizeof(src_buffer))); for (size_t i = 0; i < sizeof(src_buffer); i++) EXPECT_EQ(src_buffer[i], dest_buffer[i]); context.SetInputBuffer(src_buffer); for (size_t i = 0; i < sizeof(src_buffer); i++) { - EXPECT_TRUE(context.ReadData(dest_buffer.data(), 1)); + EXPECT_TRUE(context.ReadAllOrNone(dest_buffer.data(), 1)); EXPECT_EQ(src_buffer[i], dest_buffer[0]); } } -- cgit v1.2.3