What amazes me while developing the static code analyzer

28 Nov 2011

Author: Evgenii Ryzhkov

When developing any programmer tool, be it a compiler or a static analyzer, or anything else, it is naturally that programmers use test projects the tool is constantly ran on. For example, when developing our static analyzer, we run it on 70 real projects. It allows us to make sure that everything is alright and nothing is broken. Besides, when we develop new diagnostic rules, we can see the code fragments where the new errors have been detected, after running the tests. Everything is logical and obvious. But why did I entitle the post in that way?

The point is that using the test base of projects is an established practice in our company. We wrote articles about errors in eMule, WinMerge, TortoiseSVN, etc. long ago and seem to know them through and through. But it turns out that we do not: again and again we manage to find new funny code fragments.

For example:

void CIrcMain::Disconnect(bool bIsShuttingDown)
{
 m_pIrcSocket->Close();
 if (m_pIrcSocket != NULL)
  delete m_pIrcSocket;
}

Well, the code even may work. In some cases. Sometimes. Actually when m_pIrcSocket is not equal to NULL. However, the author is not sure that m_pIrcSocket is always not equal to NULL and has added a check to verify that. But it goes AFTER the variable is used. Therefore, the program will crash if the code is executed with m_plrcSocket present, even despite the check that it is not equal to NULL.

Here is one more similar example:

  if (!frmExist)
  {
    frmExist = 
       tag->Find(ID3FID_SYNCEDLYRICS, ID3FN_DESCRIPTION, desc);
  }

  if (NULL != tag && NULL != data)
  {

The tag variable is dereferenced first and only then is checked. My question is the same: if the author is sure that tag is not equal to NULL, what for does he/she check it? And if he/she is not sure, then this check must go earlier in the code.

So, going back to the phrase I used as a title for this post. What amazes me is that though we have ALREADY found so many errors in our test projects, we are still finding new ones. It's logical on the one hand, for the analyzer is developing and consequently becomes able to find new issues. On the other hand it is amazing. We have examined all those test projects for so many times, so it seems there is nothing new to be found there. But there is.

Well, and the most obvious conclusion is: feel confident to run a static code analyzer on your project again and again because you may find new errors even in the old code, not to speak about the fresh code.