[Interest] Klocwork errors in Qt

Thiago Macieira thiago.macieira at intel.com
Wed Dec 4 01:04:16 CET 2019


On Tuesday, 3 December 2019 14:40:08 PST Christian Gagneraud wrote:
> > No, there's no need. QStringBuilder works just fine and has been working
> > for over 10 years. Either there's a problem somewhere in your code that
> > you didn't show to us or it s a Klocwork false positive.
> 
> To me it seems that  QStringBuilder is broken. 10 years ago there was
> no C++11, no auto.
> QStringBuilder *worked* just fine, but not anymore.

It didn't stop working. There's nothing wrong with it. There's just the 
pitfall of the auto.

If there are problems, please report them, with a way for me to reproduce and 
see what's wrong. The OP's report did not, it looks like a false positive.

> > Hint: do not use "auto" to detect the type when QStringBuilder is active.
> 
> Hint: don't tell C++ programmers that by using Qt you cannot use C++
> features as you wish.

You can use auto. Just don't use it with the result of a QStringBuilder, or 
then explicitly cast it to QString. 

> > Don't write:
> >     auto str = a + b;
> > 
> > instead, write:
> >     QString str = a + b;

You can also write:

    auto str = QString(a + b);

But that's more typing than just using QString on the left side.

Anyway, this problem is not exclusive to Qt. There's a lot of discussion in 
the standards community about fixing this somehow. Search for "operator auto" 
and you'll see those discussions and the arguments for and against. So far, 
there hasn't been a paper on the subject.

Alternatively, disable the automatic QStringBulder. That way, a + b will be a 
QStirng and auto will deduce the type correctly. You'll just have more memory 
allocations when concatenating three or more items, so poorer performance. If 
you ask me, that is such a low-hanging fruit that it's worth typing "QString" 
instead of "auto".

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

But with 10 years of history of this not being a big problem (all of which 
while GCC supported auto with -std=c++0x [*]), we need very strong arguments 
for a change.

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

It's optional.

> I honestly cannot accept that Qt is dictating me how to write C++ code
> because something is broken in Qt and the Qt people refuse to ack it.
> 
> Please prove me wrong.

It's not broken in Qt, it's a well-known C++-wide limitation. Therefore, 
there's nothing to acknowledge. Change is possible, but only with strong 
evidence why it's needed.

We're not dictating how you write C++. Use auto all you want. Just be careful 
of temporaries, like in the ranged-for:

	for (auto x : foo(bar())) { ... }

If foo() returns a reference to the argument it received, you're iterating 
over a dangling container. Not Qt's fault, it's just another well-known 
pitfall. The community is also discussing a possible solution for this, but 
again no realistic paper yet.

[*] gcc_4_4_branch, with support for auto, was created March 27, 2009. 
QStringBuilder was added March 28, 2009.
https://github.com/qt/qt/commit/3331605529d2d81145259fd56d03bb59c480cac5

-- 
Thiago Macieira - thiago.macieira (AT) intel.com
  Software Architect - Intel System Software Products





More information about the Interest mailing list