Correct and portable serialization

Question

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.

• Jan Korous: @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?
• user109835: 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

Answer 1

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?

• user109835: 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.
• user109835: 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.
• user109835: 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.