HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppModerate

Binary file validity in C++/C++11: checking a binary header

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
headerfilecheckingvaliditybinary

Problem

I have some code for checking a binary header to see if a binary file is meant to be read by a certain program, and I could use some extra eyes to review it.

Assume the following typedefs:

namespace Types
{
    using Byte = std::uint8_t;      // From 
    using UInt32 = std::uint32_t;
    using String = std::string;     // From 
}

// This is a vector of bytes loaded from a binary file.
using ByteBuffer = std::vector;    // From 


Also assume the following constants:

namespace Constants
{
    const Types::String     G_BINARY_HEADER_STRING  = "A2FORMAT";
    const Types::Byte       G_BINARY_MAJOR_VERSION  = 0x00;
    const Types::Byte       G_BINARY_MINOR_VERSION  = 0x01;
    const Types::UInt32     G_BINARY_HEADER_SIZE    = 14;
}


Here is the binary-header-validating code:

bool isBufferValid (ByteBuffer &a_buffer)
{
    // Get the size of the buffer. Make a cursor here, too.
    Types::UInt32 l_size = a_buffer.size();
    Types::UInt32 l_cursor = 0;

    // Check to see if the size exceeds 14 bytes.
    if (l_size  Constants::G_BINARY_MINOR_VERSION)
        return false;

    // Bytes #11 through #14 make up a 32-bit unsigned integer that indicates how many
    // bytes of data reside after the binary header.
    //
    // These bytes are arranged in Little Endian Notation.
    Types::UInt32 l_sizeOfData = 0;
    for (Types::UInt32 i = 0, j = 10; j < 14; i++, j++)
        l_sizeOfData |= a_buffer.at(j) << (i * 8);

    // If the amount of bytes in the buffer, after the header, do not match the number found
    // in the integer above, then the buffer is invalid.
    if (l_size - Constants::G_BINARY_HEADER_SIZE != l_sizeOfData)
        return false;

    // Remove the header from the buffer, because it has been confirmed valid, and is
    // therefore ready to use.
    a_buffer.erase(a_buffer.begin(), a_buffer.begin() + Constants::G_BINARY_HEADER_SIZE);

    return true;
}


Is there anything I can do to make this code more efficient? Are

Solution

The most unforgivable fault with this function is that calling isValidBuffer() also erases the buffer! That completely violates the Principle of Least Surprise. If anything, the parameter should be const.

Context

StackExchange Code Review Q#121371, answer score: 12

Revisions (0)

No revisions yet.