Refactor CFX_BasicArray.
authorChris Palmer <palmer@google.com>
Wed, 23 Jul 2014 22:00:32 +0000 (15:00 -0700)
committerChris Palmer <palmer@google.com>
Wed, 23 Jul 2014 22:00:32 +0000 (15:00 -0700)
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
core/src/fxcrt/extension.h
core/src/fxcrt/fx_basic_array.cpp

index c400a94..ece2b43 100644 (file)
@@ -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<class TYPE>
@@ -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_ByteString> CFX_ByteStringArray;
index a736425..a2d0a14 100644 (file)
@@ -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) {
index f65d8ef..0694cf9 100644 (file)
@@ -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<int> 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<int> 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<int> 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,