Change JpegModule::LoadInfo() to return an optional struct.
Return the image info in a struct, instead of 5 separate out parameters.
Fix some nits in JpegModule code as well.
Change-Id: I0df5f112cb962733355dc41a18d005f015b68db8
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/65774
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Commit-Queue: Lei Zhang <thestig@chromium.org>
diff --git a/core/fpdfapi/page/cpdf_dib.cpp b/core/fpdfapi/page/cpdf_dib.cpp
index 1a79a2f..9bb2fb5 100644
--- a/core/fpdfapi/page/cpdf_dib.cpp
+++ b/core/fpdfapi/page/cpdf_dib.cpp
@@ -549,26 +549,28 @@
if (m_pDecoder)
return true;
- bool bTransform = false;
- int comps;
- int bpc;
- if (!pJpegModule->LoadInfo(src_span, &m_Width, &m_Height, &comps, &bpc,
- &bTransform)) {
+ Optional<JpegModule::JpegImageInfo> info_opt =
+ pJpegModule->LoadInfo(src_span);
+ if (!info_opt.has_value())
return false;
- }
- if (!CPDF_Image::IsValidJpegComponent(comps) ||
- !CPDF_Image::IsValidJpegBitsPerComponent(bpc)) {
+
+ const JpegModule::JpegImageInfo& info = info_opt.value();
+ m_Width = info.width;
+ m_Height = info.height;
+
+ if (!CPDF_Image::IsValidJpegComponent(info.num_components) ||
+ !CPDF_Image::IsValidJpegBitsPerComponent(info.bits_per_components)) {
return false;
}
- if (m_nComponents == static_cast<uint32_t>(comps)) {
- m_bpc = bpc;
- m_pDecoder = pJpegModule->CreateDecoder(src_span, m_Width, m_Height,
- m_nComponents, bTransform);
+ if (m_nComponents == static_cast<uint32_t>(info.num_components)) {
+ m_bpc = info.bits_per_components;
+ m_pDecoder = pJpegModule->CreateDecoder(
+ src_span, m_Width, m_Height, m_nComponents, info.color_transform);
return true;
}
- m_nComponents = static_cast<uint32_t>(comps);
+ m_nComponents = static_cast<uint32_t>(info.num_components);
m_CompData.clear();
if (m_pColorSpace) {
uint32_t colorspace_comps = m_pColorSpace->CountComponents();
@@ -607,9 +609,9 @@
if (!GetDecodeAndMaskArray(&m_bDefaultDecode, &m_bColorKey))
return false;
- m_bpc = bpc;
+ m_bpc = info.bits_per_components;
m_pDecoder = pJpegModule->CreateDecoder(src_span, m_Width, m_Height,
- m_nComponents, bTransform);
+ m_nComponents, info.color_transform);
return true;
}
diff --git a/core/fpdfapi/page/cpdf_image.cpp b/core/fpdfapi/page/cpdf_image.cpp
index fb109eb..5fbb154 100644
--- a/core/fpdfapi/page/cpdf_image.cpp
+++ b/core/fpdfapi/page/cpdf_image.cpp
@@ -83,29 +83,28 @@
RetainPtr<CPDF_Dictionary> CPDF_Image::InitJPEG(
pdfium::span<uint8_t> src_span) {
- int32_t width;
- int32_t height;
- int32_t num_comps;
- int32_t bits;
- bool color_trans;
- if (!fxcodec::ModuleMgr::GetInstance()->GetJpegModule()->LoadInfo(
- src_span, &width, &height, &num_comps, &bits, &color_trans)) {
+ Optional<JpegModule::JpegImageInfo> info_opt =
+ fxcodec::ModuleMgr::GetInstance()->GetJpegModule()->LoadInfo(src_span);
+ if (!info_opt.has_value())
+ return nullptr;
+
+ const JpegModule::JpegImageInfo& info = info_opt.value();
+ if (!IsValidJpegComponent(info.num_components) ||
+ !IsValidJpegBitsPerComponent(info.bits_per_components)) {
return nullptr;
}
- if (!IsValidJpegComponent(num_comps) || !IsValidJpegBitsPerComponent(bits))
- return nullptr;
auto pDict = m_pDocument->New<CPDF_Dictionary>();
pDict->SetNewFor<CPDF_Name>("Type", "XObject");
pDict->SetNewFor<CPDF_Name>("Subtype", "Image");
- pDict->SetNewFor<CPDF_Number>("Width", width);
- pDict->SetNewFor<CPDF_Number>("Height", height);
+ pDict->SetNewFor<CPDF_Number>("Width", info.width);
+ pDict->SetNewFor<CPDF_Number>("Height", info.height);
const char* csname = nullptr;
- if (num_comps == 1) {
+ if (info.num_components == 1) {
csname = "DeviceGray";
- } else if (num_comps == 3) {
+ } else if (info.num_components == 3) {
csname = "DeviceRGB";
- } else if (num_comps == 4) {
+ } else if (info.num_components == 4) {
csname = "DeviceCMYK";
CPDF_Array* pDecode = pDict->SetNewFor<CPDF_Array>("Decode");
for (int n = 0; n < 4; n++) {
@@ -114,16 +113,16 @@
}
}
pDict->SetNewFor<CPDF_Name>("ColorSpace", csname);
- pDict->SetNewFor<CPDF_Number>("BitsPerComponent", bits);
+ pDict->SetNewFor<CPDF_Number>("BitsPerComponent", info.bits_per_components);
pDict->SetNewFor<CPDF_Name>("Filter", "DCTDecode");
- if (!color_trans) {
+ if (!info.color_transform) {
CPDF_Dictionary* pParms =
pDict->SetNewFor<CPDF_Dictionary>(pdfium::stream::kDecodeParms);
pParms->SetNewFor<CPDF_Number>("ColorTransform", 0);
}
m_bIsMask = false;
- m_Width = width;
- m_Height = height;
+ m_Width = info.width;
+ m_Height = info.height;
if (!m_pStream)
m_pStream = pdfium::MakeRetain<CPDF_Stream>();
return pDict;
diff --git a/core/fxcodec/jpeg/jpegmodule.cpp b/core/fxcodec/jpeg/jpegmodule.cpp
index b582c7f..72925f1 100644
--- a/core/fxcodec/jpeg/jpegmodule.cpp
+++ b/core/fxcodec/jpeg/jpegmodule.cpp
@@ -42,13 +42,13 @@
CJpegContext();
~CJpegContext() override;
- jmp_buf* GetJumpMark() { return &m_JumpMark; }
+ jmp_buf& GetJumpMark() { return m_JumpMark; }
jmp_buf m_JumpMark;
- jpeg_decompress_struct m_Info;
- jpeg_error_mgr m_ErrMgr;
- jpeg_source_mgr m_SrcMgr;
- unsigned int m_SkipSize;
+ jpeg_decompress_struct m_Info = {};
+ jpeg_error_mgr m_ErrMgr = {};
+ jpeg_source_mgr m_SrcMgr = {};
+ unsigned int m_SkipSize = 0;
void* (*m_AllocFunc)(unsigned int);
void (*m_FreeFunc)(void*);
};
@@ -188,19 +188,16 @@
}
CJpegContext::CJpegContext()
- : m_SkipSize(0), m_AllocFunc(jpeg_alloc_func), m_FreeFunc(jpeg_free_func) {
- memset(&m_Info, 0, sizeof(m_Info));
+ : m_AllocFunc(jpeg_alloc_func), m_FreeFunc(jpeg_free_func) {
m_Info.client_data = this;
m_Info.err = &m_ErrMgr;
- memset(&m_ErrMgr, 0, sizeof(m_ErrMgr));
m_ErrMgr.error_exit = error_fatal1;
m_ErrMgr.emit_message = error_do_nothing1;
m_ErrMgr.output_message = error_do_nothing;
m_ErrMgr.format_message = error_do_nothing2;
m_ErrMgr.reset_error_mgr = error_do_nothing;
- memset(&m_SrcMgr, 0, sizeof(m_SrcMgr));
m_SrcMgr.init_source = src_do_nothing;
m_SrcMgr.term_source = src_do_nothing;
m_SrcMgr.skip_input_data = src_skip_data1;
@@ -495,14 +492,14 @@
return std::move(pDecoder);
}
-bool JpegModule::LoadInfo(pdfium::span<const uint8_t> src_span,
- int* width,
- int* height,
- int* num_components,
- int* bits_per_components,
- bool* color_transform) {
- return JpegLoadInfo(src_span, width, height, num_components,
- bits_per_components, color_transform);
+Optional<JpegModule::JpegImageInfo> JpegModule::LoadInfo(
+ pdfium::span<const uint8_t> src_span) {
+ JpegImageInfo info;
+ if (!JpegLoadInfo(src_span, &info.width, &info.height, &info.num_components,
+ &info.bits_per_components, &info.color_transform)) {
+ return pdfium::nullopt;
+ }
+ return info;
}
std::unique_ptr<ModuleIface::Context> JpegModule::Start() {
@@ -578,7 +575,7 @@
return static_cast<FX_FILESIZE>(ctx->m_SrcMgr.bytes_in_buffer);
}
-jmp_buf* JpegModule::GetJumpMark(Context* pContext) {
+jmp_buf& JpegModule::GetJumpMark(Context* pContext) {
return static_cast<CJpegContext*>(pContext)->GetJumpMark();
}
diff --git a/core/fxcodec/jpeg/jpegmodule.h b/core/fxcodec/jpeg/jpegmodule.h
index 353de7f..0ecf204 100644
--- a/core/fxcodec/jpeg/jpegmodule.h
+++ b/core/fxcodec/jpeg/jpegmodule.h
@@ -12,6 +12,7 @@
#include "build/build_config.h"
#include "core/fxcodec/codec_module_iface.h"
+#include "third_party/base/optional.h"
#include "third_party/base/span.h"
class CFX_DIBBase;
@@ -23,6 +24,14 @@
class JpegModule final : public ModuleIface {
public:
+ struct JpegImageInfo {
+ int width;
+ int height;
+ int num_components;
+ int bits_per_components;
+ bool color_transform;
+ };
+
std::unique_ptr<ScanlineDecoder> CreateDecoder(
pdfium::span<const uint8_t> src_span,
int width,
@@ -36,13 +45,8 @@
RetainPtr<CFX_CodecMemory> codec_memory,
CFX_DIBAttribute* pAttribute) override;
- jmp_buf* GetJumpMark(Context* pContext);
- bool LoadInfo(pdfium::span<const uint8_t> src_span,
- int* width,
- int* height,
- int* num_components,
- int* bits_per_components,
- bool* color_transform);
+ jmp_buf& GetJumpMark(Context* pContext);
+ Optional<JpegImageInfo> LoadInfo(pdfium::span<const uint8_t> src_span);
std::unique_ptr<Context> Start();
diff --git a/core/fxcodec/progressivedecoder.cpp b/core/fxcodec/progressivedecoder.cpp
index 7582e16..e948b39 100644
--- a/core/fxcodec/progressivedecoder.cpp
+++ b/core/fxcodec/progressivedecoder.cpp
@@ -1025,7 +1025,7 @@
// Setting jump marker before calling ReadHeader, since a longjmp to
// the marker indicates a fatal error.
- if (setjmp(*pJpegModule->GetJumpMark(m_pJpegContext.get())) == -1) {
+ if (setjmp(pJpegModule->GetJumpMark(m_pJpegContext.get())) == -1) {
m_pJpegContext.reset();
m_status = FXCODEC_STATUS_ERR_FORMAT;
return false;
@@ -1061,7 +1061,7 @@
GetDownScale(down_scale);
// Setting jump marker before calling StartScanLine, since a longjmp to
// the marker indicates a fatal error.
- if (setjmp(*pJpegModule->GetJumpMark(m_pJpegContext.get())) == -1) {
+ if (setjmp(pJpegModule->GetJumpMark(m_pJpegContext.get())) == -1) {
m_pJpegContext.reset();
m_status = FXCODEC_STATUS_ERROR;
return FXCODEC_STATUS_ERROR;
@@ -1105,7 +1105,7 @@
JpegModule* pJpegModule = m_pCodecMgr->GetJpegModule();
// Setting jump marker before calling ReadScanLine, since a longjmp to
// the marker indicates a fatal error.
- if (setjmp(*pJpegModule->GetJumpMark(m_pJpegContext.get())) == -1) {
+ if (setjmp(pJpegModule->GetJumpMark(m_pJpegContext.get())) == -1) {
m_pJpegContext.reset();
m_status = FXCODEC_STATUS_ERROR;
return FXCODEC_STATUS_ERROR;
