The End-of-file (EOF) check may not be enough

The fragment is taken from SETI@home project. The error is detected by the following diagnostic: V663 Infinite loop is possible. The ‘cin.eof()’ condition is insufficient to break from the loop. Consider adding the ‘cin.fail()’ function call to the conditional expression.

template <typename T>
std::istream &operator >>(std::istream &i, sqlblob<T> &b) 
{
  ....
  while (!i.eof()) 
  {
    i >> tmp;
    buf+=(tmp+' ');
  }
  ....
}

Explanation

The operation of reading data from a stream object is not as trivial as it may seem at first. When reading data from streams, programmers usually call the eof() method to check if the end of stream has been reached. This check, however, is not quite adequate as it is not sufficient and doesn’t allow you to find out if any data reading errors or stream integrity failures have occurred, which may cause certain issues.

Note. The information provided in this note concerns both input and output streams. To avoid repetition, we’ll only discuss one type of stream here.

This is exactly the mistake the programmer made in the code sample above: in the case of there being any data reading error, an infinite loop may occur as the eof() method will always return false. On top of that, incorrect data will be processed in the loop, as unknown values will be getting to the tmp variable.

1454370039134576502

To avoid issues like that, we need to use additional methods to check the stream status: bad(), fail().

Correct code

Let’s take advantage of the fact that the stream can implicitly cast to the bool type. The true value indicates that the value is read successfully. More details about the way this code works can be found on StackOverflow.

template <typename T>
std::istream &operator >>(std::istream &i, sqlblob<T> &b) 
{
  ....
  while (i >> tmp) 
  {
    buf+=(tmp+' ');
  }
  ....
}

Recommendation

When reading data from a stream, don’t use the eof() method only; check for any failures, too.

Use the methods bad() and fail() to check the stream status. The first method is used to check stream integrity failures, while the second is for checking data reading errors.

However, it’s much more convenient to use bool() operator, as it is shown in the example of the correct code.

Written by Andrey Karpov.

This error was found with PVS-Studio static analysis tool.

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s