[Interest] Klocwork errors in Qt

Christian Gagneraud chgans at gmail.com
Wed Dec 4 02:36:37 CET 2019


On Wed, 4 Dec 2019 at 13:05, Thiago Macieira <thiago.macieira at intel.com> wrote:
> > IMHO, it is legit to write code like:
> > const QUrl url(urlString);
> > const auto appId = url.host() + "." + url.path().mid(1);
> > doSomeThing(appId); // void doSomeThing(const QString &stuff);
> >
> > Yet it results in crashes, apparently because of QStringBuilder
> > holding references to temporary implicitly shared Qt object. I would
> > expect QStringBuilder to somehow grab a reference to this implicitely
> > shared object (eg. by incrementing a ref counter).
>
> We'd lose performance on the common case by doing atomic reference counting up
> and down. QStringBuilder would need to hold (shallow, implicitly-shared)
> copies of the QString values returned by QUrl::host() and QString::mid(). One
> can argue that the cost is reasonable and worth the extra safety it gives.

My point is that this sort of bugs are really hard to detect during
code review. This is subtle & nasty 'feature' of QStringBuilder (a
choice in its implementation).
QStringBuilder has no documentation (that i could find), and there are
no 'BIG WARNING' mention in
https://doc.qt.io/qt-5/qstring.html#more-efficient-string-construction

BTW, Clazy has a check for that very specific case, which just show
how dangerous is this QStringBuilder.
https://github.com/KDE/clazy/blob/master/docs/checks/README-auto-unexpected-qstringbuilder.md

> > AFAIU, QT_USE_QSTRINGBUILDER is a dangerous build option, don't use it.
>
> It's optional.

optional and dangerous are orthogonal concepts! :)

Chris


More information about the Interest mailing list