CppHints joined "How Not 2 Code" blog!

Read more programming tips and tricks from PVS-Studio team,
Learn from examples of bad coding practices
of various open source projects on
Hownot2code.com






Holy war against bugs - episode 6.10. New hope.

Posted by CppHints Team    |    On November 2, 2016    |    Permalink

All software developers around world are constantly in a battle.

The most important campaign goal - clean out the code from bugs and not break the system.

But there is hope. Static code analyzers are the tools that provide a secure flank cover in this mortal combat.

There is more - Linux world also got additional protection - now PVS-Studio static code analyzer can be run on Linux as well. It has already checked several projects and found bugs in them -

First and foremost - bugs in the Linux kernel.

We invite everybody to try it on the code - http://www.viva64.com/en/pvs-studio-download-linux/

Bug-free coding to you!

CppHints joined "How Not 2 Code" blog!

Posted by CppHints Team    |    On October 10, 2016    |    Permalink

We are happy to announce that CppHints articles are now a part of "How Not 2 Code" blog! (https://hownot2code.com/)

What's in the blog:

  • C/C ++/C# Programming tips and tricks from PVS-Studio team;
  • Bad coding practices found in various open source projects, so that we can learn by looking at bad examples (bug reports of Mono, Linux kernel, Qt, GCC and many more);
  • Posts are sorted by categories - Bugs in C++/C# projects, Programming tips and tricks, 64-bit issues, Development of 64-bit C/C++ applications.
  • Ability to comment on the posts and share your ideas/objections.
  • Online quizzes to test your programming knowledge and skills!

You can also subscribe to get information about new posts of How Not 2 Code blog.

Bug - free code to you!

C++Hints is taking a break

Posted by Andrey Karpov    |    On January 22, 2016    |    Permalink

We made a decision to take a break in maintaining C++Hints for some time or permanently. We are taking into account that our readers sent us positive feedback and we hope that they enjoyed this project. Big thanks to everybody who supported us.

But unfortunately, at this point our company doesn't have enough resources to keep running several projects at the same time.

We'll still post articles in our company blog about the projects we check, giving some recommendations of how to avoid bugs http://www.viva64.com/en/b/.

RSS of the blog: http://feeds.feedburner.com/viva64-blog-en

We also suggest following our CTO, Andrey Karpov, @Code_Analysis on Twitter to be in the know of the latest C++ news.

Use nullptr instead of NULL from now on

Posted by Andrey Karpov    |    On January 15, 2016    |    Permalink

New C++ standards brought quite a lot of useful changes. There are things that I would not rush into using straight away, but there are some changes which need to be applied immediately, as they will bring significant benefits.

One such modernization is the keyword 'nullptr', which is intended to replace the NULL macro.

Let me remind you that in C++ the definition of NULL is 0, nothing more.

Of course, it may seem that this is just some syntactic sugar. And what's the difference, if we write nullptr or NULL? But there is a difference! Using nullptr helps to avoid a big variety of errors.

I'll show it using examples.

Suppose, there are two overloaded functions:

void Foo(int x, int y, const char *name);
void Foo(int x, int y, int ResourceID);

A programmer might write the following call:

Foo(1, 2, NULL);

And that same programmer might be sure that he is in fact calling the first function by doing this. It is not so. As NULL is nothing more than 0, and zero is known to have 'int' type, the second function will be called instead of the first.

However, if the programmer had used 'nullptr' no such error would occur and the first function would have been called.

Another common enough use of NULL is to write code like this:

if (unknownError)
  throw NULL;

To my mind it is suspicious to generate an exception passing the pointer. Nevertheless sometimes people do so. Apparently the developer needed to write the code in this way. However, discussions whether it is a good or bad practice to do so go beyond the scope of this note.

What is important is that the programmer decided to generate an exception in the case of an unknown error and "send" a null pointer into the outher world.

In fact it is not a pointer but 'int'. As a result the exception handling will happen in the way that the programmer didn't expect it to.

"throw nullptr;" code saves us from misfortune, but this does not mean that I believe this code to be totally acceptable.

In some cases, if you use 'nullptr', the incorrect code will not compile.

Suppose that some WinApi function returns an HRESULT type. The HRESULT type has nothing to do with the pointer. However, it is quite possible to write nonsensical code like this:

if (WinApiFoo(a, b, c) != NULL)

This code will compile, because NULL is 0 and of 'int' type, and HRESULT is a 'long' type. It is quite possible to compare type values 'int' and 'long'. If you use 'nullptr', then the following code will not compile:

If (WinApiFoo (a, b, c)! = nullptr)

Thus, the error will be immediately noticed and fixed.

I think you get the idea. There are plenty of such examples. But these are mostly synthetic examples. And it is always not very convincing. So are there any real examples? Yes, there are. Here is one of them. The only thing - it's not very graceful or short.

This code is taken from the MTASA project.

We know that there exists RtlFillMemory() This can be a real function or simply a macro. It doesn't matter. It is similar to the memset() function, but the 2nd and 3rd argument switched their places. Here's how this macro can be declared:

#define RtlFillMemory(Destination,Length,Fill) \
  memset((Destination),(Fill),(Length))

There is also FillMemory(), which is nothing more than RtlFillMemory():

#define FillMemory RtlFillMemory

Yes, everything is long and complicated. But at least it is an example or real erroneous code.

And here's the code that uses the FillMemory macro.

LPCTSTR __stdcall GetFaultReason ( EXCEPTION_POINTERS * pExPtrs )
{
  ....
  PIMAGEHLP_SYMBOL pSym = (PIMAGEHLP_SYMBOL)&g_stSymbol ;
  FillMemory ( pSym , NULL , SYM_BUFF_SIZE ) ;
  ....
}

This code fragment has even more bugs. We clearly see that at least the 2 and 3 argument are confused here. That's why the analyzer issues 2 warnings V575:

  • V575 The 'memset' function processes value '512'. Inspect the second argument. crashhandler.cpp 499
  • V575 The 'memset' function processes '0' elements. Inspect the third argument. crashhandler.cpp 499

The code compiled because NULL is 0. As a result, 0 array elements get filled.

But in fact the error is not only about this. NULL is in general not appropriate here. The memset() function works with bytes, so there's no point in trying to make it fill the memory with NULL values. This is some absurdity. Correct code should be like this :

FillMemory(pSym, SYM_BUFF_SIZE, 0);

Or like this:

ZeroMemory(pSym, SYM_BUFF_SIZE);

But it's not the main point, which is that this meaningless code compiles successfully. However, if the programmer had gotten into the habit of using ''nullptr'' instead of ''NULL'' and written this instead:

FillMemory(pSym, nullptr, SYM_BUFF_SIZE);

the complier would have emitted a diagnostic, and the programmer would realize that they did something wrong and would pay more attention to the way they code.

Note. I understand that in this case NULL is not to blame. However, it is because of NULL the code compiles without any warnings.

Recommendation

Start using nullptr. Right now. And make necessary changes in the coding standard of your company.

Using nullptr will help to avoid stupid errors and thus will speed up the development process.


  

PVS-Studio analyzer is expanding the horizons

Posted by Andrey Karpov    |    On December 23, 2015    |    Permalink

This bug was found in GIT source code.

Incorrect code:

public int GetMethodsInDocument(
  ISymUnmanagedDocument document,
  int bufferLength, 
  out int count,
  ....)
{
  ....
  if (bufferLength > 0)
  {
    ....
    count = actualCount;
  }
  else
  {
    count = extentsByMethod.Length;
  }
  count = 0;
  return HResult.S_OK;
}

Explanation:

The function always returns 0 through the 'count' variable. Most likely the last line "count = 0" is redundant and accidentally got into the code.

You may say "Wait a second, this is C#!" You're absolutely right. This time let me speak on a different topic.

There happened a big event in our company. We have released the 6th version of PVS-Studio analyzer that now has C# support. So we would like to share this news. I hope you won't be offended that I wrote about it here.

There is no better way to show the abilities of the analyzer than to provide examples of real bugs found. We have several articles about checks of C# projects that I suggest reading. I suppose that among our readers there are not only C++ programmers, but those who also code in C#.

Umbraco project check: http://www.viva64.com/en/b/0357/

SharpDevelop project check: http://www.viva64.com/en/b/0359/

Microsoft Code Contracts project check: http://www.viva64.com/en/b/0361/

Roslyn: http://www.viva64.com/en/b/0363/

To those who may be interested - the new analyzer is available here. You can download and run it on your project.

Recommendation:

It is important to use static code analysis if you want to have safe and bug-free code. This is the only way to truly experience the benefits that it brings.

I am sure that when you check your project, many of the bugs will be related to the tests or error handlers. This is natural. Errors in frequently used code fragments often get corrected by thorough testing and reviews by users. But it is the fact itself that PVS-Studio can catch those bugs that matters. It means that many bugs, which took much time and effort to fix, could be fixed immediately by using PVS-Studio on a regular basis.

The same idea in other words. It is regular use that makes the analyzer valuable, not casual runs. View static code analysis as an extension to compiler warnings. It's not a good idea to enable compiler warnings once a year. You have to address the bugs as soon as they appear. It helps save development time on tracking down silly mistakes through debugging. It's just the same thing with static analyzers.

Of course, it cannot catch all the bugs. But instead of wasting time hunting for typos and slip-ups, spend it on something of a bigger worth. Let PVS-Studio deal with "stupid" errors.

P.S. You don't make stupid mistakes? Well, well! Sometimes it may seem that there are none. Everyone does. Just have a look here.

This error was found by PVS-Studio static analysis tool. The error text: V3008 The 'count' variable is assigned values twice successively. Perhaps this is a mistake.