QFixedFrameAllocator: Extract Method scan()

The old code used indices to detect when it fell off the
end of m_chunks and needed to allocate a new chunk, but
this was opaque to Coverity, which saw the potential for
m_lastAllocatedChunk == nullptr at the end of the function,
and thus reported a nullptr deref there.

Fix by moving the scanning of m_chunks into a new method,
scan(), which more clearly communicates that it never
returns nullptr, not least because it returns by reference
instead of pointer.

Coverity-Id: 154279
Change-Id: I0cfe8fd819bbfc5b03a98b5e9354c0e98a521d34
Reviewed-by: Paul Lemire <paul.lemire@kdab.com>

2 files changed, 25 insertions, 17 deletions

diff --git a/src/core/resources/qframeallocator.cpp b/src/core/resources/qframeallocator.cpp
index d2527000d..6eb904014 100644
--- a/src/core/resources/qframeallocator.cpp
+++ b/src/core/resources/qframeallocator.cpp
@@ -144,25 +144,30 @@ void QFixedFrameAllocator::init(uint blockSize, uchar pageSize)
 
 void *QFixedFrameAllocator::allocate()
 {
-    Q_ASSERT(m_blockSize && m_nbrBlock);
-    if (m_lastAllocatedChunck == Q_NULLPTR ||
-            m_lastAllocatedChunck->m_blocksAvailable == 0) {
-        int i = 0;
-        for (; i < m_chunks.size(); i++) {
-            if (m_chunks[i].m_blocksAvailable > 0) {
-                m_lastAllocatedChunck = m_chunks.begin() + i;
-                break;
-            }
-        }
-        if (i == m_chunks.size()) {
-            m_chunks.resize(m_chunks.size() + 1);
-            QFrameChunk &newChunk = m_chunks.last();
-            newChunk.init(m_blockSize, m_nbrBlock);
-            m_lastAllocatedChunck = &newChunk;
-            m_lastFreedChunck = m_lastAllocatedChunck;
+    Q_ASSERT(m_blockSize);
+    return scan().allocate(m_blockSize);
+}
+
+QFrameChunk &QFixedFrameAllocator::scan()
+{
+    Q_ASSERT(m_blockSize);
+    Q_ASSERT(m_nbrBlock);
+
+    if (m_lastAllocatedChunck && m_lastAllocatedChunck->m_blocksAvailable)
+        return *m_lastAllocatedChunck;
+
+    for (int i = 0; i < m_chunks.size(); i++) {
+        if (m_chunks[i].m_blocksAvailable > 0) {
+            m_lastAllocatedChunck = m_chunks.begin() + i;
+            return *m_lastAllocatedChunck;
         }
     }
-    return m_lastAllocatedChunck->allocate(m_blockSize);
+    m_chunks.resize(m_chunks.size() + 1);
+    QFrameChunk &newChunk = m_chunks.last();
+    newChunk.init(m_blockSize, m_nbrBlock);
+    m_lastAllocatedChunck = &newChunk;
+    m_lastFreedChunck = &newChunk;
+    return newChunk;
 }
 
 void QFixedFrameAllocator::deallocate(void *ptr)
diff --git a/src/core/resources/qframeallocator_p_p.h b/src/core/resources/qframeallocator_p_p.h
index 3e6cdf4d8..2dea2dc23 100644
--- a/src/core/resources/qframeallocator_p_p.h
+++ b/src/core/resources/qframeallocator_p_p.h
@@ -93,6 +93,9 @@ public:
     inline uint blockSize() const { return m_blockSize; }
 
 private:
+    QFrameChunk &scan();
+
+private:
     uint m_blockSize;
     uchar m_nbrBlock;
     QVector<QFrameChunk> m_chunks;