From 6f3593c5cdf62915fc086448312671ce1fce5291 Mon Sep 17 00:00:00 2001 From: Nicolas Pena Date: Tue, 23 May 2017 19:07:25 -0400 Subject: Remove longjmp from the Gif module MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Gif module is only using the longjmp as a way to catch errors, so this CL makes it use GifDecodeStatus::Error instead. Change-Id: I9c97e63ed851d2a80e38f1d2cd9e8f297d608cb2 Reviewed-on: https://pdfium-review.googlesource.com/5850 Commit-Queue: Nicolás Peña Reviewed-by: Tom Sepez --- core/fxcodec/codec/ccodec_gifmodule.cpp | 42 +++++++++++++-------------------- core/fxcodec/lgif/cgifcontext.cpp | 3 +-- core/fxcodec/lgif/cgifcontext.h | 5 +--- core/fxcodec/lgif/fx_gif.cpp | 29 ++++++++--------------- core/fxcodec/lgif/fx_gif.h | 1 - 5 files changed, 28 insertions(+), 52 deletions(-) diff --git a/core/fxcodec/codec/ccodec_gifmodule.cpp b/core/fxcodec/codec/ccodec_gifmodule.cpp index 438f019ee9..11980ee7a5 100644 --- a/core/fxcodec/codec/ccodec_gifmodule.cpp +++ b/core/fxcodec/codec/ccodec_gifmodule.cpp @@ -30,9 +30,6 @@ GifDecodeStatus CCodec_GifModule::ReadHeader(CGifContext* context, void** pal_pp, int* bg_index, CFX_DIBAttribute* pAttribute) { - if (setjmp(context->jmpbuf)) - return GifDecodeStatus::Error; - GifDecodeStatus ret = gif_read_header(context); if (ret != GifDecodeStatus::Success) return ret; @@ -48,9 +45,6 @@ GifDecodeStatus CCodec_GifModule::ReadHeader(CGifContext* context, GifDecodeStatus CCodec_GifModule::LoadFrameInfo(CGifContext* context, int* frame_num) { - if (setjmp(context->jmpbuf)) - return GifDecodeStatus::Error; - GifDecodeStatus ret = gif_get_frame(context); if (ret != GifDecodeStatus::Success) return ret; @@ -62,28 +56,24 @@ GifDecodeStatus CCodec_GifModule::LoadFrameInfo(CGifContext* context, GifDecodeStatus CCodec_GifModule::LoadFrame(CGifContext* context, int frame_num, CFX_DIBAttribute* pAttribute) { - if (setjmp(context->jmpbuf)) - return GifDecodeStatus::Error; - GifDecodeStatus ret = gif_load_frame(context, frame_num); - if (ret == GifDecodeStatus::Success) { - if (pAttribute) { - pAttribute->m_nGifLeft = context->m_Images[frame_num]->m_ImageInfo.left; - pAttribute->m_nGifTop = context->m_Images[frame_num]->m_ImageInfo.top; - pAttribute->m_fAspectRatio = context->pixel_aspect; - const uint8_t* buf = - reinterpret_cast(context->cmt_data.GetBuffer(0)); - uint32_t len = context->cmt_data.GetLength(); - if (len > 21) { - uint8_t size = *buf++; - if (size != 0) - pAttribute->m_strAuthor = CFX_ByteString(buf, size); - else - pAttribute->m_strAuthor.clear(); - } - } + if (ret != GifDecodeStatus::Success || !pAttribute) + return ret; + + pAttribute->m_nGifLeft = context->m_Images[frame_num]->m_ImageInfo.left; + pAttribute->m_nGifTop = context->m_Images[frame_num]->m_ImageInfo.top; + pAttribute->m_fAspectRatio = context->pixel_aspect; + const uint8_t* buf = + reinterpret_cast(context->cmt_data.GetBuffer(0)); + uint32_t len = context->cmt_data.GetLength(); + if (len > 21) { + uint8_t size = *buf++; + if (size != 0) + pAttribute->m_strAuthor = CFX_ByteString(buf, size); + else + pAttribute->m_strAuthor.clear(); } - return ret; + return GifDecodeStatus::Success; } uint32_t CCodec_GifModule::GetAvailInput(CGifContext* context, diff --git a/core/fxcodec/lgif/cgifcontext.cpp b/core/fxcodec/lgif/cgifcontext.cpp index b7a513c7e8..9c03cffa21 100644 --- a/core/fxcodec/lgif/cgifcontext.cpp +++ b/core/fxcodec/lgif/cgifcontext.cpp @@ -33,9 +33,8 @@ CGifContext::CGifContext(CCodec_GifModule* gif_module, char* error_string) CGifContext::~CGifContext() {} -void CGifContext::ThrowError(const char* err_msg) { +void CGifContext::AddError(const char* err_msg) { strncpy(err_ptr, err_msg, GIF_MAX_ERROR_SIZE - 1); - longjmp(jmpbuf, 1); } void CGifContext::RecordCurrentPosition(uint32_t* cur_pos_ptr) { diff --git a/core/fxcodec/lgif/cgifcontext.h b/core/fxcodec/lgif/cgifcontext.h index e65d6c7b0b..08a66c18e2 100644 --- a/core/fxcodec/lgif/cgifcontext.h +++ b/core/fxcodec/lgif/cgifcontext.h @@ -7,7 +7,6 @@ #ifndef CORE_FXCODEC_LGIF_CGIFCONTEXT_H_ #define CORE_FXCODEC_LGIF_CGIFCONTEXT_H_ -#include #include #include @@ -21,8 +20,7 @@ class CGifContext { CGifContext(CCodec_GifModule* gif_module, char* error_string); ~CGifContext(); - // TODO(npm): Remove longjmp from this method!!! - void ThrowError(const char* err_msg); + void AddError(const char* err_msg); void RecordCurrentPosition(uint32_t* cur_pos_ptr); void ReadScanline(int32_t row_num, uint8_t* row_buf); bool GetRecordPosition(uint32_t cur_pos, @@ -38,7 +36,6 @@ class CGifContext { int32_t disposal_method, bool interlace); - jmp_buf jmpbuf; std::vector m_GlobalPalette; int32_t global_pal_num; uint32_t img_row_offset; diff --git a/core/fxcodec/lgif/fx_gif.cpp b/core/fxcodec/lgif/fx_gif.cpp index 5e257cc2f5..b4fe328f6c 100644 --- a/core/fxcodec/lgif/fx_gif.cpp +++ b/core/fxcodec/lgif/fx_gif.cpp @@ -119,8 +119,7 @@ GifDecodeStatus gif_decode_extension(CGifContext* context) { GifDecodeStatus gif_decode_image_info(CGifContext* context) { if (context->width == 0 || context->height == 0) { - context->ThrowError("No Image Header Info"); - NOTREACHED(); + context->AddError("No Image Header Info"); return GifDecodeStatus::Error; } uint32_t skip_size_org = context->skip_size; @@ -143,9 +142,7 @@ GifDecodeStatus gif_decode_image_info(CGifContext* context) { context->width || gif_image->m_ImageInfo.top + gif_image->m_ImageInfo.height > context->height) { - gif_image = nullptr; - context->ThrowError("Image Data Out Of LSD, The File May Be Corrupt"); - NOTREACHED(); + context->AddError("Image Data Out Of LSD, The File May Be Corrupt"); return GifDecodeStatus::Error; } GifLF* gif_img_info_lf_ptr = (GifLF*)&gif_img_info_ptr->local_flag; @@ -182,8 +179,7 @@ void gif_decoding_failure_at_tail_cleanup(CGifContext* context, GifImage* gif_image_ptr) { gif_image_ptr->m_ImageRowBuf.clear(); gif_save_decoding_status(context, GIF_D_STATUS_TAIL); - context->ThrowError("Decode Image Data Error"); - NOTREACHED(); + context->AddError("Decode Image Data Error"); } } // namespace @@ -378,8 +374,7 @@ GifDecodeStatus gif_read_header(CGifContext* context) { if (strncmp(gif_header_ptr->signature, GIF_SIGNATURE, 3) != 0 || gif_header_ptr->version[0] != '8' || gif_header_ptr->version[2] != 'a') { - context->ThrowError("Not A Gif Image"); - NOTREACHED(); + context->AddError("Not A Gif Image"); return GifDecodeStatus::Error; } GifLSD* gif_lsd_ptr = nullptr; @@ -474,6 +469,7 @@ GifDecodeStatus gif_get_frame(CGifContext* context) { ret = gif_decode_image_info(context); if (ret != GifDecodeStatus::Success) return ret; + continue; } case GIF_D_STATUS_IMG_DATA: { @@ -517,8 +513,7 @@ GifDecodeStatus gif_load_frame(CGifContext* context, int32_t frame_num) { GifImage* gif_image_ptr = context->m_Images[frame_num].get(); uint32_t gif_img_row_bytes = gif_image_ptr->m_ImageInfo.width; if (gif_img_row_bytes == 0) { - context->ThrowError("Error Invalid Number of Row Bytes"); - NOTREACHED(); + context->AddError("Error Invalid Number of Row Bytes"); return GifDecodeStatus::Error; } if (context->decode_status == GIF_D_STATUS_TAIL) { @@ -541,8 +536,7 @@ GifDecodeStatus gif_load_frame(CGifContext* context, int32_t frame_num) { (bool)((GifLF*)&gif_image_ptr->m_ImageInfo.local_flag)->interlace); if (!bRes) { gif_image_ptr->m_ImageRowBuf.clear(); - context->ThrowError("Error Read Record Position Data"); - NOTREACHED(); + context->AddError("Error Read Record Position Data"); return GifDecodeStatus::Error; } } else { @@ -560,15 +554,13 @@ GifDecodeStatus gif_load_frame(CGifContext* context, int32_t frame_num) { (bool)((GifLF*)&gif_image_ptr->m_ImageInfo.local_flag)->interlace); if (!bRes) { gif_image_ptr->m_ImageRowBuf.clear(); - context->ThrowError("Error Read Record Position Data"); - NOTREACHED(); + context->AddError("Error Read Record Position Data"); return GifDecodeStatus::Error; } } if (gif_image_ptr->image_code_size >= 13) { gif_image_ptr->m_ImageRowBuf.clear(); - context->ThrowError("Error Invalid Code Size"); - NOTREACHED(); + context->AddError("Error Invalid Code Size"); return GifDecodeStatus::Error; } if (!context->m_ImgDecoder.get()) @@ -665,8 +657,7 @@ GifDecodeStatus gif_load_frame(CGifContext* context, int32_t frame_num) { } gif_save_decoding_status(context, GIF_D_STATUS_TAIL); } - context->ThrowError("Decode Image Data Error"); - NOTREACHED(); + context->AddError("Decode Image Data Error"); return GifDecodeStatus::Error; } diff --git a/core/fxcodec/lgif/fx_gif.h b/core/fxcodec/lgif/fx_gif.h index d7cd5d83fb..5216eeb70e 100644 --- a/core/fxcodec/lgif/fx_gif.h +++ b/core/fxcodec/lgif/fx_gif.h @@ -7,7 +7,6 @@ #ifndef CORE_FXCODEC_LGIF_FX_GIF_H_ #define CORE_FXCODEC_LGIF_FX_GIF_H_ -#include #include #include -- cgit v1.2.3