Skip to content

Commit

Permalink
[struct_pack][fix] fix bug of struct_pack::write (#537)
Browse files Browse the repository at this point in the history
  • Loading branch information
poor-circle authored Dec 26, 2023
1 parent a21a606 commit d669b58
Show file tree
Hide file tree
Showing 5 changed files with 86 additions and 20 deletions.
2 changes: 1 addition & 1 deletion include/ylt/struct_pack/derived_marco.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -32,4 +32,4 @@
STRUCT_PACK_EXPAND_EACH(, GET_STRUCT_PACK_ID_IMPL_FOR_LOOP, base, \
__VA_ARGS__) \
STRUCT_PACK_DERIVED_DECL(base, __VA_ARGS__); \
static_assert(struct_pack::detail::check_ID_collision<base, __VA_ARGS__>());\
static_assert(struct_pack::detail::check_ID_collision<base, __VA_ARGS__>());
28 changes: 18 additions & 10 deletions include/ylt/struct_pack/endian_wrapper.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -287,9 +287,10 @@ STRUCT_PACK_INLINE void write(Writer& writer, const T& t) {
}
else if constexpr (detail::string<T> || detail::container<T>) {
std::uint64_t len = t.size();
detail::write_wrapper<sizeof(std::size_t)>(writer, (char*)&len);
detail::write_wrapper<8>(writer, (char*)&len);
if constexpr (detail::continuous_container<T> &&
detail::is_little_endian_copyable<sizeof(t[0])>) {
detail::is_little_endian_copyable<sizeof(t[0])> &&
std::is_fundamental_v<typename T::value_type>) {
write_bytes_array(writer, (const char*)t.data(), len * sizeof(t[0]));
}
else {
Expand Down Expand Up @@ -333,11 +334,12 @@ STRUCT_PACK_INLINE constexpr std::size_t get_write_size(const T& t) {
else if constexpr (detail::string<T> || detail::container<T>) {
std::size_t ret = 8;
if constexpr (detail::continuous_container<T> &&
detail::is_little_endian_copyable<sizeof(t[0])>) {
detail::is_little_endian_copyable<sizeof(t[0])> &&
std::is_fundamental_v<typename T::value_type>) {
ret += t.size() * sizeof(t[0]);
}
else {
for (auto& e : t) ret += write(e);
for (auto& e : t) ret += get_write_size(e);
}
return ret;
}
Expand All @@ -353,7 +355,7 @@ STRUCT_PACK_INLINE constexpr std::size_t get_write_size(const T* t,
template <typename Reader, typename T>
STRUCT_PACK_INLINE struct_pack::errc read(Reader& reader, T& t) {
if constexpr (std::is_fundamental_v<T>) {
if (!detail::read_wrapper<sizeof(T)>(reader, (char*)&t)) {
if SP_UNLIKELY (!detail::read_wrapper<sizeof(T)>(reader, (char*)&t)) {
return struct_pack::errc::no_buffer_space;
}
else {
Expand All @@ -364,7 +366,12 @@ STRUCT_PACK_INLINE struct_pack::errc read(Reader& reader, T& t) {
if constexpr (std::is_fundamental_v<
std::remove_reference_t<decltype(t[0])>> &&
detail::is_little_endian_copyable<sizeof(t[0])>) {
return read_bytes_array(reader, (char*)t.data(), sizeof(T));
if SP_UNLIKELY (!read_bytes_array(reader, (char*)t.data(), sizeof(T))) {
return struct_pack::errc::no_buffer_space;
}
else {
return {};
}
}
else {
struct_pack::errc ec;
Expand Down Expand Up @@ -395,15 +402,16 @@ STRUCT_PACK_INLINE struct_pack::errc read(Reader& reader, T& t) {
if SP_UNLIKELY (!reader.check(mem_size)) {
return struct_pack::errc::no_buffer_space;
}
detail::resize(t, mem_size);
if (!read_bytes_array(reader, (char*)t.data(), mem_size)) {
detail::resize(t, sz);
if SP_UNLIKELY (!read_bytes_array(reader, (char*)t.data(), mem_size)) {
return struct_pack::errc::no_buffer_space;
}
return struct_pack::errc{};
}
else {
t.clear();
for (std::size_t i = 0; i < sz; ++i) {
t.emplace_back();
t.push_back(typename T::value_type{});
ec = read(reader, t.back());
if SP_UNLIKELY (ec != struct_pack::errc{}) {
return ec;
Expand All @@ -420,7 +428,7 @@ template <typename Reader, typename T>
struct_pack::errc read(Reader& reader, T* t, std::size_t length) {
if constexpr (std::is_fundamental_v<T>) {
if constexpr (detail::is_little_endian_copyable<sizeof(T)>) {
if (!read_bytes_array(reader, (char*)t, sizeof(T) * length)) {
if SP_UNLIKELY (!read_bytes_array(reader, (char*)t, sizeof(T) * length)) {
return struct_pack::errc::no_buffer_space;
}
else {
Expand Down
4 changes: 2 additions & 2 deletions include/ylt/struct_pack/unpacker.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -780,10 +780,10 @@ class unpacker {
}
else if constexpr (id == type_id::user_defined_type) {
if constexpr (NotSkip) {
sp_deserialize_to(reader_, item);
code = sp_deserialize_to(reader_, item);
}
else {
sp_deserialize_to_with_skip(reader_, item);
code = sp_deserialize_to_with_skip(reader_, item);
}
}
else if constexpr (detail::varint_t<type, parent_tag>) {
Expand Down
3 changes: 2 additions & 1 deletion include/ylt/struct_pack/util.h
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,8 @@ void string_set_length_hacker(std::string &, std::size_t);
template <typename ch>
inline void resize(std::basic_string<ch> &raw_str, std::size_t sz) {
std::string &str = *reinterpret_cast<std::string *>(&raw_str);
#if defined(__SANITIZE_ADDRESS__) || __has_feature(address_sanitizer)
#if defined(__SANITIZE_ADDRESS__) || __has_feature(address_sanitizer) || \
(!defined(NDEBUG) && defined(_MSVC_STL_VERSION))
raw_str.resize(sz);
#elif defined(__GLIBCXX__) || defined(_LIBCPP_VERSION) || \
defined(_MSVC_STL_VERSION)
Expand Down
69 changes: 63 additions & 6 deletions src/struct_pack/tests/test_user_defined_type.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3,49 +3,81 @@

#include "doctest.h"
#include "ylt/struct_pack.hpp"
#include "ylt/struct_pack/endian_wrapper.hpp"
#include "ylt/struct_pack/error_code.hpp"
#include "ylt/struct_pack/type_id.hpp"
namespace my_name_space {

struct array2D {
std::string name = "Hello";
std::vector<std::vector<int>> values = {{-1, 2, 3}};
std::array<float, 3> values2 = {1.1, 2.3, 4.5};
unsigned int x;
unsigned int y;
float* p;
array2D(unsigned int x, unsigned int y) : x(x), y(y) {
p = (float*)calloc(1ull * x * y, sizeof(float));
}
array2D(const array2D&) = delete;
array2D(array2D&& o) : x(o.x), y(o.y), p(o.p) { o.p = nullptr; };
array2D(array2D&& o)
: x(o.x),
y(o.y),
p(o.p),
values(std::move(o.values)),
values2(o.values2) {
o.p = nullptr;
};
array2D& operator=(const array2D&) = delete;
array2D& operator=(array2D&& o) {
x = o.x;
y = o.y;
p = o.p;
values = o.values;
values2 = o.values2;
o.p = nullptr;
return *this;
}
float& operator()(std::size_t i, std::size_t j) { return p[i * y + j]; }
bool operator==(const array2D& o) const {
return x == o.x && y == o.y &&
memcmp(p, o.p, 1ull * x * y * sizeof(float)) == 0;
memcmp(p, o.p, 1ull * x * y * sizeof(float)) == 0 &&
values == o.values && values2 == o.values2;
}
array2D() : x(0), y(0), p(nullptr) {}
~array2D() { free(p); }
};

std::size_t sp_get_needed_size(const array2D& ar) {
return 2 * struct_pack::get_write_size(ar.x) +
struct_pack::get_write_size(ar.p, 1ull * ar.x * ar.y);
auto sz = struct_pack::get_write_size(ar.name) +
struct_pack::get_write_size(ar.values) +
struct_pack::get_write_size(ar.values2) +
2 * struct_pack::get_write_size(ar.x) +
struct_pack::get_write_size(ar.p, 1ull * ar.x * ar.y);
return sz;
}
template <typename Writer>
void sp_serialize_to(Writer& writer, const array2D& ar) {
struct_pack::write(writer, ar.name);
struct_pack::write(writer, ar.values);
struct_pack::write(writer, ar.values2);
struct_pack::write(writer, ar.x);
struct_pack::write(writer, ar.y);
struct_pack::write(writer, ar.p, 1ull * ar.x * ar.y);
}

template <typename Reader>
struct_pack::errc sp_deserialize_to(Reader& reader, array2D& ar) {
if (auto ec = struct_pack::read(reader, ar.name); ec != struct_pack::errc{}) {
return ec;
}
if (auto ec = struct_pack::read(reader, ar.values);
ec != struct_pack::errc{}) {
return ec;
}
if (auto ec = struct_pack::read(reader, ar.values2);
ec != struct_pack::errc{}) {
return ec;
}
if (auto ec = struct_pack::read(reader, ar.x); ec != struct_pack::errc{}) {
return ec;
}
Expand All @@ -69,6 +101,17 @@ struct_pack::errc sp_deserialize_to(Reader& reader, array2D& ar) {

template <typename Reader>
struct_pack::errc sp_deserialize_to_with_skip(Reader& reader, array2D& ar) {
if (auto ec = struct_pack::read(reader, ar.name); ec != struct_pack::errc{}) {
return ec;
}
if (auto ec = struct_pack::read(reader, ar.values);
ec != struct_pack::errc{}) {
return ec;
}
if (auto ec = struct_pack::read(reader, ar.values2);
ec != struct_pack::errc{}) {
return ec;
}
if (auto ec = struct_pack::read(reader, ar.x); ec != struct_pack::errc{}) {
return ec;
}
Expand All @@ -92,7 +135,7 @@ TEST_CASE("test user-defined_type") {
auto result = struct_pack::deserialize<my_name_space::array2D>(buffer);
CHECK(result.has_value());
auto& ar2 = result.value();
CHECK(ar == result.value());
CHECK(ar == ar2);
}

TEST_CASE("test user-defined_type nested") {
Expand All @@ -104,7 +147,21 @@ TEST_CASE("test user-defined_type nested") {
auto buffer = struct_pack::serialize(ar);
auto result = struct_pack::deserialize<decltype(ar)>(buffer);
CHECK(result.has_value());
CHECK(ar == result);
auto& ar2 = result.value();
CHECK(ar == ar2);
}

TEST_CASE("test user-defined_type nested ec") {
std::vector<my_name_space::array2D> ar;
ar.emplace_back(11, 22);
ar.emplace_back(114, 514);
ar[0](1, 6) = 3.14;
ar[1](87, 111) = 2.71;
auto buffer = struct_pack::serialize(ar);
buffer.pop_back();
auto result = struct_pack::deserialize<decltype(ar)>(buffer);
REQUIRE(!result.has_value());
CHECK(result.error() == struct_pack::errc::no_buffer_space);
}

TEST_CASE("test user-defined_type get_field") {
Expand Down

0 comments on commit d669b58

Please sign in to comment.