Suspicous code in KDE 4 trunk r764353

[Christoph Bartoschek]

Am Dienstag 22 Januar 2008 schrieb Michael Pyne:
> On Tuesday 22 January 2008, Christoph Bartoschek wrote:
> > Here is what is left from the list:
> > -------------------------------------
> > - kdemultimedia/juk/sortedstringlist.cpp:163
> >
> > Line 153 indicates that previousNode could be NULL here.
>
> previousNode cannot be NULL at 163.  The logic is:
>
> For that branch to run, m_root must have been non-zero.
> Line 141 assigns node := m_root.  So if m_root was non-zero, so is node.
> At line 143, if node is non-zero (true iff m_root was non-zero),
> previousNode is set to a non-zero value (node).
>
> So the only way previousValue would be dereferenced here is if it were
> proven non-zero.
>
> However this shouldn't need to be proved everytime the code is read.  Is
> assertion checking enough to quiet the check?

Yes, for this tool assertions are the perfect way to make it silent.

But what about removing the lines:

if(previousNode && value == previousNode->key)
   return true;

The if-statement is only true if previousNode is not NULL and value == 
previousNode->key. The variable previousNode is only set to node in line 
144. But then value == previousNode->key is equivalent to value == 
node->key. And if the first is true then line 145 is also true and returns 
before line 153 can be reached.

The tool only reports the error because of the check in line 153. Without the 
check there would also be no complaint.

Greetings
Christoph