Suspicious code in kdegraphics-3.5.2
frans.englich at telia.com
Sat Apr 22 13:19:12 BST 2006
On Saturday 22 April 2006 11:16, Clarence Dang wrote:
> On Saturday 22 April 2006 09:36, Christoph Bartoschek wrote:
> > - kolourpaint/tools/kptoolselection.cpp:1095
> > - kolourpaint/tools/kptoolselection.cpp:1217
> > && true ?
> Left in for debugging - easy to change to "&& false".
I think better debugging practices exists. That code ends up in production
code and it is confusing.
I don't see any significant gain. The only difference is that one have to
change "false" instead of adding "&& false", the times one debug that
According to my standards, debugging code shouldn't be ordinary code, it
should either not end up in the repository in the first place, or be macros
disabled with NDEBUG.
> > If m_mainWindow is NULL as indicated by line 1637, then line 1646
> > crashes.
> "Will never happen". I am tempted simply to remove the "m_mainWindow"
> checks entirely as "m_mainWindow" can never be NULL.
If it's clear that it never can be null, such as that m_mainWindow is set in
the constructor and that it's deleted in the destructor, I suggest:
* Declare m_mainWindow to be a const pointer("QMainWindow *const
m_mainWindow;") and initialize it with constructor initialization.
* Add Q_ASSERTs do document and check that really is the case, in critical
paths. This should hardly be necessary if you declare the pointer const.
More information about the kde-core-devel