From fc04f41200e4ba4f47f52f188708547e8a1bee6d Mon Sep 17 00:00:00 2001 From: Tom Sepez Date: Wed, 31 May 2017 15:18:28 -0700 Subject: Move all of ccodec_bmpmodule state to its context This avoids a stale delegate pointer issue in the module. In theory, it should also allow for multiple decodes at the same time from different contexts within the same module, but this isn't used. Rename associated context, and use |new| to create it. Along the way, resolve a subtle FX_Alloc() vs. user-supplied callback free() issue, and remove supporting code. Bug: 728323 Change-Id: I7bb66bb5d5b4fa173bec2b445a8e71ab691fdf5c Reviewed-on: https://pdfium-review.googlesource.com/6133 Reviewed-by: Lei Zhang Commit-Queue: Tom Sepez --- core/fxcodec/codec/ccodec_bmpmodule.cpp | 137 ++++++++++++------------- core/fxcodec/codec/ccodec_bmpmodule.h | 22 ++-- core/fxcodec/codec/ccodec_progressivedecoder.h | 2 +- core/fxcodec/codec/fx_codec_progress.cpp | 3 +- 4 files changed, 76 insertions(+), 88 deletions(-) diff --git a/core/fxcodec/codec/ccodec_bmpmodule.cpp b/core/fxcodec/codec/ccodec_bmpmodule.cpp index 003f5d3d49..ae0a30c4b4 100644 --- a/core/fxcodec/codec/ccodec_bmpmodule.cpp +++ b/core/fxcodec/codec/ccodec_bmpmodule.cpp @@ -9,81 +9,78 @@ #include "core/fxcodec/codec/codec_int.h" #include "core/fxcodec/fx_codec.h" #include "core/fxcodec/lbmp/fx_bmp.h" +#include "core/fxcrt/cfx_unowned_ptr.h" #include "core/fxge/fx_dib.h" -struct FXBMP_Context { - bmp_decompress_struct_p bmp_ptr; - void* parent_ptr; +class CCodec_BmpModule::Context { + public: + Context(bmp_decompress_struct_p pBmp, + CCodec_BmpModule* pModule, + CCodec_BmpModule::Delegate* pDelegate); + ~Context(); - void* (*m_AllocFunc)(unsigned int); - void (*m_FreeFunc)(void*); + bmp_decompress_struct_p m_pBmp; + CFX_UnownedPtr const m_pModule; + CFX_UnownedPtr const m_pDelegate; + char m_szLastError[256]; }; + extern "C" { -static void* bmp_alloc_func(unsigned int size) { - return FX_Alloc(char, size); -} -static void bmp_free_func(void* p) { - FX_Free(p); -} -}; -static void bmp_error_data(bmp_decompress_struct_p bmp_ptr, - const char* err_msg) { - strncpy((char*)bmp_ptr->err_ptr, err_msg, BMP_MAX_ERROR_SIZE - 1); - longjmp(bmp_ptr->jmpbuf, 1); + +static void bmp_error_data(bmp_decompress_struct_p pBmp, const char* err_msg) { + strncpy(pBmp->err_ptr, err_msg, BMP_MAX_ERROR_SIZE - 1); + longjmp(pBmp->jmpbuf, 1); } -static void bmp_read_scanline(bmp_decompress_struct_p bmp_ptr, + +static void bmp_read_scanline(bmp_decompress_struct_p pBmp, int32_t row_num, uint8_t* row_buf) { - FXBMP_Context* p = (FXBMP_Context*)bmp_ptr->context_ptr; - CCodec_BmpModule* pModule = (CCodec_BmpModule*)p->parent_ptr; - pModule->GetDelegate()->BmpReadScanline(row_num, row_buf); + auto* p = reinterpret_cast(pBmp->context_ptr); + p->m_pDelegate->BmpReadScanline(row_num, row_buf); } -static bool bmp_get_data_position(bmp_decompress_struct_p bmp_ptr, + +static bool bmp_get_data_position(bmp_decompress_struct_p pBmp, uint32_t rcd_pos) { - FXBMP_Context* p = (FXBMP_Context*)bmp_ptr->context_ptr; - CCodec_BmpModule* pModule = (CCodec_BmpModule*)p->parent_ptr; - return pModule->GetDelegate()->BmpInputImagePositionBuf(rcd_pos); + auto* p = reinterpret_cast(pBmp->context_ptr); + return p->m_pDelegate->BmpInputImagePositionBuf(rcd_pos); } -CCodec_BmpModule::CCodec_BmpModule() { +} // extern "C" + +CCodec_BmpModule::Context::Context(bmp_decompress_struct_p pBmp, + CCodec_BmpModule* pModule, + CCodec_BmpModule::Delegate* pDelegate) + : m_pBmp(pBmp), m_pModule(pModule), m_pDelegate(pDelegate) { memset(m_szLastError, 0, sizeof(m_szLastError)); } -CCodec_BmpModule::~CCodec_BmpModule() {} +CCodec_BmpModule::Context::~Context() { + bmp_destroy_decompress(&m_pBmp); +} -FXBMP_Context* CCodec_BmpModule::Start() { - FXBMP_Context* p = FX_Alloc(FXBMP_Context, 1); - if (!p) - return nullptr; +CCodec_BmpModule::CCodec_BmpModule() {} - memset(p, 0, sizeof(FXBMP_Context)); - if (!p) - return nullptr; +CCodec_BmpModule::~CCodec_BmpModule() {} - p->m_AllocFunc = bmp_alloc_func; - p->m_FreeFunc = bmp_free_func; - p->bmp_ptr = nullptr; - p->parent_ptr = this; - p->bmp_ptr = bmp_create_decompress(); - if (!p->bmp_ptr) { - FX_Free(p); +CCodec_BmpModule::Context* CCodec_BmpModule::Start(Delegate* pDelegate) { + bmp_decompress_struct_p pBmp = bmp_create_decompress(); + if (!pBmp) return nullptr; - } - p->bmp_ptr->context_ptr = p; - p->bmp_ptr->err_ptr = m_szLastError; - p->bmp_ptr->bmp_error_fn = bmp_error_data; - p->bmp_ptr->bmp_get_row_fn = bmp_read_scanline; - p->bmp_ptr->bmp_get_data_position_fn = bmp_get_data_position; + + auto* p = new Context(pBmp, this, pDelegate); + p->m_pBmp->context_ptr = p; + p->m_pBmp->err_ptr = p->m_szLastError; + p->m_pBmp->bmp_error_fn = bmp_error_data; + p->m_pBmp->bmp_get_row_fn = bmp_read_scanline; + p->m_pBmp->bmp_get_data_position_fn = bmp_get_data_position; return p; } -void CCodec_BmpModule::Finish(FXBMP_Context* ctx) { - if (ctx) { - bmp_destroy_decompress(&ctx->bmp_ptr); - ctx->m_FreeFunc(ctx); - } +void CCodec_BmpModule::Finish(Context* ctx) { + delete ctx; } -int32_t CCodec_BmpModule::ReadHeader(FXBMP_Context* ctx, + +int32_t CCodec_BmpModule::ReadHeader(Context* ctx, int32_t* width, int32_t* height, bool* tb_flag, @@ -91,41 +88,41 @@ int32_t CCodec_BmpModule::ReadHeader(FXBMP_Context* ctx, int32_t* pal_num, uint32_t** pal_pp, CFX_DIBAttribute* pAttribute) { - if (setjmp(ctx->bmp_ptr->jmpbuf)) { + if (setjmp(ctx->m_pBmp->jmpbuf)) { return 0; } - int32_t ret = bmp_read_header(ctx->bmp_ptr); + int32_t ret = bmp_read_header(ctx->m_pBmp); if (ret != 1) { return ret; } - *width = ctx->bmp_ptr->width; - *height = ctx->bmp_ptr->height; - *tb_flag = ctx->bmp_ptr->imgTB_flag; - *components = ctx->bmp_ptr->components; - *pal_num = ctx->bmp_ptr->pal_num; - *pal_pp = ctx->bmp_ptr->pal_ptr; + *width = ctx->m_pBmp->width; + *height = ctx->m_pBmp->height; + *tb_flag = ctx->m_pBmp->imgTB_flag; + *components = ctx->m_pBmp->components; + *pal_num = ctx->m_pBmp->pal_num; + *pal_pp = ctx->m_pBmp->pal_ptr; if (pAttribute) { pAttribute->m_wDPIUnit = FXCODEC_RESUNIT_METER; - pAttribute->m_nXDPI = ctx->bmp_ptr->dpi_x; - pAttribute->m_nYDPI = ctx->bmp_ptr->dpi_y; - pAttribute->m_nBmpCompressType = ctx->bmp_ptr->compress_flag; + pAttribute->m_nXDPI = ctx->m_pBmp->dpi_x; + pAttribute->m_nYDPI = ctx->m_pBmp->dpi_y; + pAttribute->m_nBmpCompressType = ctx->m_pBmp->compress_flag; } return 1; } -int32_t CCodec_BmpModule::LoadImage(FXBMP_Context* ctx) { - if (setjmp(ctx->bmp_ptr->jmpbuf)) +int32_t CCodec_BmpModule::LoadImage(Context* ctx) { + if (setjmp(ctx->m_pBmp->jmpbuf)) return 0; - return bmp_decode_image(ctx->bmp_ptr); + return bmp_decode_image(ctx->m_pBmp); } -uint32_t CCodec_BmpModule::GetAvailInput(FXBMP_Context* ctx, +uint32_t CCodec_BmpModule::GetAvailInput(Context* ctx, uint8_t** avail_buf_ptr) { - return bmp_get_avail_input(ctx->bmp_ptr, avail_buf_ptr); + return bmp_get_avail_input(ctx->m_pBmp, avail_buf_ptr); } -void CCodec_BmpModule::Input(FXBMP_Context* ctx, +void CCodec_BmpModule::Input(Context* ctx, const uint8_t* src_buf, uint32_t src_size) { - bmp_input_buffer(ctx->bmp_ptr, (uint8_t*)src_buf, src_size); + bmp_input_buffer(ctx->m_pBmp, (uint8_t*)src_buf, src_size); } diff --git a/core/fxcodec/codec/ccodec_bmpmodule.h b/core/fxcodec/codec/ccodec_bmpmodule.h index c9fcabb2bd..7893fba4c7 100644 --- a/core/fxcodec/codec/ccodec_bmpmodule.h +++ b/core/fxcodec/codec/ccodec_bmpmodule.h @@ -11,10 +11,10 @@ #include "core/fxcrt/fx_system.h" class CFX_DIBAttribute; -struct FXBMP_Context; class CCodec_BmpModule { public: + class Context; class Delegate { public: virtual bool BmpInputImagePositionBuf(uint32_t rcd_pos) = 0; @@ -24,13 +24,11 @@ class CCodec_BmpModule { CCodec_BmpModule(); ~CCodec_BmpModule(); - FXBMP_Context* Start(); - void Finish(FXBMP_Context* pContext); - uint32_t GetAvailInput(FXBMP_Context* pContext, uint8_t** avail_buf_ptr); - void Input(FXBMP_Context* pContext, - const uint8_t* src_buf, - uint32_t src_size); - int32_t ReadHeader(FXBMP_Context* pContext, + Context* Start(Delegate* pDelegate); + void Finish(Context* pContext); + uint32_t GetAvailInput(Context* pContext, uint8_t** avail_buf_ptr); + void Input(Context* pContext, const uint8_t* src_buf, uint32_t src_size); + int32_t ReadHeader(Context* pContext, int32_t* width, int32_t* height, bool* tb_flag, @@ -38,13 +36,7 @@ class CCodec_BmpModule { int32_t* pal_num, uint32_t** pal_pp, CFX_DIBAttribute* pAttribute); - int32_t LoadImage(FXBMP_Context* pContext); - Delegate* GetDelegate() const { return m_pDelegate.Get(); } - void SetDelegate(Delegate* pDelegate) { m_pDelegate = pDelegate; } - - protected: - CFX_UnownedPtr m_pDelegate; - char m_szLastError[256]; + int32_t LoadImage(Context* pContext); }; #endif // CORE_FXCODEC_CODEC_CCODEC_BMPMODULE_H_ diff --git a/core/fxcodec/codec/ccodec_progressivedecoder.h b/core/fxcodec/codec/ccodec_progressivedecoder.h index 42d0ee3aba..f9a67ccee3 100644 --- a/core/fxcodec/codec/ccodec_progressivedecoder.h +++ b/core/fxcodec/codec/ccodec_progressivedecoder.h @@ -134,7 +134,7 @@ class CCodec_ProgressiveDecoder : public CCodec_BmpModule::Delegate, FXJPEG_Context* m_pJpegContext; FXPNG_Context* m_pPngContext; std::unique_ptr m_pGifContext; - FXBMP_Context* m_pBmpContext; + CCodec_BmpModule::Context* m_pBmpContext; CFX_UnownedPtr m_pTiffContext; FXCODEC_IMAGE_TYPE m_imagType; uint32_t m_offSet; diff --git a/core/fxcodec/codec/fx_codec_progress.cpp b/core/fxcodec/codec/fx_codec_progress.cpp index dba0e20533..4cadb152d8 100644 --- a/core/fxcodec/codec/fx_codec_progress.cpp +++ b/core/fxcodec/codec/fx_codec_progress.cpp @@ -1020,8 +1020,7 @@ bool CCodec_ProgressiveDecoder::DetectImageType(FXCODEC_IMAGE_TYPE imageType, m_status = FXCODEC_STATUS_ERR_MEMORY; return false; } - pBmpModule->SetDelegate(this); - m_pBmpContext = pBmpModule->Start(); + m_pBmpContext = pBmpModule->Start(this); if (!m_pBmpContext) { m_status = FXCODEC_STATUS_ERR_MEMORY; return false; -- cgit v1.2.3