1berry/qtbase
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

QBrush: symmetrize QBrushData subclasses

Browse Source 
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>
This commit is contained in:
Marc Mutz 2016-10-06 09:45:04 +02:00 committed by Marc Mutz
parent 511ee39570
commit 13965b020e
1 changed files with 10 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

15
src/gui/painting/qbrush.cpp
View File

@ -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