SeanOMik
/
libki
mirror of https://github.com/SeanOMik/libki.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

serialization: Fix inconsistencies in binary serialization 

- Fixed incorrect byte alignment following bit-based types.
- Boolean types are now serialized as 1 bit.
- write_to/read_from methods now take an is_file argument.
  Types such as enums, and strings change their serialization format depending on if they're in file mode or not.
- Enums now serialize the element name in file mode, and element value otherwise.
This commit is contained in:
pythonology 2019-06-02 22:38:29 -04:00
parent 8d1b9c21be
commit 40d088efe3
13 changed files with 279 additions and 102 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
include/ki/pclass/ClassType.h
View File

@ -26,8 +26,8 @@ namespace pclass
*/
bool inherits(const Type &type) const;
void write_to(BitStream &stream, Value &value) const override = 0;
Value read_from(BitStream &stream) const override = 0;
void write_to(BitStream &stream, const bool is_file, Value &value) const override = 0;
Value read_from(BitStream &stream, const bool is_file) const override = 0;
private:
const IClassType *m_base_class;
@ -64,20 +64,20 @@ namespace pclass
);
}
void write_to(BitStream &stream, Value &value) const override
void write_to(BitStream &stream, const bool is_file, Value &value) const override
{
const auto &object = value.get<ClassT>();
const auto &properties = object.get_properties();
for (auto it = properties.begin(); it != properties.end(); ++it)
it->write_value_to(stream);
it->write_value_to(stream, is_file);
}
Value read_from(BitStream &stream) const override
Value read_from(BitStream &stream, const bool is_file) const override
{
auto object = ClassT(*this, get_type_system());
auto &properties = object.get_properties();
for (auto it = properties.begin(); it != properties.end(); ++it)
it->read_value_from(stream);
it->read_value_from(stream, is_file);
return Value::make_value<ClassT>(object);
}
};

4
include/ki/pclass/Enum.h
View File

@ -22,8 +22,8 @@ namespace pclass
void set_value(enum_value_t value);
void set_value(const std::string &element_name);
void write_to(BitStream& stream) const;
void read_from(BitStream& stream);
void write_to(BitStream &stream, const bool is_file) const;
void read_from(BitStream &stream, const bool is_file);
operator enum_value_t() const;
void operator=(enum_value_t value);

12
include/ki/pclass/EnumType.h
View File

@ -49,8 +49,8 @@ namespace pclass
EnumType &add_element(const std::string &name, enum_value_t value);
void write_to(BitStream &stream, Value &value) const override;
Value read_from(BitStream &stream) const override;
void write_to(BitStream &stream, const bool is_file, Value &value) const override;
Value read_from(BitStream &stream, const bool is_file) const override;
private:
std::vector<Element *> m_elements;
@ -75,17 +75,17 @@ namespace pclass
m_kind = Kind::ENUM;
}
void write_to(BitStream &stream, Value &value) const override
void write_to(BitStream &stream, const bool is_file, Value &value) const override
{
auto &enum_reference = value.get<EnumT>();
auto &underlying_reference = reinterpret_cast<const underlying_type &>(enum_reference);
detail::primitive_type_helper<underlying_type>::write_to(stream, underlying_reference);
detail::primitive_type_helper<underlying_type>::write_to(stream, is_file, underlying_reference);
}
Value read_from(BitStream &stream) const override
Value read_from(BitStream &stream, const bool is_file) const override
{
Value read_result =
detail::primitive_type_helper<underlying_type>::read_from(stream);
detail::primitive_type_helper<underlying_type>::read_from(stream, is_file);
auto underlying_value = read_result.get<underlying_type>();
return Value::make_value<EnumT>(
static_cast<EnumT>(underlying_value)

172
include/ki/pclass/PrimitiveType.h
View File

@ -8,50 +8,60 @@ namespace pclass
namespace detail
{
/**
* Provides implementations to PrimitiveType<ValueT>::write_to,
* and PrimitiveType<ValueT>::read_from.
* Provides implementations to PrimitiveType<ValueT>::is_byte_based,
* PrimitiveType<ValueT>::write_to, and PrimitiveType<ValueT>::read_from.
*/
template <typename ValueT, typename Enable = void>
struct primitive_type_helper
{
static void write_to(BitStream &stream, const ValueT &value)
static bool is_byte_based()
{
// Provide a compiler error if this is not specialized
static_assert(
sizeof(ValueT) == 0,
"Missing specialization of primitive_type_writer::write_to"
"Missing specialization of primitive_type_helper::is_byte_based"
);
}
static Value read_from(BitStream &stream)
static void write_to(BitStream &stream, const bool is_file, const ValueT &value)
{
// Provide a compiler error if this is not specialized
static_assert(
sizeof(ValueT) == 0,
"Missing specialization of PrimitiveTypeReader::read_from"
"Missing specialization of primitive_type_helper::write_to"
);
}
// This should be impossible to reach.
throw runtime_error("Missing specialization of PrimitiveTypeReader::read_from");
static Value read_from(BitStream &stream, const bool is_file)
{
// Provide a compiler error if this is not specialized
static_assert(
sizeof(ValueT) == 0,
"Missing specialization of primitive_type_helper::read_from"
);
}
};
/**
* Specialization of primitive_type_helper for integer types.
* This includes instantiations of ki::BitInteger<>.
*/
template <typename ValueT>
struct primitive_type_helper<
ValueT,
typename std::enable_if<is_integral<ValueT>::value>::type
typename std::enable_if<std::is_integral<ValueT>::value>::type
>
{
static void write_to(BitStream &stream, const ValueT &value)
static bool is_byte_based()
{
return true;
}
static void write_to(BitStream &stream, const bool is_file, const ValueT &value)
{
stream.write<ValueT>(value);
}
static Value read_from(BitStream &stream)
static Value read_from(BitStream &stream, const bool is_file)
{
return Value::make_value<ValueT>(
stream.read<ValueT>()
@ -60,8 +70,63 @@ namespace pclass
};
/**
* Specialization of primitive_type_helper for floating point
* types.
* Specialization of primitive_type_helper for ki::BitInteger<> types.
*/
template <uint8_t N, bool Unsigned>
struct primitive_type_helper<ki::BitInteger<N, Unsigned>>
{
private:
using type = ki::BitInteger<N, Unsigned>;
public:
static bool is_byte_based()
{
return false;
}
static void write_to(BitStream &stream, const bool is_file, const type &value)
{
stream.write<type>(value);
}
static Value read_from(BitStream &stream, const bool is_file)
{
return Value::make_value<type>(
stream.read<type>()
);
}
};
/**
* Specialization of primitive_type_helper for boolean types.
*/
template <>
struct primitive_type_helper<bool>
{
private:
using underlying_type = ki::BitInteger<1, true>;
public:
static bool is_byte_based()
{
return false;
}
static void write_to(BitStream &stream, const bool is_file, const bool &value)
{
stream.write<underlying_type>(value);
}
static Value read_from(BitStream &stream, const bool is_file)
{
return Value::make_value<bool>(
stream.read<underlying_type>() != 0
);
}
};
/**
* Specialization of primitive_type_helper for floating point types.
*/
template <typename ValueT>
struct primitive_type_helper<
@ -69,27 +134,33 @@ namespace pclass
typename std::enable_if<std::is_floating_point<ValueT>::value>::type
>
{
static void write_to(BitStream &stream, const ValueT &value)
private:
/**
* An unsigned integer type with the same size as the floating point type
* ValueT.
*/
using uint_type = typename bits<bitsizeof<ValueT>::value>::uint_type;
public:
static bool is_byte_based()
{
return true;
}
static void write_to(BitStream &stream, const bool is_file, const ValueT &value)
{
// Reinterpret the reference as a reference to an integer
const uint_type &v = *(
reinterpret_cast<const uint_type *>(&value)
);
);
stream.write<uint_type>(v, bitsizeof<ValueT>::value);
}
static Value read_from(BitStream &stream)
static Value read_from(BitStream &stream, const bool is_file)
{
uint_type uint_value = stream.read<uint_type>(bitsizeof<ValueT>::value);
return Value::make_value<ValueT>(*reinterpret_cast<ValueT *>(&uint_value));
}
private:
/**
* An unsigned integer type with the same size as the floating point type
* ValueT.
*/
using uint_type = typename bits<bitsizeof<ValueT>::value>::uint_type;
};
/**
@ -106,21 +177,47 @@ namespace pclass
using type = std::basic_string<_Elem, _Traits, _Alloc>;
public:
static void write_to(BitStream &stream, const type &value)
static bool is_byte_based()
{
// Write the length as an unsigned short
stream.write<uint16_t>(value.length());
return true;
}
static void write_to(BitStream &stream, const bool is_file, const type &value)
{
if (is_file)
{
// TODO: Determine how the size of the length prefix is chosen
// while in file mode
stream.write<uint8_t>(value.length() * 2);
}
else
{
// Write the length as an unsigned short
stream.write<uint16_t>(value.length());
}
// Write each character as _Elem
for (auto it = value.begin(); it != value.end(); ++it)
stream.write<_Elem>(*it);
}
static Value read_from(BitStream &stream)
static Value read_from(BitStream &stream, const bool is_file)
{
// Read the length and create a new string with the correct capacity
auto length = stream.read<uint16_t>();
auto value = type(length, ' ');;
std::size_t length;
if (is_file)
{
// TODO: Determine how the size of the length prefix is chosen
// while in file mode
length = stream.read<uint8_t>() / 2;
}
else
{
// Read the length as an unsigned short
length = stream.read<uint16_t>();
}
// Create a new string with the correct capacity
auto value = type(length, ' ');
// Read each character into the string
for (auto it = value.begin(); it != value.end(); ++it)
@ -153,13 +250,18 @@ namespace pclass
}
~PrimitiveType() = default;
void write_to(BitStream &stream, Value &value) const override
bool is_byte_based() const override
{
return detail::primitive_type_helper<ValueT>::is_byte_based();
}
void write_to(BitStream &stream, const bool is_file, Value &value) const override
{
try
{
Value casted_value = value.as<ValueT>();
detail::primitive_type_helper<ValueT>::write_to(
stream,
stream, is_file,
casted_value.get<ValueT>()
);
}
@ -171,11 +273,11 @@ namespace pclass
}
}
Value read_from(BitStream &stream) const override
Value read_from(BitStream &stream, const bool is_file) const override
{
try
{
return detail::primitive_type_helper<ValueT>::read_from(stream);
return detail::primitive_type_helper<ValueT>::read_from(stream, is_file);
}
catch (runtime_error &e)
{

4
include/ki/pclass/Property.h
View File

@ -101,14 +101,14 @@ namespace pclass
* @param[in] stream The stream to write to.
* @param[in] index The index of the element to retrieve the value from
*/
virtual void write_value_to(BitStream &stream, std::size_t index = 0) const;
virtual void write_value_to(BitStream &stream, const bool is_file, std::size_t index = 0) const;
/**
* Read a value from a BitStream into the specified element of this property.
* @param[in] stream The stream to read from.
* @param[in] index The index of the element to read a value into.
*/
virtual void read_value_from(BitStream &stream, std::size_t index = 0);
virtual void read_value_from(BitStream &stream, const bool is_file, std::size_t index = 0);
private:
const PropertyClass *m_instance;

9
include/ki/pclass/Type.h
View File

@ -55,6 +55,11 @@ namespace pclass
hash_t get_hash() const;
Kind get_kind() const;
/**
* @returns Whether or not this type works in bytes, rather than bits.
*/
virtual bool is_byte_based() const;
/**
* The TypeSystem used to define this Type instance.
*/
@ -71,14 +76,14 @@ namespace pclass
* @param[in] stream The stream to write to.
* @param[in] value The value to write to the stream.
*/
virtual void write_to(BitStream &stream, Value &value) const;
virtual void write_to(BitStream &stream, const bool is_file, Value &value) const;
/**
* Read a value of this type from a BitStream.
* @param stream[in] The stream to read from.
* @returns The value read from the stream.
*/
virtual Value read_from(BitStream &stream) const;
virtual Value read_from(BitStream &stream, const bool is_file) const;
protected:
Kind m_kind;

35
src/pclass/Enum.cpp
View File

@ -56,21 +56,42 @@ namespace pclass
m_value = value;
}
void Enum::set_value(const std::string& element_name)
void Enum::set_value(const std::string &element_name)
{
m_value = get_type().get_element(element_name).get_value();
}
void Enum::write_to(BitStream& stream) const
void Enum::write_to(BitStream& stream, const bool is_file) const
{
detail::primitive_type_helper<enum_value_t>::write_to(stream, m_value);
if (is_file)
{
// Write the element name
const auto &name = get_type().get_element(m_value).get_name();
detail::primitive_type_helper<std::string>::write_to(stream, is_file, name);
}
else
{
// Write the element value
detail::primitive_type_helper<enum_value_t>::write_to(stream, is_file, m_value);
}
}
void Enum::read_from(BitStream& stream)
void Enum::read_from(BitStream& stream, const bool is_file)
{
const auto value = detail::primitive_type_helper<enum_value_t>
::read_from(stream).get<enum_value_t>();
set_value(value);
if (is_file)
{
// Set the value using the element name
const auto name = detail::primitive_type_helper<std::string>
::read_from(stream, is_file).get<std::string>();
set_value(name);
}
else
{
// Set the value using the element value
const auto value = detail::primitive_type_helper<enum_value_t>
::read_from(stream, is_file).get<enum_value_t>();
set_value(value);
}
}
Enum::operator enum_value_t() const

8
src/pclass/EnumType.cpp
View File

@ -109,16 +109,16 @@ namespace pclass
return *this;
}
void EnumType::write_to(BitStream &stream, Value &value) const
void EnumType::write_to(BitStream &stream, const bool is_file, Value &value) const
{
// Get an Enum reference and use it to write to the stream
value.as<Enum>().get<Enum>().write_to(stream);
value.as<Enum>().get<Enum>().write_to(stream, is_file);
}
Value EnumType::read_from(BitStream &stream) const
Value EnumType::read_from(BitStream &stream, const bool is_file) const
{
auto value = Enum(*this);
value.read_from(stream);
value.read_from(stream, is_file);
return Value::make_value<Enum>(value);
}
}

8
src/pclass/Property.cpp
View File

@ -81,19 +81,19 @@ namespace pclass
return 0;
}
void IProperty::write_value_to(BitStream& stream, const std::size_t index) const
void IProperty::write_value_to(BitStream &stream, const bool is_file, const std::size_t index) const
{
if (index < 0 || index >= get_element_count())
throw runtime_error("Index out of bounds.");
auto ref_value = get_value(index);
get_type().write_to(stream, ref_value);
get_type().write_to(stream, is_file, ref_value);
}
void IProperty::read_value_from(BitStream& stream, const std::size_t index)
void IProperty::read_value_from(BitStream &stream, const bool is_file, const std::size_t index)
{
if (index < 0 || index >= get_element_count())
throw runtime_error("Index out of bounds.");
set_value(get_type().read_from(stream), index);
set_value(get_type().read_from(stream, is_file), index);
}
}
}

9
src/pclass/Type.cpp
View File

@ -38,14 +38,19 @@ namespace pclass
return m_type_system;
}
void Type::write_to(BitStream &stream, Value &value) const
bool Type::is_byte_based() const
{
return true;
}
void Type::write_to(BitStream &stream, const bool is_file, Value &value) const
{
std::ostringstream oss;
oss << "Type '" << m_name << "' does not implement Type::write_to.";
throw runtime_error(oss.str());
}
Value Type::read_from(BitStream &stream) const
Value Type::read_from(BitStream &stream, const bool is_file) const
{
std::ostringstream oss;
oss << "Type '" << m_name << "' does not implement Type::read_from.";

5
src/protocol/net/DMLSession.cpp
View File

@ -70,7 +70,10 @@ namespace net
on_message(message);
else
on_invalid_message(InvalidDMLMessageErrorCode::INSUFFICIENT_ACCESS);
delete message;
// WARNING: Refactor, and utilize smart pointers to avoid memory leaks in DML messages.
// The following line breaks asynchronous code.
// delete message;
}
}
}

74
src/serialization/BinarySerializer.cpp
View File

@ -27,7 +27,8 @@ namespace serialization
const auto compression_header_pos = stream.tell();
if (FLAG_IS_SET(m_flags, flags::COMPRESSED))
{
stream.write<bool>(false);
if (m_is_file)
stream.write<bool>(false);
stream.write<uint32_t>(0);
}
@ -102,7 +103,8 @@ namespace serialization
// Write the compression header
const auto use_compression = compressed.size() < size_bytes;
stream.seek(compression_header_pos);
stream.write<bool>(use_compression);
if (m_is_file)
stream.write<bool>(use_compression);
stream.write<uint32_t>(size_bytes);
// Write the compressed data
@ -157,14 +159,14 @@ namespace serialization
stream.write(size_bits);
stream.seek(end_pos);
}
// Re-align the stream so that our position lies on a byte
// TODO: Look into how/when the serialization re-aligns as this may not be the right place
stream.seek(BitStream::stream_pos(stream.tell().as_bytes(), 0));
}
void BinarySerializer::save_property(const pclass::IProperty &prop, BitStream &stream) const
{
// Determine if we need to re-align the stream so that our position lies on a byte
if (prop.get_type().is_byte_based() || prop.is_dynamic() || m_is_file)
stream.seek(BitStream::stream_pos(stream.tell().as_bytes(), 0));
// Remember where we started writing the property data
const auto start_pos = stream.tell();
@ -178,7 +180,19 @@ namespace serialization
// If the property is dynamic, write the element count
if (prop.is_dynamic())
stream.write<uint32_t>(prop.get_element_count());
{
if (m_is_file)
{
// TODO: Determine how the size of the element count is chosen
// while in file mode
stream.write<uint8_t>(prop.get_element_count() * 2);
}
else
{
// Write the element count as an unsigned int
stream.write<uint32_t>(prop.get_element_count());
}
}
for (auto i = 0; i < prop.get_element_count(); ++i)
{
@ -189,7 +203,7 @@ namespace serialization
save_object(prop.get_object(i), stream);
}
else
prop.write_value_to(stream, i);
prop.write_value_to(stream, m_is_file, i);
}
// Finish writing the property header by writing the length
@ -226,7 +240,9 @@ namespace serialization
if (FLAG_IS_SET(m_flags, flags::COMPRESSED))
{
// Read the compression header
const auto use_compression = segment_stream.read<bool>();
bool use_compression = true;
if (m_is_file)
use_compression = segment_stream.read<bool>();
const auto uncompressed_size = segment_stream.read<uint32_t>();
// Work out how much data is available after the compression header
@ -305,14 +321,15 @@ namespace serialization
stream.tell(), object_size
);
auto object_stream = BitStream(*object_buffer);
stream.seek(stream.tell() + object_size, false);
// Instead of loading properties sequentially, the file format specifies
// the hash of a property before writing its value, so we just need to
// iterate for how ever many properties the object has declared.
for (std::size_t i = 0;
i < properties.get_property_count(); i++)
// iterate over how ever many bits were specified as the object size.
while (object_stream.tell().as_bits() < object_size)
{
// Re-align the object stream so that our position lies on a byte
object_stream.seek(BitStream::stream_pos(object_stream.tell().as_bytes(), 0));
// Read the property's size, and create a new BitBufferSegment to
// ensure that data is only read from inside this region.
const auto property_size =
@ -321,14 +338,21 @@ namespace serialization
object_stream.tell(), property_size
);
auto property_stream = BitStream(*property_buffer);
object_stream.seek(object_stream.tell() + property_size, false);
// Get the property to load based on it's hash, and then load
// it's value.
const auto property_hash = property_stream.read<uint32_t>();
auto &prop = properties.get_property(property_hash);
load_property(prop, property_stream);
// Seek out the end of the property in the object stream
auto byte_pos = object_stream.tell().as_bytes() + property_stream.tell().as_bytes();
object_stream.seek(BitStream::stream_pos(byte_pos, 0), false);
}
// Seek out the end of the object in the stream
auto byte_pos = stream.tell().as_bytes() + object_stream.tell().as_bytes();
stream.seek(BitStream::stream_pos(byte_pos, 0), false);
}
else
{
@ -341,17 +365,29 @@ namespace serialization
// All properties on this object have been set now, so call the
// created handler on the object
dest->on_created();
// Re-align the stream so that our position lies on a byte
stream.seek(BitStream::stream_pos(stream.tell().as_bytes(), 0), false);
}
void BinarySerializer::load_property(pclass::IProperty &prop, BitStream &stream) const
{
// Determine if we need to re-align the stream so that our position lies on a byte
if (prop.get_type().is_byte_based() || prop.is_dynamic())
stream.seek(BitStream::stream_pos(stream.tell().as_bytes(), 0));
// If the property is dynamic, we need to load the element count
if (prop.is_dynamic())
{
const auto element_count = stream.read<uint32_t>();
std::size_t element_count;
if (m_is_file)
{
// TODO: Determine how the size of the element count is chosen
// while in file mode
element_count = stream.read<uint8_t>() / 2;
}
else
{
// Load the element count as an unsigned int
element_count = stream.read<uint32_t>();
}
prop.set_element_count(element_count);
}
@ -366,7 +402,7 @@ namespace serialization
prop.set_object(object, i);
}
else
prop.read_value_from(stream, i);
prop.read_value_from(stream, m_is_file, i);
}
}
}

29
test/src/unit-serialization.cpp
View File

@ -54,24 +54,24 @@ struct Vector3D
m_z == that.m_z;
}
void write_to(BitStream &stream) const
void write_to(BitStream &stream, const bool is_file) const
{
pclass::detail::primitive_type_helper<float>
::write_to(stream, m_x);
::write_to(stream, is_file, m_x);
pclass::detail::primitive_type_helper<float>
::write_to(stream, m_y);
::write_to(stream, is_file, m_y);
pclass::detail::primitive_type_helper<float>
::write_to(stream, m_z);
::write_to(stream, is_file, m_z);
}
void read_from(BitStream &stream)
void read_from(BitStream &stream, const bool is_file)
{
m_x = pclass::detail::primitive_type_helper<float>
::read_from(stream).get<float>();
::read_from(stream, is_file).get<float>();
m_y = pclass::detail::primitive_type_helper<float>
::read_from(stream).get<float>();
::read_from(stream, is_file).get<float>();
m_z = pclass::detail::primitive_type_helper<float>
::read_from(stream).get<float>();
::read_from(stream, is_file).get<float>();
}
private:
@ -93,15 +93,20 @@ namespace detail
template <>
struct primitive_type_helper<Vector3D>
{
static void write_to(BitStream &stream, const Vector3D &value)
static bool is_byte_based()
{
value.write_to(stream);
return true;
}
static Value read_from(BitStream &stream)
static void write_to(BitStream &stream, const bool is_file, const Vector3D &value)
{
value.write_to(stream, is_file);
}
static Value read_from(BitStream &stream, const bool is_file)
{
Vector3D value;
value.read_from(stream);
value.read_from(stream, is_file);
return Value::make_value<Vector3D>(value);
}
};