3
\$\begingroup\$

I have written this code for serializing and deserializing integer and floating point numbers to/from vector/array of bytes. The aim of the code is to provide a simple interface to use, but make it as readable on the caller side as possible (e.g. not allow hiding type conversions).

This code doesn't cover runtime validation on purpose. It is expected that the user will check that the stream has enough bytes to read and that the resulting values can be used safely (signaling NaN etc.), it can be done by wrapping calls to these functions into "safe" versions that do the runtime checks.

I've included a short example at the end of how it can be used (wandbox link with this code).

I'm interested in any suggestions or any kind of input (goals, readability, safety, performance, code style, etc.).

// Serialization.h

//#pragma once

#include <array>
#include <bit>
#include <type_traits>
#include <vector>
#include <stdexcept>

namespace Serialization
{
    template<typename Num, typename NumArg>
    void AppendNumber(std::vector<std::byte>& inOutByteStream, NumArg number)
    {
        static_assert(std::is_same_v<typename std::decay<NumArg>::type, Num>, "We should provide argument of the same type that we want to write. If you want to make conversion, you can use WriteNumberNarrowCast or WriteNumberWideCast");
        static_assert(std::is_arithmetic_v<Num>, "Type should be ariphmetic to be serialized with WriteNumber");
        static_assert(sizeof(std::array<std::byte, sizeof(Num)>) == sizeof(Num), "Unexpected std::array layout");
        static_assert(std::is_standard_layout_v<std::array<std::byte, sizeof(Num)>>, "Unexpected std::array layout");
        static_assert(std::is_trivially_constructible_v<std::array<std::byte, sizeof(Num)>>, "Unexpected std::array implementation");
        static_assert(std::is_trivially_copyable_v<std::array<std::byte, sizeof(Num)>>, "Unexpected std::array implementation");

        const auto* byteRepresentation = std::bit_cast<std::array<std::byte, sizeof(Num)>*>(&number);

        if constexpr (std::endian::native == std::endian::little)
        {
            inOutByteStream.insert(
                inOutByteStream.end(),
                std::begin(*byteRepresentation),
                std::end(*byteRepresentation)
            );
        }
        else if constexpr (std::endian::native == std::endian::big)
        {
            inOutByteStream.insert(
                inOutByteStream.end(),
                std::rbegin(*byteRepresentation),
                std::rend(*byteRepresentation)
            );
        }
        else
        {
            throw std::logic_error("Mixed entian is not supported");
        }
    }

    template<typename Num, typename NumArg, typename ByteStream>
    void WriteNumber(ByteStream& inOutByteStream, NumArg number, size_t& cursorPos)
    {
        static_assert(std::is_same_v<typename std::decay<NumArg>::type, Num>, "We should provide argument of the same type that we want to write. If you want to make conversion, you can use WriteNumberNarrowCast or WriteNumberWideCast");
        static_assert(std::is_arithmetic_v<Num>, "Type should be ariphmetic to be serialized with WriteNumber");
        static_assert(sizeof(std::array<std::byte, sizeof(Num)>) == sizeof(Num), "Unexpected std::array layout");
        static_assert(std::is_standard_layout_v<std::array<std::byte, sizeof(Num)>>, "Unexpected std::array layout");
        static_assert(std::is_trivially_constructible_v<std::array<std::byte, sizeof(Num)>>, "Unexpected std::array implementation");
        static_assert(std::is_trivially_copyable_v<std::array<std::byte, sizeof(Num)>>, "Unexpected std::array implementation");

        const auto* byteRepresentation = std::bit_cast<std::array<std::byte, sizeof(Num)>*>(&number);

        if constexpr (std::endian::native == std::endian::little)
        {
            std::copy(
                std::begin(*byteRepresentation),
                std::end(*byteRepresentation),
                std::begin(inOutByteStream) + cursorPos
            );
        }
        else if constexpr (std::endian::native == std::endian::big)
        {
            std::copy(
                std::rbegin(*byteRepresentation),
                std::rend(*byteRepresentation),
                std::begin(inOutByteStream) + cursorPos
            );
        }
        else
        {
            throw std::logic_error("Mixed entian is not supported");
        }

        cursorPos += sizeof(Num);
    }

    template<typename Num, typename NumArg>
    void AppendNumberNarrowCast(std::vector<std::byte>& inOutByteStream, NumArg number)
    {
        static_assert(std::is_convertible_v<NumArg, Num>, "Argument type should be convertible to the data type");
        static_assert(sizeof(NumArg) >= sizeof(Num), "WriteNumberNarrowCast called with a value of smaller type, that may be a sign of a logical error or inefficient use of the stream space");
        static_assert(std::is_signed_v<NumArg> == std::is_signed_v<Num>, "The provided type has different signess");

        AppendNumber<Num>(inOutByteStream, static_cast<Num>(number));
    }

    template<typename Num, typename NumArg>
    void AppendNumberWideCast(std::vector<std::byte>& inOutByteStream, NumArg number)
    {
        static_assert(std::is_convertible_v<NumArg, Num>, "Argument type should be convertible to the data type");
        static_assert(sizeof(NumArg) <= sizeof(Num), "WriteNumberWideCast called with a value of bigger type, that may be a sign of a logical error or potential data loss");
        static_assert(std::is_signed_v<NumArg> == std::is_signed_v<Num>, "The provided type has different signess");

        AppendNumber<Num>(inOutByteStream, static_cast<Num>(number));
    }

    template<typename Num, typename NumArg, typename ByteStream>
    void WriteNumberNarrowCast(std::vector<std::byte>& inOutByteStream, NumArg number, size_t& cursorPos)
    {
        static_assert(std::is_convertible_v<NumArg, Num>, "Argument type should be convertible to the data type");
        static_assert(sizeof(NumArg) >= sizeof(Num), "WriteNumberNarrowCast called with a value of smaller type, that may be a sign of a logical error or inefficient use of the stream space");
        static_assert(std::is_signed_v<NumArg> == std::is_signed_v<Num>, "The provided type has different signess");

        WriteNumber<Num>(inOutByteStream, static_cast<Num>(number), cursorPos);
    }

    template<typename Num, typename NumArg, typename ByteStream>
    void WriteNumberWideCast(std::vector<std::byte>& inOutByteStream, NumArg number, size_t& cursorPos)
    {
        static_assert(std::is_convertible_v<NumArg, Num>, "Argument type should be convertible to the data type");
        static_assert(sizeof(NumArg) <= sizeof(Num), "WriteNumberWideCast called with a value of bigger type, that may be a sign of a logical error or potential data loss");
        static_assert(std::is_signed_v<NumArg> == std::is_signed_v<Num>, "The provided type has different signess");

        WriteNumber<Num>(inOutByteStream, static_cast<Num>(number), cursorPos);
    }

    template<typename Num, typename ByteStream>
    Num ReadNumber(const ByteStream& inOutByteStream, size_t& cursorPos)
    {
        static_assert(std::is_arithmetic_v<Num>, "Type should be ariphmetic to be deserialized with ReadNumber");
        static_assert(sizeof(std::array<std::byte, sizeof(Num)>) == sizeof(Num), "Unexpected std::array layout");
        static_assert(std::is_standard_layout_v<std::array<std::byte, sizeof(Num)>>, "Unexpected std::array layout");
        static_assert(std::is_trivially_constructible_v<std::array<std::byte, sizeof(Num)>>, "Unexpected std::array implementation");
        static_assert(std::is_trivially_copyable_v<std::array<std::byte, sizeof(Num)>>, "Unexpected std::array implementation");

        Num number;
        auto* byteRepresentation = std::bit_cast<std::array<std::byte, sizeof(Num)>*>(&number);

        const size_t lastCursorPos = cursorPos;

        if constexpr (std::endian::native == std::endian::little)
        {
            std::copy(
                inOutByteStream.begin() + lastCursorPos,
                inOutByteStream.begin() + (lastCursorPos + sizeof(Num)),
                byteRepresentation->begin()
            );
        }
        else if constexpr (std::endian::native == std::endian::big)
        {
            std::copy(
                inOutByteStream.rbegin() + (inOutByteStream.size() - lastCursorPos - sizeof(Num)),
                inOutByteStream.rbegin() + (inOutByteStream.size() - lastCursorPos),
                byteRepresentation->begin()
            );
        }
        else
        {
            throw std::logic_error("Mixed endian is not supported");
        }

        cursorPos += sizeof(Num);
        return number;
    }
}

// example_main.cpp

#include <cstdint>
#include <vector>
#include <iostream>

//#include "Serialization.h"

using u8 = uint8_t;
using f32 = float;
using f64 = double;

int main()
{
    std::vector<std::byte> byteStream;
    const size_t expectedBytesToRead = 5;
    byteStream.reserve(expectedBytesToRead);

    // serialization side
    const int someValue1 = 42;
    //Serialization::AppendNumber<u8>(byteStream, someValue1); // fails to compile, implicit narrow cast
    //Serialization::AppendNumberNarrowCast<u8>(byteStream, someValue1); // fails to compile, different signess
    Serialization::AppendNumber<u8>(byteStream, static_cast<u8>(someValue1)); // good

    const f64 someValue2 = 0.25; // some test value that can be exactly represented as f32
    //Serialization::AppendNumber<f32>(byteStream, someValue2); // fails to compile, implicit narrow cast
    Serialization::AppendNumberNarrowCast<f32>(byteStream, someValue2); // good

    // deserialization side
    size_t cursor = 0;
    const int someResultValue1 = Serialization::ReadNumber<u8>(byteStream, cursor);
    const f32 someResultValue2 = Serialization::ReadNumber<f32>(byteStream, cursor);

    // validate the result
    std::cout << ((someResultValue1 == someValue1 && someResultValue2 == someValue2 && cursor == expectedBytesToRead) ? "passed" : "failed");
}
\$\endgroup\$
1
  • \$\begingroup\$ After writing this message I decided to write a safe wrapper around these functions, and it turned out I had a few reads out of bounds in my app that I didn't know about. So it feels like putting runtime checks on the user may have been a bad idea. \$\endgroup\$ Sep 17, 2023 at 17:39

1 Answer 1

4
\$\begingroup\$

Reduce responsibilities

Your AppendNumber() family of functions do too much; they not only take care of serialization, but also deal with conversions and offsets into the output container. This complicates these functions, and at the same time makes them less general.

Consider having only one AppendNumber() function that converts given value to binary form, and then passes it to an output iterator that will take care of storing the binary data. The caller can do the conversion and pass whatever iterator is necessary so the data ends up in the right place. You can still make helper functions to do these things if it is not already trivial to do.

For example:

template<typename T, typename OutputIt>
OutputIt AppendNumber(OutputIt output, T value) {
    …
    const auto* bytes = std::bit_cast<…>(…);

    if constexpr (std::endian::native == std::endian::little)
    {
        std::copy(std::begin(bytes), std::end(bytes), output);
    }
    …
    return output;
}

Now you can write:

std::vector<std::byte> bytes(5);
auto cursor = std::begin(bytes); // you can just add an offset here
cursor = AppendNumber<std::uint8_t>(cursor, 42);
cursor = AppendNumber<float>(cursor, 0.25);

I've used the same interface as std::copy() and many other STL algorithms use: they take iterators by value, and possibly return the value of the iterator after they are done.

Note that now you are also no longer required to serialize to a std::vector<byte>, you can serialize to anything that supports iterators. You can even serialize directly to a file:

std::ofstream file("serialized.bin");
auto cursor = std::ostreambuf_iterator(file);
cursor = AppendNumber<std::uint8_t>(cursor, 42);
…

If it is more suitable for your application, you could of course decide to pass the iterator by reference, so you don't need to look at the return value.

As for conversions: the above will use implicit casting. When combined with compiler warnings about unsafe casts (using -Wconversion for example), I would consider this fine. It will deduce T here, so you could also use the convention in your code that you always have to write something like:

double someValue2 = 0.25;
cursor = AppendNumber(cursor, NarrowCast<float>(someValue2));

Where NarrowCast() is now a utility function whose sole responsibility is narrow casting of values. Note how you can now add different ways to cast, without having to add new AppendNumber*() functions.

You can also consider enforcing that an explicit cast operation has to be used, by making the cast functions actually be classes that hold their cast value:

class ExplicitCast {};

template<typename T>
class NarrowCast: ExplicitCast
{
    T value;
public:
    template<typename Arg>
    NarrowCast(Arg number): value(number)
    {
        static_assert(…);
    }

    T get() const {
        return value;
    }
}

template<typename T, typename OutputIt>
OutputIt AppendNumber(OutputIt output, T value) {
    static_assert(std::is_base_of_v<ExplicitCast, T>,
        "value must be explicitly cast with a safe cast operation");
    …
    const auto* bytes = std::bit_cast<…>(&value.get());
    …
}

reserve() does not do what you want

I see in your example that you use reserve() to add space to a vector. However, this is just changes the capacity, not the size. It will only ensure that calling push_back() will not require reallocations until the capacity is reached, but after calling AppendNumber() as in your example, byteStream.size() will still be zero. The correct way is to use resize(), or use an iterator that will call push_back() for you, for example by using std::back_inserter():

std::vector<std::byte> bytes;
auto cursor = std::back_inserter(bytes);
AppendNumber<std::uint8_t>(cursor, 42);
AppendNumber<float>(cursor, 0.25);

Alternatives

Instead of using output iterators, you could pass in an output stream. This makes it easier to serialize directly to a stream object like a file. And if you want to serialize to a std::vector, then since C++23 you could use std::spanstream as an adapter, although typically you would then use a std::basic_stringstream<std::byte> as a container instead. You could then also make SerializeNumber() a class instead of a function, and add overloads for std::operator<<(std::ostream&), so you can write something like:

std::ofstream file("serialized.bin");
file << SerializeNumber(NarrowCast<float>(0.25));
\$\endgroup\$
2
  • \$\begingroup\$ Thank you for such detailed feedback, this is really helpful to me. I haven't thought about iterators, I think this is a great improvement, will try it About type conversions, I wanted the user to always explicitly specify the type they use, so it is easier to see if there are less/more bytes have been written than read or if a wrong type is used. Giving the ability to deduce the type would make it more bug-prone (if someone refactors something and changes the type by accident). And if the type is always specified, then it feels a bit verbose to write type twice for narrow/wide cast \$\endgroup\$ Sep 17, 2023 at 17:35
  • \$\begingroup\$ You could make classes that hold converted values, have the cast functions return those, and only allow those to be passed to AppendNumber(). Then you can enforce that AppendNumber(cursor, SomeCast(42)) is called, and AppendNumber(cursor, 42) will fail. That still keeps the separation of concerns, and avoids writing the type twice. \$\endgroup\$
    – G. Sliepen
    Sep 17, 2023 at 18:27

Your Answer

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

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