From 13965b020ee2a615e414d2a58388f018607ba418 Mon Sep 17 00:00:00 2001
From: Marc Mutz <marc.mutz@kdab.com>
Date: Thu, 6 Oct 2016 09:45:04 +0200
Subject: [PATCH] QBrush: symmetrize QBrushData subclasses

Coverity complains that QBrushData, a class with no virtual
destructor, has subclasses, but is in some code paths deleted as a
QBrushData, which, if the object deleted was actually an instance of a
derived class, would be undefined behavior.

This warning has already unearthed a real bug, since fixed in
8082c0dc81b50c44a0cf1984cb2c484b007c64a4, but it became clear that
Coverity does not see through the tying of d->style to a particular
subclass, probably because we're mixing hierarchy levels.

So try to guide Coverity better by inheriting an empty subclass from
QBrushData for those styles that are neither textures nor gradients,
and create and destroy only derived classes now.

This greatly simplifies the analysis, since to check this patch for
correctness, I basically only needed to make the QBrushData ctor and
dtor protected (something we should eventually do, but is not included
in this patch, yet (might be BiC)).

Coverity-Id: 11772
Coverity-Id: 218724
Change-Id: I374a0e45160f6ef16f813d782fecc797966ddf78
Reviewed-by: Volker Hilsheimer <volker.hilsheimer@qt.io>
---
 src/gui/painting/qbrush.cpp | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/src/gui/painting/qbrush.cpp b/src/gui/painting/qbrush.cpp
index b2f1631f21c..bf013f57574 100644
--- a/src/gui/painting/qbrush.cpp
+++ b/src/gui/painting/qbrush.cpp
@@ -148,6 +148,10 @@ Q_GUI_EXPORT QImage qt_imageForBrush(int brushStyle, bool invert)
     return qt_brushPatternImageCache()->getImage(brushStyle, invert);
 }
 
+struct QBasicBrushData : public QBrushData
+{
+};
+
 struct QTexturedBrushData : public QBrushData
 {
     QTexturedBrushData() {
@@ -224,7 +228,8 @@ static void deleteData(QBrushData *d)
         delete static_cast<QGradientBrushData*>(d);
         break;
     default:
-        delete d;
+        delete static_cast<QBasicBrushData*>(d);
+        break;
     }
 }
 
@@ -318,8 +323,8 @@ void QBrushDataPointerDeleter::operator()(QBrushData *d) const noexcept
 class QNullBrushData
 {
 public:
-    QBrushData *brush;
-    QNullBrushData() : brush(new QBrushData)
+    QBasicBrushData *brush;
+    QNullBrushData() : brush(new QBasicBrushData)
     {
         brush->ref.storeRelaxed(1);
         brush->style = Qt::BrushStyle(0);
@@ -377,7 +382,7 @@ void QBrush::init(const QColor &color, Qt::BrushStyle style)
         d.reset(new QGradientBrushData);
         break;
     default:
-        d.reset(new QBrushData);
+        d.reset(new QBasicBrushData);
         break;
     }
     d->ref.storeRelaxed(1);
@@ -586,7 +591,7 @@ void QBrush::detach(Qt::BrushStyle newStyle)
         break;
         }
     default:
-        x.reset(new QBrushData);
+        x.reset(new QBasicBrushData);
         break;
     }
     x->ref.storeRelaxed(1); // must be first lest the QBrushDataPointerDeleter turns into a no-op