From a08cf99d066b16e4e16393efc15174193e002371 Mon Sep 17 00:00:00 2001 From: Chris Palmer Date: Wed, 23 Jul 2014 15:00:32 -0700 Subject: Refactor CFX_BasicArray. The |nGrowBy| argument to |SetSize| was always -1, which caused the effective m_nGrowBy value to always be its default value: 0. So it was not needed, and was cluttering up the logic. BUG=384662 Check for integer overflow in CFX_BasicArray. BUG=384662 R=bo_xu@foxitsoftware.com, rsesek@chromium.org Review URL: https://codereview.chromium.org/415803002 --- core/include/fxcrt/fx_basic.h | 18 ++++++++---------- core/src/fxcrt/extension.h | 2 +- core/src/fxcrt/fx_basic_array.cpp | 26 ++++++-------------------- 3 files changed, 15 insertions(+), 31 deletions(-) (limited to 'core') diff --git a/core/include/fxcrt/fx_basic.h b/core/include/fxcrt/fx_basic.h index c400a940ff..ece2b43a2e 100644 --- a/core/include/fxcrt/fx_basic.h +++ b/core/include/fxcrt/fx_basic.h @@ -350,7 +350,7 @@ protected: ~CFX_BasicArray(); - FX_BOOL SetSize(int nNewSize, int nGrowBy); + FX_BOOL SetSize(int nNewSize); FX_BOOL Append(const CFX_BasicArray& src); @@ -371,8 +371,6 @@ protected: int m_nMaxSize; - int m_nGrowBy; - int m_nUnitSize; }; template @@ -391,14 +389,14 @@ public: return m_nSize - 1; } - FX_BOOL SetSize(int nNewSize, int nGrowBy = -1) + FX_BOOL SetSize(int nNewSize) { - return CFX_BasicArray::SetSize(nNewSize, nGrowBy); + return CFX_BasicArray::SetSize(nNewSize); } void RemoveAll() { - SetSize(0, -1); + SetSize(0); } const TYPE GetAt(int nIndex) const @@ -442,7 +440,7 @@ public: return FALSE; } if (nIndex >= m_nSize) - if (!SetSize(nIndex + 1, -1)) { + if (!SetSize(nIndex + 1)) { return FALSE; } ((TYPE*)m_pData)[nIndex] = newElement; @@ -453,7 +451,7 @@ public: { if (m_nSize < m_nMaxSize) { m_nSize ++; - } else if (!SetSize(m_nSize + 1, -1)) { + } else if (!SetSize(m_nSize + 1)) { return FALSE; } ((TYPE*)m_pData)[m_nSize - 1] = newElement; @@ -616,7 +614,7 @@ public: return 0; } RemoveAll(); - SetSize(nCount, -1); + SetSize(nCount); ObjectClass* pStartObj = (ObjectClass*)m_pData; nSize = nStart + nCount; for (FX_INT32 i = nStart; i < nSize; i ++, pStartObj++) { @@ -653,7 +651,7 @@ public: for (int i = 0; i < m_nSize; i ++) { ((ObjectClass*)GetDataPtr(i))->~ObjectClass(); } - CFX_BasicArray::SetSize(0, -1); + CFX_BasicArray::SetSize(0); } }; typedef CFX_ObjectArray CFX_ByteStringArray; diff --git a/core/src/fxcrt/extension.h b/core/src/fxcrt/extension.h index a736425d57..a2d0a1462f 100644 --- a/core/src/fxcrt/extension.h +++ b/core/src/fxcrt/extension.h @@ -358,7 +358,7 @@ protected: } FX_INT32 iCount = m_Blocks.GetSize(); size = (size - m_nTotalSize + m_nGrowSize - 1) / m_nGrowSize; - m_Blocks.SetSize(m_Blocks.GetSize() + (FX_INT32)size, -1); + m_Blocks.SetSize(m_Blocks.GetSize() + (FX_INT32)size); while (size --) { FX_LPBYTE pBlock = FX_Alloc(FX_BYTE, m_nGrowSize); if (!pBlock) { diff --git a/core/src/fxcrt/fx_basic_array.cpp b/core/src/fxcrt/fx_basic_array.cpp index f65d8efcd9..0694cf9cbd 100644 --- a/core/src/fxcrt/fx_basic_array.cpp +++ b/core/src/fxcrt/fx_basic_array.cpp @@ -11,7 +11,6 @@ CFX_BasicArray::CFX_BasicArray(int unit_size) : m_pData(NULL) , m_nSize(0) , m_nMaxSize(0) - , m_nGrowBy(0) { if (unit_size < 0 || unit_size > (1 << 28)) { m_nUnitSize = 4; @@ -23,7 +22,7 @@ CFX_BasicArray::~CFX_BasicArray() { FX_Free(m_pData); } -FX_BOOL CFX_BasicArray::SetSize(int nNewSize, int nGrowBy) +FX_BOOL CFX_BasicArray::SetSize(int nNewSize) { if (nNewSize <= 0) { FX_Free(m_pData); @@ -32,8 +31,6 @@ FX_BOOL CFX_BasicArray::SetSize(int nNewSize, int nGrowBy) return 0 == nNewSize; } - m_nGrowBy = nGrowBy >= 0 ? nGrowBy : m_nGrowBy; - if (m_pData == NULL) { base::CheckedNumeric totalSize = nNewSize; totalSize *= m_nUnitSize; @@ -53,18 +50,7 @@ FX_BOOL CFX_BasicArray::SetSize(int nNewSize, int nGrowBy) } m_nSize = nNewSize; } else { - int nGrowBy = m_nGrowBy; - if (nGrowBy == 0) { - nGrowBy = m_nSize / 8; - nGrowBy = (nGrowBy < 4) ? 4 : ((nGrowBy > 1024) ? 1024 : nGrowBy); - } - int nNewMax; - if (nNewSize < m_nMaxSize + nGrowBy) { - nNewMax = m_nMaxSize + nGrowBy; - } else { - nNewMax = nNewSize; - } - + int nNewMax = nNewSize < m_nMaxSize ? m_nMaxSize : nNewSize; base::CheckedNumeric totalSize = nNewMax; totalSize *= m_nUnitSize; if (!totalSize.IsValid() || nNewMax < m_nSize) { @@ -86,7 +72,7 @@ FX_BOOL CFX_BasicArray::Append(const CFX_BasicArray& src) int nOldSize = m_nSize; base::CheckedNumeric newSize = m_nSize; newSize += src.m_nSize; - if (m_nUnitSize != src.m_nUnitSize || !newSize.IsValid() || !SetSize(newSize.ValueOrDie(), -1)) { + if (m_nUnitSize != src.m_nUnitSize || !newSize.IsValid() || !SetSize(newSize.ValueOrDie())) { return FALSE; } @@ -95,7 +81,7 @@ FX_BOOL CFX_BasicArray::Append(const CFX_BasicArray& src) } FX_BOOL CFX_BasicArray::Copy(const CFX_BasicArray& src) { - if (!SetSize(src.m_nSize, -1)) { + if (!SetSize(src.m_nSize)) { return FALSE; } FXSYS_memcpy32(m_pData, src.m_pData, src.m_nSize * m_nUnitSize); @@ -107,12 +93,12 @@ FX_LPBYTE CFX_BasicArray::InsertSpaceAt(int nIndex, int nCount) return NULL; } if (nIndex >= m_nSize) { - if (!SetSize(nIndex + nCount, -1)) { + if (!SetSize(nIndex + nCount)) { return NULL; } } else { int nOldSize = m_nSize; - if (!SetSize(m_nSize + nCount, -1)) { + if (!SetSize(m_nSize + nCount)) { return NULL; } FXSYS_memmove32(m_pData + (nIndex + nCount)*m_nUnitSize, m_pData + nIndex * m_nUnitSize, -- cgit v1.2.3