From 405ac0f09e1622d7ff3cf60314d290851ac9f7fd Mon Sep 17 00:00:00 2001 From: tsepez Date: Mon, 28 Nov 2016 13:59:14 -0800 Subject: Use CFX_MaybeOwned<> in fpdf_edit_create.cpp Fix missing second template parameter in cfx_maybe_owned.h Review-Url: https://codereview.chromium.org/2522313002 --- core/fpdfapi/edit/fpdf_edit_create.cpp | 117 ++++++++++++++------------------ core/fpdfapi/parser/cpdf_stream.cpp | 2 +- core/fpdfapi/parser/cpdf_stream_acc.cpp | 8 +-- core/fpdfapi/parser/cpdf_stream_acc.h | 4 +- core/fxcrt/cfx_maybe_owned.h | 12 ++-- 5 files changed, 66 insertions(+), 77 deletions(-) diff --git a/core/fpdfapi/edit/fpdf_edit_create.cpp b/core/fpdfapi/edit/fpdf_edit_create.cpp index b2e31054c0..308fc49048 100644 --- a/core/fpdfapi/edit/fpdf_edit_create.cpp +++ b/core/fpdfapi/edit/fpdf_edit_create.cpp @@ -6,6 +6,7 @@ #include "core/fpdfapi/edit/editint.h" +#include #include #include "core/fpdfapi/edit/cpdf_creator.h" @@ -22,6 +23,7 @@ #include "core/fpdfapi/parser/cpdf_stream_acc.h" #include "core/fpdfapi/parser/cpdf_string.h" #include "core/fpdfapi/parser/fpdf_parser_decode.h" +#include "core/fxcrt/cfx_maybe_owned.h" #include "core/fxcrt/fx_ext.h" #include "third_party/base/stl_util.h" @@ -396,52 +398,43 @@ class CPDF_FlateEncoder { void CloneDict(); - uint8_t* m_pData; uint32_t m_dwSize; - CPDF_Dictionary* m_pDict; - bool m_bCloned; - bool m_bNewData; + CFX_MaybeOwned m_pData; + CFX_MaybeOwned m_pDict; CPDF_StreamAcc m_Acc; }; void CPDF_FlateEncoder::CloneDict() { - if (!m_bCloned) { - m_pDict = ToDictionary(m_pDict->Clone().release()); - ASSERT(m_pDict); - m_bCloned = true; - } + if (m_pDict.IsOwned()) + return; + m_pDict = ToDictionary(m_pDict->Clone()); + ASSERT(m_pDict.IsOwned()); } CPDF_FlateEncoder::CPDF_FlateEncoder(CPDF_Stream* pStream, bool bFlateEncode) - : m_pData(nullptr), - m_dwSize(0), - m_pDict(nullptr), - m_bCloned(false), - m_bNewData(false) { + : m_dwSize(0) { m_Acc.LoadAllData(pStream, true); bool bHasFilter = pStream && pStream->HasFilter(); if (bHasFilter && !bFlateEncode) { CPDF_StreamAcc destAcc; destAcc.LoadAllData(pStream); m_dwSize = destAcc.GetSize(); - m_pData = (uint8_t*)destAcc.DetachData(); - m_pDict = ToDictionary(pStream->GetDict()->Clone().release()); + m_pData = destAcc.DetachData(); + m_pDict = ToDictionary(pStream->GetDict()->Clone()); m_pDict->RemoveFor("Filter"); - m_bNewData = true; - m_bCloned = true; return; } if (bHasFilter || !bFlateEncode) { - m_pData = (uint8_t*)m_Acc.GetData(); + m_pData = const_cast(m_Acc.GetData()); m_dwSize = m_Acc.GetSize(); m_pDict = pStream->GetDict(); return; } - m_bNewData = true; - m_bCloned = true; // TODO(thestig): Move to Init() and check return value. - ::FlateEncode(m_Acc.GetData(), m_Acc.GetSize(), &m_pData, &m_dwSize); - m_pDict = ToDictionary(pStream->GetDict()->Clone().release()); + uint8_t* buffer = nullptr; + ::FlateEncode(m_Acc.GetData(), m_Acc.GetSize(), &buffer, &m_dwSize); + m_pData = std::unique_ptr(buffer); + m_pDict = ToDictionary(pStream->GetDict()->Clone()); m_pDict->SetNewFor("Length", static_cast(m_dwSize)); m_pDict->SetNewFor("Filter", "FlateDecode"); m_pDict->RemoveFor("DecodeParms"); @@ -451,30 +444,22 @@ CPDF_FlateEncoder::CPDF_FlateEncoder(const uint8_t* pBuffer, uint32_t size, bool bFlateEncode, bool bXRefStream) - : m_pData(nullptr), - m_dwSize(0), - m_pDict(nullptr), - m_bCloned(false), - m_bNewData(false) { + : m_dwSize(0) { if (!bFlateEncode) { - m_pData = (uint8_t*)pBuffer; + m_pData = const_cast(pBuffer); m_dwSize = size; return; } - m_bNewData = true; + uint8_t* buffer = nullptr; // TODO(thestig): Move to Init() and check return value. if (bXRefStream) - ::PngEncode(pBuffer, size, &m_pData, &m_dwSize); + ::PngEncode(pBuffer, size, &buffer, &m_dwSize); else - ::FlateEncode(pBuffer, size, &m_pData, &m_dwSize); + ::FlateEncode(pBuffer, size, &buffer, &m_dwSize); + m_pData = std::unique_ptr(buffer); } -CPDF_FlateEncoder::~CPDF_FlateEncoder() { - if (m_bCloned) - delete m_pDict; - if (m_bNewData) - FX_Free(m_pData); -} +CPDF_FlateEncoder::~CPDF_FlateEncoder() {} class CPDF_Encryptor { public: @@ -583,7 +568,7 @@ FX_FILESIZE CPDF_ObjectStream::End(CPDF_Creator* pCreator) { CPDF_FlateEncoder encoder(tempBuffer.GetBuffer(), tempBuffer.GetLength(), true, false); CPDF_Encryptor encryptor(pCreator->m_pCryptoHandler, m_dwObjNum, - encoder.m_pData, encoder.m_dwSize); + encoder.m_pData.Get(), encoder.m_dwSize); if ((len = pFile->AppendDWord(encryptor.m_dwSize)) < 0) { return -1; } @@ -829,20 +814,21 @@ bool CPDF_XRefStream::GenerateXRefStream(CPDF_Creator* pCreator, bool bEOF) { offset += len; } } - if ((len = pFile->AppendString(">>stream\r\n")) < 0) { + if ((len = pFile->AppendString(">>stream\r\n")) < 0) return false; - } + offset += len; - if (pFile->AppendBlock(encoder.m_pData, encoder.m_dwSize) < 0) { + if (pFile->AppendBlock(encoder.m_pData.Get(), encoder.m_dwSize) < 0) return false; - } - if ((len = pFile->AppendString("\r\nendstream\r\nendobj\r\n")) < 0) { + + if ((len = pFile->AppendString("\r\nendstream\r\nendobj\r\n")) < 0) return false; - } + offset += encoder.m_dwSize + len; m_PrevOffset = offset_tmp; return true; } + bool CPDF_XRefStream::End(CPDF_Creator* pCreator, bool bEOF) { if (EndObjectStream(pCreator, bEOF) < 0) { return false; @@ -991,28 +977,29 @@ int32_t CPDF_Creator::WriteStream(const CPDF_Object* pStream, CPDF_CryptoHandler* pCrypto) { CPDF_FlateEncoder encoder(const_cast(pStream->AsStream()), pStream != m_pMetadata); - CPDF_Encryptor encryptor(pCrypto, objnum, encoder.m_pData, encoder.m_dwSize); - if ((uint32_t)encoder.m_pDict->GetIntegerFor("Length") != + CPDF_Encryptor encryptor(pCrypto, objnum, encoder.m_pData.Get(), + encoder.m_dwSize); + if (static_cast(encoder.m_pDict->GetIntegerFor("Length")) != encryptor.m_dwSize) { encoder.CloneDict(); encoder.m_pDict->SetNewFor( "Length", static_cast(encryptor.m_dwSize)); } - if (WriteDirectObj(objnum, encoder.m_pDict) < 0) { + if (WriteDirectObj(objnum, encoder.m_pDict.Get()) < 0) return -1; - } + int len = m_File.AppendString("stream\r\n"); - if (len < 0) { + if (len < 0) return -1; - } + m_Offset += len; - if (m_File.AppendBlock(encryptor.m_pData, encryptor.m_dwSize) < 0) { + if (m_File.AppendBlock(encryptor.m_pData, encryptor.m_dwSize) < 0) return -1; - } + m_Offset += encryptor.m_dwSize; - if ((len = m_File.AppendString("\r\nendstream")) < 0) { + if ((len = m_File.AppendString("\r\nendstream")) < 0) return -1; - } + m_Offset += len; return 1; } @@ -1104,28 +1091,28 @@ int32_t CPDF_Creator::WriteDirectObj(uint32_t objnum, case CPDF_Object::STREAM: { CPDF_FlateEncoder encoder(const_cast(pObj->AsStream()), true); - CPDF_Encryptor encryptor(m_pCryptoHandler, objnum, encoder.m_pData, + CPDF_Encryptor encryptor(m_pCryptoHandler, objnum, encoder.m_pData.Get(), encoder.m_dwSize); - if ((uint32_t)encoder.m_pDict->GetIntegerFor("Length") != + if (static_cast(encoder.m_pDict->GetIntegerFor("Length")) != encryptor.m_dwSize) { encoder.CloneDict(); encoder.m_pDict->SetNewFor( "Length", static_cast(encryptor.m_dwSize)); } - if (WriteDirectObj(objnum, encoder.m_pDict) < 0) { + if (WriteDirectObj(objnum, encoder.m_pDict.Get()) < 0) return -1; - } - if ((len = m_File.AppendString("stream\r\n")) < 0) { + + if ((len = m_File.AppendString("stream\r\n")) < 0) return -1; - } + m_Offset += len; - if (m_File.AppendBlock(encryptor.m_pData, encryptor.m_dwSize) < 0) { + if (m_File.AppendBlock(encryptor.m_pData, encryptor.m_dwSize) < 0) return -1; - } + m_Offset += encryptor.m_dwSize; - if ((len = m_File.AppendString("\r\nendstream")) < 0) { + if ((len = m_File.AppendString("\r\nendstream")) < 0) return -1; - } + m_Offset += len; break; } diff --git a/core/fpdfapi/parser/cpdf_stream.cpp b/core/fpdfapi/parser/cpdf_stream.cpp index 7a54fcf8ff..de69bfae7b 100644 --- a/core/fpdfapi/parser/cpdf_stream.cpp +++ b/core/fpdfapi/parser/cpdf_stream.cpp @@ -91,7 +91,7 @@ std::unique_ptr CPDF_Stream::CloneNonCyclic( pNewDict = ToDictionary( static_cast(pDict)->CloneNonCyclic(bDirect, pVisited)); } - return pdfium::MakeUnique(acc.DetachData(), streamSize, + return pdfium::MakeUnique(acc.DetachData().release(), streamSize, std::move(pNewDict)); } diff --git a/core/fpdfapi/parser/cpdf_stream_acc.cpp b/core/fpdfapi/parser/cpdf_stream_acc.cpp index 01d8e148df..423de7c571 100644 --- a/core/fpdfapi/parser/cpdf_stream_acc.cpp +++ b/core/fpdfapi/parser/cpdf_stream_acc.cpp @@ -74,14 +74,14 @@ uint32_t CPDF_StreamAcc::GetSize() const { return m_pStream ? m_pStream->GetRawSize() : 0; } -uint8_t* CPDF_StreamAcc::DetachData() { +std::unique_ptr CPDF_StreamAcc::DetachData() { if (m_bNewBuf) { - uint8_t* p = m_pData; + std::unique_ptr p(m_pData); m_pData = nullptr; m_dwSize = 0; return p; } - uint8_t* p = FX_Alloc(uint8_t, m_dwSize); - FXSYS_memcpy(p, m_pData, m_dwSize); + std::unique_ptr p(FX_Alloc(uint8_t, m_dwSize)); + FXSYS_memcpy(p.get(), m_pData, m_dwSize); return p; } diff --git a/core/fpdfapi/parser/cpdf_stream_acc.h b/core/fpdfapi/parser/cpdf_stream_acc.h index 654055f96b..24ae5d2ed8 100644 --- a/core/fpdfapi/parser/cpdf_stream_acc.h +++ b/core/fpdfapi/parser/cpdf_stream_acc.h @@ -7,6 +7,8 @@ #ifndef CORE_FPDFAPI_PARSER_CPDF_STREAM_ACC_H_ #define CORE_FPDFAPI_PARSER_CPDF_STREAM_ACC_H_ +#include + #include "core/fpdfapi/parser/cpdf_dictionary.h" #include "core/fpdfapi/parser/cpdf_stream.h" #include "core/fxcrt/fx_string.h" @@ -31,7 +33,7 @@ class CPDF_StreamAcc { uint32_t GetSize() const; const CFX_ByteString& GetImageDecoder() const { return m_ImageDecoder; } const CPDF_Dictionary* GetImageParam() const { return m_pImageParam; } - uint8_t* DetachData(); + std::unique_ptr DetachData(); protected: uint8_t* m_pData; diff --git a/core/fxcrt/cfx_maybe_owned.h b/core/fxcrt/cfx_maybe_owned.h index 76bd580e28..92b1c1c242 100644 --- a/core/fxcrt/cfx_maybe_owned.h +++ b/core/fxcrt/cfx_maybe_owned.h @@ -21,7 +21,7 @@ class CFX_MaybeOwned { public: CFX_MaybeOwned() : m_pObj(nullptr) {} explicit CFX_MaybeOwned(T* ptr) : m_pObj(ptr) {} - explicit CFX_MaybeOwned(std::unique_ptr ptr) + explicit CFX_MaybeOwned(std::unique_ptr ptr) : m_pOwnedObj(std::move(ptr)), m_pObj(m_pOwnedObj.get()) {} CFX_MaybeOwned(const CFX_MaybeOwned& that) = delete; @@ -30,7 +30,7 @@ class CFX_MaybeOwned { that.m_pObj = nullptr; } - void Reset(std::unique_ptr ptr) { + void Reset(std::unique_ptr ptr) { m_pOwnedObj = std::move(ptr); m_pObj = m_pOwnedObj.get(); } @@ -41,7 +41,7 @@ class CFX_MaybeOwned { bool IsOwned() const { return !!m_pOwnedObj; } T* Get() const { return m_pObj; } - std::unique_ptr Release() { + std::unique_ptr Release() { ASSERT(IsOwned()); return std::move(m_pOwnedObj); } @@ -57,7 +57,7 @@ class CFX_MaybeOwned { Reset(ptr); return *this; } - CFX_MaybeOwned& operator=(std::unique_ptr ptr) { + CFX_MaybeOwned& operator=(std::unique_ptr ptr) { Reset(std::move(ptr)); return *this; } @@ -65,13 +65,13 @@ class CFX_MaybeOwned { bool operator==(const CFX_MaybeOwned& that) const { return Get() == that.Get(); } - bool operator==(const std::unique_ptr& ptr) const { + bool operator==(const std::unique_ptr& ptr) const { return Get() == ptr.get(); } bool operator==(T* ptr) const { return Get() == ptr; } bool operator!=(const CFX_MaybeOwned& that) const { return !(*this == that); } - bool operator!=(const std::unique_ptr ptr) const { + bool operator!=(const std::unique_ptr ptr) const { return !(*this == ptr); } bool operator!=(T* ptr) const { return !(*this == ptr); } -- cgit v1.2.3