Guard against unsafe usage of QFrameAllocator::(de)allocateRawMemory.

Before one could potentially fall into a trap when one would call

allocateRawMemory<BaseClass>(size);

When size belongs to a child class inheriting from BaseClass and
falls into a different (larger) bucket, the following deallocation
via

deallocateRawMemory<BaseClass>()

would fail. Because there we just picked the size of the BaseClass
to find the bucket.

Now the whole API is restructured to guard against this wrong usage:

- we only ever operator on void* in the *Raw* methods
- we pass on the size explictily

This also means we must use the safe operator deallocate overloads
that also take the size as a parameter.

The unit test was run first to check that this was indeed an issue.
Then it is ported to the new API and cleanup up a bit as well to
reduce duplicated code.

Change-Id: I559af1c0ffc633962fac12d6e3cadf87c09ef92d
Reviewed-by: Paul Lemire <paul.lemire@kdab.com>
Reviewed-by: Sean Harmer <sean.harmer@kdab.com>

1 files changed, 18 insertions, 19 deletions

diff --git a/tests/auto/core/qframeallocator/tst_qframeallocator.cpp b/tests/auto/core/qframeallocator/tst_qframeallocator.cpp
index 4933524b4..7ef9a758c 100644
--- a/tests/auto/core/qframeallocator/tst_qframeallocator.cpp
+++ b/tests/auto/core/qframeallocator/tst_qframeallocator.cpp
@@ -80,6 +80,8 @@ public:
     struct subclass : public composed
     {
         float toto;
+        // add some more data to force the subclass into a different bucket
+        char data[32];
     };
 
 private slots:
@@ -608,7 +610,7 @@ void tst_QFrameAllocator::allocateSubclass()
     for (int i = 0; i < 256; i++) {
         // Allocate a composed object of size subclass
         // c is actually a subclass
-        composed *c = f.allocateRawMemory<composed>(sizeof(subclass));
+        composed *c = static_cast<composed*>(f.allocateRawMemory(sizeof(subclass)));
         composeds << c;
     }
 
@@ -629,30 +631,27 @@ void tst_QFrameAllocator::deallocateSubclass()
 {
     Qt3D::QFrameAllocator f(128, 32);
 
-    QList<composed *> composeds;
-
-    for (int i = 0; i < 256; i++) {
-        // Allocate a composed object of size subclass
-        // c is actually a subclass
-        composed *c = f.allocateRawMemory<composed>(sizeof(subclass));
-        composeds << c;
-    }
 
-    for (int l = 0; l < 5; l++) {
+    const int NUM_ITEMS = 256;
+    QVector<void *> allocated(NUM_ITEMS);
+    uint chunkCount = 0;
 
-        uint chunkCount = f.totalChunkCount();
+    for (int l = 0; l < 6; l++) {
 
-        for (int i = 0; i < 256; i++) {
-            f.deallocateRawMemory(composeds.takeLast());
-        }
-
-        for (int i = 0; i < 256; i++) {
+        for (int i = 0; i < NUM_ITEMS; i++) {
             // Allocate a composed object of size subclass
             // c is actually a subclass
-            composed *c = f.allocateRawMemory<composed>(sizeof(subclass));
-            composeds << c;
+            allocated[i] = f.allocateRawMemory(sizeof(subclass));
+        }
+
+        if (!chunkCount)
+            chunkCount = f.totalChunkCount();
+        else
+            QCOMPARE(chunkCount, f.totalChunkCount());
+
+        for (int i = 0; i < NUM_ITEMS; i++) {
+            f.deallocateRawMemory(allocated[i], sizeof(subclass));
         }
-        QCOMPARE(chunkCount, f.totalChunkCount());
     }
 
 }