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\$
2
  • \$\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, 2016 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\$
    – user109835
    Jun 29, 2016 at 17:30

1 Answer 1

1
\$\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\$
3
  • \$\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\$
    – user109835
    Jul 1, 2016 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\$
    – user109835
    Jul 1, 2016 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\$
    – user109835
    Jul 1, 2016 at 6:56

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.