Redo CFX_DIBitmap::CalculatePitchAndSize(). Remove the in-out parameters and replace them with Optional<CFX_DIBitmap::PitchAndSize>, where PitchAndSize is a struct. Clean up the callers and document the function behavior. Change-Id: Iaee5ee4243328661a004e8885bb670a79d04945d Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/73391 Reviewed-by: Tom Sepez <tsepez@chromium.org> Commit-Queue: Lei Zhang <thestig@chromium.org>
diff --git a/core/fpdfapi/render/cpdf_scaledrenderbuffer.cpp b/core/fpdfapi/render/cpdf_scaledrenderbuffer.cpp index ab54d85..257fcfe 100644 --- a/core/fpdfapi/render/cpdf_scaledrenderbuffer.cpp +++ b/core/fpdfapi/render/cpdf_scaledrenderbuffer.cpp
@@ -46,14 +46,13 @@ int32_t width = bitmap_rect.Width(); int32_t height = bitmap_rect.Height(); // Set to 0 to make CalculatePitchAndSize() calculate it. - uint32_t pitch = 0; - uint32_t size; - if (!CFX_DIBitmap::CalculatePitchAndSize(width, height, dibFormat, &pitch, - &size)) { + constexpr uint32_t kNoPitch = 0; + Optional<CFX_DIBitmap::PitchAndSize> pitch_size = + CFX_DIBitmap::CalculatePitchAndSize(width, height, dibFormat, kNoPitch); + if (!pitch_size.has_value()) return false; - } - if (size <= kImageSizeLimitBytes && + if (pitch_size.value().size <= kImageSizeLimitBytes && m_pBitmapDevice->Create(width, height, dibFormat, nullptr)) { break; }
diff --git a/core/fxcodec/progressive_decoder.cpp b/core/fxcodec/progressive_decoder.cpp index c22fde0..7171f46 100644 --- a/core/fxcodec/progressive_decoder.cpp +++ b/core/fxcodec/progressive_decoder.cpp
@@ -743,17 +743,19 @@ return false; } - uint32_t pitch = 0; - uint32_t neededData = 0; - if (!CFX_DIBitmap::CalculatePitchAndSize(m_SrcWidth, m_SrcHeight, format, - &pitch, &neededData)) { + // Set to 0 to make CalculatePitchAndSize() calculate it. + constexpr uint32_t kNoPitch = 0; + Optional<CFX_DIBitmap::PitchAndSize> needed_data = + CFX_DIBitmap::CalculatePitchAndSize(m_SrcWidth, m_SrcHeight, format, + kNoPitch); + if (!needed_data.has_value()) { m_status = FXCODEC_STATUS_ERR_FORMAT; return false; } - uint32_t availableData = m_pFile->GetSize() - m_offSet + - BmpDecoder::GetAvailInput(pBmpContext.get()); - if (neededData > availableData) { + uint32_t available_data = m_pFile->GetSize() - m_offSet + + BmpDecoder::GetAvailInput(pBmpContext.get()); + if (needed_data.value().size > available_data) { m_status = FXCODEC_STATUS_ERR_FORMAT; return false; }
diff --git a/core/fxge/dib/cfx_dibitmap.cpp b/core/fxge/dib/cfx_dibitmap.cpp index 8b0551c..ad12939 100644 --- a/core/fxge/dib/cfx_dibitmap.cpp +++ b/core/fxge/dib/cfx_dibitmap.cpp
@@ -47,14 +47,15 @@ m_Height = 0; m_Pitch = 0; - uint32_t calculatedSize; - if (!CalculatePitchAndSize(height, width, format, &pitch, &calculatedSize)) + Optional<PitchAndSize> pitch_size = + CalculatePitchAndSize(height, width, format, pitch); + if (!pitch_size.has_value()) return false; if (pBuffer) { m_pBuffer.Reset(pBuffer); } else { - size_t bufferSize = calculatedSize + 4; + size_t bufferSize = pitch_size.value().size + 4; if (bufferSize >= kMaxOOMLimit) { m_pBuffer = std::unique_ptr<uint8_t, FxFreeDeleter>( FX_TryAlloc(uint8_t, bufferSize)); @@ -67,7 +68,7 @@ } m_Width = width; m_Height = height; - m_Pitch = pitch; + m_Pitch = pitch_size.value().pitch; if (!HasAlpha() || format == FXDIB_Argb) return true; @@ -840,29 +841,30 @@ } // static -bool CFX_DIBitmap::CalculatePitchAndSize(int height, - int width, - FXDIB_Format format, - uint32_t* pitch, - uint32_t* size) { +Optional<CFX_DIBitmap::PitchAndSize> CFX_DIBitmap::CalculatePitchAndSize( + int height, + int width, + FXDIB_Format format, + uint32_t pitch) { if (width <= 0 || height <= 0) - return false; + return pdfium::nullopt; int bpp = GetBppFromFormat(format); if (!bpp) - return false; + return pdfium::nullopt; if ((INT_MAX - 31) / width < bpp) - return false; + return pdfium::nullopt; - if (!*pitch) - *pitch = static_cast<uint32_t>((width * bpp + 31) / 32 * 4); + uint32_t actual_pitch = pitch; + if (actual_pitch == 0) + actual_pitch = static_cast<uint32_t>((width * bpp + 31) / 32 * 4); - if ((1 << 30) / *pitch < static_cast<uint32_t>(height)) - return false; + if ((1 << 30) / actual_pitch < static_cast<uint32_t>(height)) + return pdfium::nullopt; - *size = *pitch * static_cast<uint32_t>(height); - return true; + uint32_t size = actual_pitch * static_cast<uint32_t>(height); + return PitchAndSize{actual_pitch, size}; } bool CFX_DIBitmap::CompositeBitmap(int dest_left,
diff --git a/core/fxge/dib/cfx_dibitmap.h b/core/fxge/dib/cfx_dibitmap.h index 5c59c7a..27a8de0 100644 --- a/core/fxge/dib/cfx_dibitmap.h +++ b/core/fxge/dib/cfx_dibitmap.h
@@ -13,9 +13,15 @@ #include "core/fxcrt/retain_ptr.h" #include "core/fxge/dib/cfx_dibbase.h" #include "core/fxge/fx_dib.h" +#include "third_party/base/optional.h" class CFX_DIBitmap : public CFX_DIBBase { public: + struct PitchAndSize { + uint32_t pitch; + uint32_t size; + }; + CONSTRUCT_VIA_MAKE_RETAIN; bool Create(int width, int height, FXDIB_Format format); @@ -93,11 +99,17 @@ bool ConvertColorScale(uint32_t forecolor, uint32_t backcolor); - static bool CalculatePitchAndSize(int height, - int width, - FXDIB_Format format, - uint32_t* pitch, - uint32_t* size); + // |width| and |height| must be greater than 0. + // |format| must have a valid bits per pixel count. + // If |pitch| is zero, then the actual pitch will be calculated based on + // |width| and |format|. + // If |pitch| is non-zero, then that be used as the actual pitch. + // The actual pitch will be used to calculate the size. + // Returns the calculated pitch and size on success, or nullopt on failure. + static Optional<PitchAndSize> CalculatePitchAndSize(int height, + int width, + FXDIB_Format format, + uint32_t pitch); #if defined(_SKIA_SUPPORT_) || defined(_SKIA_SUPPORT_PATHS_) void PreMultiply();