[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-devel
Subject:    Re: auto QString(Builder) considered VERY HARMFUL -> use clazy, especially before releases
From:       Albert Astals Cid <aacid () kde ! org>
Date:       2018-09-27 23:03:01
Message-ID: 192261953.7fyuCsQfMi () xps
[Download RAW message or body]

El dijous, 27 de setembre de 2018, a les 21:01:13 CEST, Friedrich W. H. Kossebau va \
escriure:
> Am Donnerstag, 27. September 2018, 20:08:54 CEST schrieb Andreas Hartmetz:
> > Today I fixed the third or so crash in KDE software due to the following
> > pattern:
> > 
> > const auto str = someString + anotherString;
> > 
> > What happens is that the type of str ends up being QStringBuilder
> > instead of QString. The QStringBuilder is destroyed after the semicolon,
> > and all access to "str" produces undefined behavior.
> 
> The QStringBuilder actually survives as long as str is in scope, but the 
> references it potentially takes from someString or anotherString (if e.g. some 
> temporary QString object as returned from QStringLiteral or other QString-
> returning methods) will be no longer valid after the scope of the expression 
> is left.
> So if str is finally resolved to a real QString, those references are dangling 
> and non-funny things happen.
> 
> > While I'm not a particularly big fan of using auto to hide variable
> > types anyway, this kind of usage is just wrong and must be avoided.
> > Please take care.
> 
> Care can be done e.g. by deploying clazy with auto-unexpected-qstringbuilder:
> 
> clazy-standalone \
> 	-checks=auto-unexpected-qstringbuilder  -p <buildroot>  <projectroot>
> 
> See
> https://phabricator.kde.org/source/clazy/browse/master/docs/checks/README-auto-unexpected-qstringbuilder.md?as=remarkup
>  https://phabricator.kde.org/source/clazy/browse/master/README.md?as=remarkup
> 
> One would recommend to run clazy over your code at least before releases, to 
> catch all kind of potential issues :)

Or since this is a crasher, just run your app and it'll crash?

Or even better, add autotests that exercise the code and they'll crash too?

Cheers,
  Albert

> 
> Cheers
> Friedrich
> 
> 
> 


[prev in list] [next in list] [prev in thread] [next in thread]