IneffectiveReadByte

IneffectiveReadByte

Summary

Avoid invoking System.IO.BinaryReader.ReadByte() method in a loop. Instead, use Read(byte[], int, int) method.

Default severity

Warning

Description

This analyzer reports diagnostics for the following code:

for (expr1; expr2; expr3)
{
    byteArray[i] = binaryReader.ReadByte();
}

where:

  • byteArray can be any byte[] variable or auto-implemented property returning byte[]
  • binaryReader can be any System.IO.BinaryReader variable or auto-implemented property returning System.IO.BinaryReader
  • i can be any int variable, but it must be declared in expr1
  • expr1 must be int i = START or var i = START
  • expr2 must be i < END or i <= END
  • expr3 must be ++i or i++
  • START and END are constant integers, and START is less than or equal to END

because it is ineffective and can be replaced with more effective one invoking Read(byte[], int, int).

For example, following code invoking ReadByte() method in the for loop is reported with the diagnostic:

BinaryReader reader = ...;
byte[] buffer = ...;

for (var i = 0; i < 1000; ++i)
{
    buffer[i] = reader.ReadByte();
}

The for loop and invoking ReadByte() method can be replaced with the readFully-like code as follows:

BinaryReader reader = ...;
byte[] buffer = ...;

var offset = 0;
var length = 1000;
while (length > 0)
{
    var size = reader.Read(buffer, offset, length);
    if (size == 0)
    {
        throw new EndOfStreamException();
    }
    offset += size;
    length -= size;
}

If the underlying stream reader.BaseStream has always available data except for end of stream, it is more simply rewritten as follows:

BinaryReader reader = ...;
byte[] buffer = ...;

var size = reader.Read(buffer, 0, 1000);
if (size < 1000)
{
    throw new EndOfStreamException();
}

However, even System.IO.MemoryStream doesn't guarantee to read requested bytes when the end of the stream has not been reached. See the specifications of MemoryStream.Read Method [1], which are quoted as follows:

The Read method will return zero only if the end of the stream is reached. In all other cases, Read always reads at least one byte from the stream before returning.

An implementation is free to return fewer bytes than requested even if the end of the stream has not been reached.

Code fix

The code fix provides an option replacing the for loop with a code fragment, declaring an Action delegate and invoking it. You should refactor the auto-generated code with renaming identifiers and replacing the delegate with the local function or extension method if possible.

Example

Diagnostic

public void Method(Stream inputStream)
{
    var reader = new BinaryReader(inputStream);
    var buffer = new byte[1000];

    for (var i = 0; i < 1000; ++i)
    {
        buffer[i] = reader.ReadByte();
    }
}

Code fix

public void Method(Stream inputStream)
{
    var reader = new BinaryReader(inputStream);
    var buffer = new byte[1000];

    {
        System.Action<byte[], int, int> _readFully = (_array, _offset, _length) =>
        {
            var _reader = reader;
            while (_length > 0)
            {
                var _size = _reader.Read(_array, _offset, _length);
                if (_size == 0)
                {
                    throw new System.IO.EndOfStreamException();
                }
                _offset += _size;
                _length -= _size;
            }
        };
        _readFully(buffer, 0, 1000);
    }
}

References

[1] Microsoft, .NET API Browser