6
\$\begingroup\$

So I've made custom binary streams which work like this:

/// \brief Reads a variable from the stream.
/// \tparam T Type of the variable.
/// \param[out] var Variable to read.
/// \return Reference to this stream.
/// \throw TODO
/// \note No endianness conversion will be performed if a byte is not 8 bit
/// wide.
template <typename T>
BinaryInputStream& operator>>(T& var)
{
    static_assert(std::is_pod<T>::value, "BinaryInputStream::operator>>: "
        "Binary stream can only read POD types.");
    static_assert(!std::is_enum<T>::value, "BinaryInputStream::operator>>: "
        "Directly reading enums is unsafe. Implement a custom overload and "
        "check the value inside it.");
    char* buffer = reinterpret_cast<char*>(&var);
    std::size_t buffersize = sizeof(var);
    this->Read(buffer, buffersize);
#if CHAR_BIT == 8
    Endianness systemendianness = General::GetEndianness();
    if (systemendianness == Endianness::Unknown)
    {
        return *this;
    }
    if (systemendianness != this->GetEndianness())
    {
        SwapBytes(var);
    }
#endif
    return *this;
}

/// \brief Writes a variable to the stream.
/// \tparam T Type of the variable.
/// \param[in] var Variable to write.
/// \return Reference to this stream.
/// \throw TODO
/// \note No endianness conversion will be performed if a byte is not 8 bit
/// wide.
template <typename T>
BinaryOutputStream& operator<<(T var)
{
    static_assert(std::is_pod<T>::value,
        "Binary stream can only write POD types.");
#if CHAR_BIT == 8
    Endianness systemendianness = General::GetEndianness();
    if ((systemendianness != Endianness::Unknown) &&
        (systemendianness != this->GetEndianness()))
    {
        SwapBytes(var);
    }
#endif
    char* buffer = reinterpret_cast<char*>(&var);
    std::size_t buffersize = sizeof(var);
    this->Write(buffer, buffersize);
    return *this;
}

/// \brief Swaps bytes of a given variable.
/// \tparam T type of the variable.
/// \param[in,out] var Input variable.
template <typename T>
void SwapBytes(T &var)
{
    char* buffer = reinterpret_cast<char*>(&var);
    for (std::size_t i = 0, j = sizeof(var) - 1; i < j; ++i, --j)
    {
        std::swap(buffer[i], buffer[j]);
    }
}

Is this correct? Have I triggered undefined behavior? Are static_asserts correct? Maybe they are overly restrictive or not strict enough? Please quote the standard if possible.

\$\endgroup\$
  • \$\begingroup\$ @FaTone If I understand your usage of this pointer correctly these are class method templates. Wouldn't it be better to provide rest of the class or at least relevant methods? \$\endgroup\$ – Jan Korous Jun 28 '16 at 22:14
  • \$\begingroup\$ Well the rest is just lots of wrappers around these and std::istream and std::ostream functions. You can view the full source code here: gitlab.com/ftz/general \$\endgroup\$ – Lyberta Jun 29 '16 at 17:30
2
\$\begingroup\$

TODO throw documentation

Throw documentation TODO? E. g. here:

/// \throw TODO

this pointer

Usage of this as in here is unnecessary.

this->Read(buffer, buffersize);

Calling the method without it is more common.

Read(buffer, buffersize);

endiannes

Endianness systemendianness = General::GetEndianness();

I would speculate that system endianness is not going to change during runtime.

Therefore I would probably use "more constant" representation something like

  • static const variable
  • template parameter
  • maybe even MACRO

read & swap

It doesn't look quite efficient to always read memory and swap if endiannesses don't match. What about having alternative version of Read() method already reversing the order during read?

\$\endgroup\$
  • \$\begingroup\$ throw docs: I use std::istream and std::ostream which have exceptions flags set for eofbit, failbit and badbit. If you know what they throw and when, I'll be glad to hear it. this pointer: I always use this-> for member function calls for clarity. Endianness, good observation, I guess I will put it in a Meyers singleton, I can't determine it and compile time since compiler and host endianness may be different. read & swap: I'm doing it this way so 1 part of the file can be big endian and another is little. You never know. \$\endgroup\$ – Lyberta Jul 1 '16 at 5:17
  • \$\begingroup\$ Well I remembered that some CPUs can change endianness: en.wikipedia.org/wiki/Endianness#Bi-endian_hardware So I'm gonna keep the runtime check. \$\endgroup\$ – Lyberta Jul 1 '16 at 6:12
  • \$\begingroup\$ After thinking about it more. If CPU will change endianness during the execution of a program, this means swapping the whole RAM, I guess I will check the endianness once and store it in a singleton, but that wouldn't change the interface here. \$\endgroup\$ – Lyberta Jul 1 '16 at 6:56

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.