From 309978f48d00c71366cf529c5459b739f9e0673e Mon Sep 17 00:00:00 2001 From: Evgenii Zhemchugov Date: Mon, 20 Feb 2023 18:02:04 +0600 Subject: [PATCH 1/7] BinaryStream: fix segmentation fault when working with std::deque Since std::deque elements are not stored contiguously but rather as a sequence of individually allocated fixed-size arrays, `Bwrite(&rhs[0], rhs.size() * sizeof(T))` for `std::deque rhs` will read garbage for sufficiently large `rhs.size()`. --- src/Store/BinaryStream.h | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/src/Store/BinaryStream.h b/src/Store/BinaryStream.h index 17cfcc4..33375f6 100644 --- a/src/Store/BinaryStream.h +++ b/src/Store/BinaryStream.h @@ -267,10 +267,11 @@ class BinaryStream : public SerialisableObject{ unsigned int tmp=rhs.size(); ret*=(*this) << tmp; if(tmp){ - if(check_base::value){ - for(typename std::deque::iterator it=rhs.begin(); it!=rhs.end(); it++) ret*=(*this) << (*it); - } - else ret*=Bwrite(&(rhs[0]), tmp*sizeof(T)); + for(typename std::deque::iterator it=rhs.begin(); it!=rhs.end(); it++) + if(check_base::value) + ret*=(*this) << (*it); + else + ret*=Bwrite(&*it, sizeof(T)); } return ret; } @@ -284,10 +285,11 @@ class BinaryStream : public SerialisableObject{ ret*=(*this) >> tmp; rhs.resize(tmp); if(tmp){ - if(check_base::value){ - for(typename std::deque::iterator it=rhs.begin(); it!=rhs.end(); it++) ret*=(*this) >> (*it); - } - else ret*=Bread(&(rhs[0]), tmp*sizeof(T)); + for(typename std::deque::iterator it=rhs.begin(); it!=rhs.end(); it++) + if(check_base::value) + ret*=(*this) >> (*it); + else + ret*=Bread(&*it, sizeof(T)); } return ret; } From 4733154ca3e057731d06811f179cf48d3d1787b8 Mon Sep 17 00:00:00 2001 From: Evgenii Zhemchugov Date: Mon, 20 Feb 2023 19:27:10 +0600 Subject: [PATCH 2/7] BinaryStream: fix compiler warnings on using '*' in boolean context Note that the code behaviour after this patch is different. Current implementation stops serialization as soon as an error is detected while previous code would would attempt to serialize every element of a container before reporting failure. --- src/Store/BinaryStream.h | 360 +++++++++++++++++++-------------------- 1 file changed, 172 insertions(+), 188 deletions(-) diff --git a/src/Store/BinaryStream.h b/src/Store/BinaryStream.h index 33375f6..d2ec9e8 100644 --- a/src/Store/BinaryStream.h +++ b/src/Store/BinaryStream.h @@ -56,45 +56,42 @@ class BinaryStream : public SerialisableObject{ //operator overloads - bool operator<<(std::string& rhs){ - if(m_mode!=READ){ - bool ret=true; - unsigned int tmp=rhs.length(); - ret*=(*this) << tmp; - if(tmp) ret*=Bwrite(&(rhs[0]), tmp); - return ret; - } - else return false; - } - - bool operator>>(std::string& rhs){ - if(m_mode!=NEW && m_mode!=APPEND){ - bool ret=true; - unsigned int tmp=0; - ret*=(*this) >> tmp; - rhs.resize(tmp); - if(tmp) ret*=Bread(&(rhs[0]), tmp); - return ret; - } - else return false; - } - + bool operator<<(std::string& rhs) { + if (m_mode == READ) return false; + + unsigned int len = rhs.length(); + if (!(*this << len)) return false; + + if (len == 0) return true; + return Bwrite(rhs.data(), len); + }; + + bool operator<<(const std::string& rhs) { + if (m_mode == READ) return false; + + unsigned int len = rhs.length(); + if (!(*this << len)) return false; + + if (len == 0) return true; + return Bwrite(rhs.data(), len); + }; + + bool operator>>(std::string& rhs) { + if (m_mode == NEW || m_mode == APPEND) return false; + + unsigned int len = 0; + if (!(*this >> len)) return false; + + rhs.resize(len); + if (len == 0) return true; + return Bread(rhs.data(), len); + }; + bool operator&(std::string& rhs){ if(m_write) return (*this) << rhs; else return (*this) >> rhs; } - bool operator<<(const std::string& rhs){ - if(m_mode!=READ){ - bool ret=true; - unsigned int tmp=rhs.length(); - ret*=(*this) << tmp; - if(tmp) ret*=Bwrite(&(rhs[0]), tmp); - return ret; - } - else return false; - } - bool operator&(const std::string& rhs){ if(m_write) return (*this) << rhs; return false; @@ -110,9 +107,9 @@ class BinaryStream : public SerialisableObject{ } else return Bwrite(&rhs, sizeof(T)); } - else return false; + else return false; } - + template bool operator>>(T& rhs){ if(m_mode!=NEW && m_mode!=APPEND){ @@ -149,40 +146,37 @@ class BinaryStream : public SerialisableObject{ return false; } - - template bool operator<<(std::vector& rhs){ - - if(m_mode!=READ){ - bool ret=true; - unsigned int tmp=rhs.size(); - ret*=(*this) << tmp; - if(tmp){ - if(check_base::value){ - for(typename std::vector::iterator it=rhs.begin(); it!=rhs.end(); it++) ret*=(*this) << (*it); - } - else ret*=Bwrite(&(rhs[0]), tmp*sizeof(T)); - } - return ret; - } - else return false; + template bool operator<<(std::vector& rhs) { + if (m_mode == READ) return false; + + unsigned int size = rhs.size(); + if (!(*this << size)) return false; + + if (size == 0) return true; + + if (!check_base::value) + return Bwrite(rhs.data(), size * sizeof(T)); + + for (typename std::vector::iterator it = rhs.begin(); it != rhs.end(); ++it) + if (!(*this << *it)) return false; + return true; } - - template bool operator>>(std::vector& rhs){ - - if(m_mode!=NEW && m_mode!=APPEND){ - bool ret=true; - unsigned int tmp=0; - ret*=(*this) >> tmp; - rhs.resize(tmp); - if(tmp){ - if(check_base::value){ - for(typename std::vector::iterator it=rhs.begin(); it!=rhs.end(); it++) ret*=(*this) >> (*it); - } - else ret*=Bread(&(rhs[0]), tmp*sizeof(T)); - } - return ret; - } - else return false; + + template bool operator>>(std::vector& rhs) { + if (m_mode == NEW || m_mode == APPEND) return false; + + unsigned int size = 0; + if (!(*this >> size)) return false; + + rhs.resize(size); + if (size == 0) return true; + + if (!check_base::value) + return Bread(rhs.data(), size * sizeof(T)); + + for (typename std::vector::iterator it = rhs.begin(); it != rhs.end(); ++it) + if (!(*this >> *it)) return false; + return true; } template bool operator&(std::vector& rhs){ @@ -190,143 +184,135 @@ class BinaryStream : public SerialisableObject{ if(m_write) return (*this) << rhs; else return (*this) >> rhs; } - - bool operator<<(std::vector& rhs){ - if(m_mode!=READ){ - bool ret=true; - unsigned int tmp=rhs.size(); - ret*=(*this) << tmp; - for(unsigned int i=0; i& rhs) { + if (m_mode == READ) return false; + + unsigned int size = rhs.size(); + if (!(*this << size)) return false; + + for (unsigned int i = 0; i < size; ++i) + if (!(*this << rhs[i])) return false; + return true; } - - bool operator>>(std::vector& rhs){ - if(m_mode!=NEW && m_mode!=APPEND){ - bool ret=true; - unsigned int tmp=0; - ret*=(*this) >> tmp; - rhs.resize(tmp); - for(unsigned int i=0; i> rhs.at(i); - } - return ret; - } - else return false; + + bool operator>>(std::vector& rhs) { + if (m_mode == NEW || m_mode == APPEND) return false; + + unsigned int size = 0; + if (!(*this >> size)) return false; + + rhs.resize(size); + for (unsigned int i = 0; i < size; ++i) + if (!(*this >> rhs[i])) return false; + return true; } bool operator&(std::vector& rhs){ if(m_write) return (*this) << rhs; else return (*this) >> rhs; } - - template bool operator<<(std::map& rhs){ - if(m_mode!=READ){ - bool ret=true; - unsigned int tmp=rhs.size(); - ret*=(*this) << tmp; - for (typename std::map::iterator it=rhs.begin(); it!=rhs.end(); ++it){ - T key=it->first; - U value=it->second; - ret*=(*this) << key; - ret*=(*this) << value; - } - return ret; + + template bool operator<<(std::map& rhs) { + if (m_mode == READ) return false; + + unsigned int size = rhs.size(); + if (!(*this << size)) return false; + + for (typename std::map::iterator it = rhs.begin(); it != rhs.end(); ++it) { + T key = it->first; + U value = it->second; + if (!(*this << key && *this << value)) return false; } - else return false; + return true; } - - template bool operator>>(std::map& rhs){ - if(m_mode!=NEW && m_mode!=APPEND){ - bool ret=true; - unsigned int tmp=0; - ret*=(*this) >> tmp; - for (unsigned int i=0; i> key; - ret*=(*this) >> value; - rhs[key]=value; - } - return ret; - } - else return false; + + template bool operator>>(std::map& rhs) { + if (m_mode == NEW || m_mode == APPEND) return false; + + unsigned int size = 0; + if (!(*this >> size)) return false; + + for (unsigned int i = 0; i < size; ++i) { + T key; + U value; + if (!(*this >> key && *this >> value)) return false; + rhs[key] = value; + }; + return true; } template bool operator&(std::map& rhs){ if(m_write) return (*this) << rhs; else return (*this) >> rhs; } - - template bool operator<<(std::deque& rhs){ - if(m_mode!=READ){ - bool ret=true; - unsigned int tmp=rhs.size(); - ret*=(*this) << tmp; - if(tmp){ - for(typename std::deque::iterator it=rhs.begin(); it!=rhs.end(); it++) - if(check_base::value) - ret*=(*this) << (*it); - else - ret*=Bwrite(&*it, sizeof(T)); - } - return ret; - } - else return false; + + template bool operator<<(std::deque& rhs) { + if (m_mode == READ) return false; + + unsigned int size = rhs.size(); + if (!(*this << size)) return false; + + if (size == 0) return true; + + for (typename std::deque::iterator it = rhs.begin(); + it != rhs.end(); + ++it) + if (check_base::value) { + if (!(*this << *it)) + return false; + } else if (!Bwrite(&*it, sizeof(T))) + return false; + return true; } - - template bool operator>>(std::deque& rhs){ - if(m_mode!=NEW && m_mode!=APPEND){ - bool ret=true; - unsigned int tmp=0; - ret*=(*this) >> tmp; - rhs.resize(tmp); - if(tmp){ - for(typename std::deque::iterator it=rhs.begin(); it!=rhs.end(); it++) - if(check_base::value) - ret*=(*this) >> (*it); - else - ret*=Bread(&*it, sizeof(T)); - } - return ret; - } - else return false; + + template bool operator>>(std::deque& rhs) { + if (m_mode == NEW || m_mode == APPEND) return false; + + unsigned int size = 0; + if (!(*this >> size)) return false; + + rhs.resize(size); + if (size == 0) return true; + + for (typename std::deque::iterator it = rhs.begin(); + it != rhs.end(); + ++it) + if (check_base::value) { + if (!(*this >> *it)) + return false; + } else if (!Bread(&*it, sizeof(T))) + return false; + return true; } - + template bool operator&(std::deque& rhs){ if(m_write) return(*this) << rhs; else return(*this) >> rhs; } - - bool operator<<(std::deque& rhs){ - if(m_mode!=READ){ - bool ret=true; - unsigned int tmp=rhs.size(); - ret*=(*this) << tmp; - for(unsigned int i=0; i>(std::deque& rhs){ - if(m_mode!=NEW && m_mode!=APPEND){ - bool ret=true; - unsigned int tmp=0; - ret*=(*this) >> tmp; - rhs.resize(tmp); - for(unsigned int i=0; i> rhs.at(i); - } - return ret; - } - else return false; - } + + bool operator<<(std::deque& rhs) { + if (m_mode == READ) return false; + + unsigned int len = rhs.size(); + if (!(*this << len)) return false; + + for (unsigned int i = 0; i < len; ++i) + if (!(*this << rhs[i])) return false; + return true; + }; + + bool operator>>(std::deque& rhs) { + if (m_mode == NEW || m_mode == APPEND) return false; + + unsigned int size = 0; + if (!(*this >> size)) return false; + + rhs.resize(size); + for (unsigned int i = 0; i < size; ++i) + if (!(*this >> rhs[i])) return false; + return true; + }; bool operator&(std::deque& rhs){ if(m_write) return (*this) << rhs; @@ -355,9 +341,7 @@ class BinaryStream : public SerialisableObject{ static const bool value = sizeof(check(Host(), int())) == sizeof(short); }; - - - + /* derived: From 3d43ef97ad4d5c74a839032d2fc81da39e4e15cf Mon Sep 17 00:00:00 2001 From: Evgenii Zhemchugov Date: Wed, 22 Feb 2023 12:09:34 +0600 Subject: [PATCH 3/7] BinaryStream: generalize the API and make it work with const references The function ```c++ void f(BinaryStream& bs, const std::vector& v) { bs << v; } ``` failed to compile because `template bool BinaryStream::operator<<(const& T)` was used instead of `template bool BinaryStream::operator<<(std::vector&)`. In this patch, BinaryStream API was redesigned, duplicating code was removed and `BinaryStream::operator<<` was made less fragile when working with const references. The changes are fully backwards compatible. Note that normally there is no need for serialization to mutate the object, so `BinaryStream::operator<<` should be working with const references only. However, `SerialisableObject::SerialiseWrapper` is not a constant method because it does both serialization and deserialization. A proper design would be to separate this logic to two methods. This is to be considered in the future. For now, since instances of class SerialisableObject can appear in vectors and other containers, we have to require non-const references for the corresponding specializations of `BinaryStream::opreator<<`. --- src/Store/BinaryStream.h | 340 ++++++++++++++------------------------- 1 file changed, 117 insertions(+), 223 deletions(-) diff --git a/src/Store/BinaryStream.h b/src/Store/BinaryStream.h index d2ec9e8..b219abe 100644 --- a/src/Store/BinaryStream.h +++ b/src/Store/BinaryStream.h @@ -22,6 +22,25 @@ enum enum_endpoint { RAM , UNCOMPRESSED , POST_PRE_COMPRESS, COMPRESSED }; enum enum_mode { READ , NEW , APPEND, UPDATE, READ_APPEND, NEW_READ }; class BinaryStream : public SerialisableObject{ + private: + // Equivalent to std::is_baseof in C++11 + template + class is_base_of { + private: + static short check(const Base*); + static char check(...); + public: + static const bool value + = sizeof(check(static_cast(0))) == sizeof(short); + }; + + // Equivalent to std::enable_if in C++11 + template struct enable_if {}; + + template + struct enable_if { + typedef T type; + }; public: @@ -56,15 +75,45 @@ class BinaryStream : public SerialisableObject{ //operator overloads - bool operator<<(std::string& rhs) { + template bool operator&(const T& rhs) { + if (!m_write) return false; + return *this << rhs; + } + + template bool operator&(T& rhs) { + if (m_write) return *this << rhs; + return *this >> rhs; + } + + template + typename enable_if::value, bool>::type + operator<<(T& rhs) { if (m_mode == READ) return false; + m_write = true; + return rhs.SerialiseWrapper(*this); + } - unsigned int len = rhs.length(); - if (!(*this << len)) return false; + template + typename enable_if::value, bool>::type + operator>>(T& rhs) { + if (m_mode == NEW || m_mode == APPEND) return false; + m_write = false; + return rhs.SerialiseWrapper(*this); + } - if (len == 0) return true; - return Bwrite(rhs.data(), len); - }; + template + typename enable_if::value, bool>::type + operator<<(const T& rhs) { + if (m_mode == READ) return false; + return Bwrite(&rhs, sizeof(T)); + } + + template + typename enable_if::value, bool>::type + operator>>(T& rhs) { + if (m_mode == NEW || m_mode == APPEND) return false; + return Bread(&rhs, sizeof(T)); + } bool operator<<(const std::string& rhs) { if (m_mode == READ) return false; @@ -74,7 +123,7 @@ class BinaryStream : public SerialisableObject{ if (len == 0) return true; return Bwrite(rhs.data(), len); - }; + } bool operator>>(std::string& rhs) { if (m_mode == NEW || m_mode == APPEND) return false; @@ -85,84 +134,35 @@ class BinaryStream : public SerialisableObject{ rhs.resize(len); if (len == 0) return true; return Bread(rhs.data(), len); - }; - - bool operator&(std::string& rhs){ - if(m_write) return (*this) << rhs; - else return (*this) >> rhs; } - bool operator&(const std::string& rhs){ - if(m_write) return (*this) << rhs; - return false; + template + typename enable_if::value, bool>::type + operator<<(std::vector& rhs) { + return serialise_container(rhs); } - - template bool operator<<(T& rhs){ - if(m_mode!=READ){ - if(check_base::value){ - SerialisableObject* tmp=reinterpret_cast(&rhs); - m_write=true; - return tmp->SerialiseWrapper(*this); - } - else return Bwrite(&rhs, sizeof(T)); - } - else return false; + template + typename enable_if::value, bool>::type + operator>>(std::vector& rhs) { + return deserialise_container(rhs); } - - template bool operator>>(T& rhs){ - if(m_mode!=NEW && m_mode!=APPEND){ - if(check_base::value){ - SerialisableObject* tmp=reinterpret_cast(&rhs); - m_write=false; - return tmp->SerialiseWrapper(*this); - } - else return Bread(&rhs, sizeof(T)); - } - return false; - } - - - template bool operator&(T& rhs){ - if(m_write) return (*this) << rhs; - else return (*this) >> rhs; - } - - template bool operator<<(const T& rhs){ - if(m_mode!=READ){ - if(check_base::value){ - SerialisableObject* tmp=reinterpret_cast(&rhs); - m_write=true; - return tmp->SerialiseWrapper(*this); - } - return Bwrite(&rhs, sizeof(T)); - } - else return false; - } - - template bool operator&(const T& rhs){ - if(m_write) return (*this) << rhs; - return false; - } - - template bool operator<<(std::vector& rhs) { + template + typename enable_if::value, bool>::type + operator<<(const std::vector& rhs) { if (m_mode == READ) return false; unsigned int size = rhs.size(); if (!(*this << size)) return false; if (size == 0) return true; - - if (!check_base::value) - return Bwrite(rhs.data(), size * sizeof(T)); - - for (typename std::vector::iterator it = rhs.begin(); it != rhs.end(); ++it) - if (!(*this << *it)) return false; - return true; + return Bwrite(rhs.data(), size * sizeof(T)); } - template bool operator>>(std::vector& rhs) { + template + typename enable_if::value, bool>::type + operator>>(std::vector& rhs) { if (m_mode == NEW || m_mode == APPEND) return false; unsigned int size = 0; @@ -170,64 +170,35 @@ class BinaryStream : public SerialisableObject{ rhs.resize(size); if (size == 0) return true; - - if (!check_base::value) - return Bread(rhs.data(), size * sizeof(T)); - - for (typename std::vector::iterator it = rhs.begin(); it != rhs.end(); ++it) - if (!(*this >> *it)) return false; - return true; - } - - template bool operator&(std::vector& rhs){ - - if(m_write) return (*this) << rhs; - else return (*this) >> rhs; + return Bread(rhs.data(), size * sizeof(T)); } - bool operator<<(std::vector& rhs) { - if (m_mode == READ) return false; + bool operator<<(const std::vector& rhs) { + return serialise_container(rhs); + }; - unsigned int size = rhs.size(); - if (!(*this << size)) return false; + bool operator>>(std::vector& rhs) { + return deserialise_container(rhs); + }; - for (unsigned int i = 0; i < size; ++i) - if (!(*this << rhs[i])) return false; - return true; + template + bool operator<<(std::pair& rhs) { + if (m_mode == READ) return false; + return *this << rhs.first && *this << rhs.second; } - bool operator>>(std::vector& rhs) { + template + bool operator>>(std::pair& rhs) { if (m_mode == NEW || m_mode == APPEND) return false; - - unsigned int size = 0; - if (!(*this >> size)) return false; - - rhs.resize(size); - for (unsigned int i = 0; i < size; ++i) - if (!(*this >> rhs[i])) return false; - return true; + return *this >> rhs.first && *this >> rhs.second; } - - bool operator&(std::vector& rhs){ - if(m_write) return (*this) << rhs; - else return (*this) >> rhs; - } - - template bool operator<<(std::map& rhs) { - if (m_mode == READ) return false; - - unsigned int size = rhs.size(); - if (!(*this << size)) return false; - for (typename std::map::iterator it = rhs.begin(); it != rhs.end(); ++it) { - T key = it->first; - U value = it->second; - if (!(*this << key && *this << value)) return false; - } - return true; + template bool operator<<(std::map& rhs) { + return serialise_container(rhs); } - template bool operator>>(std::map& rhs) { + template bool operator>>(std::map& rhs) { + // std::map has no resize, so we cannot use deserialise_container if (m_mode == NEW || m_mode == APPEND) return false; unsigned int size = 0; @@ -241,130 +212,53 @@ class BinaryStream : public SerialisableObject{ }; return true; } - - template bool operator&(std::map& rhs){ - if(m_write) return (*this) << rhs; - else return (*this) >> rhs; - } - template bool operator<<(std::deque& rhs) { + template bool operator<<(std::deque& rhs) { + return serialise_container(rhs); + } + + template bool operator>>(std::deque& rhs) { + return deserialise_container(rhs); + } + + private: + + int def(FILE *source, FILE *dest, int level); + int inf(FILE *source, FILE *dest); + void zerr(int ret); + + template + bool serialise_container(const Container& container) { if (m_mode == READ) return false; - unsigned int size = rhs.size(); + unsigned int size = container.size(); if (!(*this << size)) return false; if (size == 0) return true; - for (typename std::deque::iterator it = rhs.begin(); - it != rhs.end(); - ++it) - if (check_base::value) { - if (!(*this << *it)) - return false; - } else if (!Bwrite(&*it, sizeof(T))) - return false; + for (typename Container::const_iterator i = container.begin(); + i != container.end(); + ++i) + if (!(*this << *i)) return false; return true; } - template bool operator>>(std::deque& rhs) { + template + bool deserialise_container(Container& container) { if (m_mode == NEW || m_mode == APPEND) return false; unsigned int size = 0; if (!(*this >> size)) return false; - rhs.resize(size); if (size == 0) return true; - for (typename std::deque::iterator it = rhs.begin(); - it != rhs.end(); - ++it) - if (check_base::value) { - if (!(*this >> *it)) - return false; - } else if (!Bread(&*it, sizeof(T))) - return false; + container.resize(size); + for (typename Container::iterator i = container.begin(); + i != container.end(); + ++i) + if (!(*this >> *i)) return false; return true; } - - template bool operator&(std::deque& rhs){ - if(m_write) return(*this) << rhs; - else return(*this) >> rhs; - } - - bool operator<<(std::deque& rhs) { - if (m_mode == READ) return false; - - unsigned int len = rhs.size(); - if (!(*this << len)) return false; - - for (unsigned int i = 0; i < len; ++i) - if (!(*this << rhs[i])) return false; - return true; - }; - - bool operator>>(std::deque& rhs) { - if (m_mode == NEW || m_mode == APPEND) return false; - - unsigned int size = 0; - if (!(*this >> size)) return false; - - rhs.resize(size); - for (unsigned int i = 0; i < size; ++i) - if (!(*this >> rhs[i])) return false; - return true; - }; - - bool operator&(std::deque& rhs){ - if(m_write) return (*this) << rhs; - else return (*this) >> rhs; - } - - - private: - - int def(FILE *source, FILE *dest, int level); - int inf(FILE *source, FILE *dest); - void zerr(int ret); - - template struct Host{ - - operator B*() const; - operator D*(); - - }; - - - template struct check_base { - template - static short check(D*, T); - static char check(B*, int); - - static const bool value = sizeof(check(Host(), int())) == sizeof(short); - }; - - /* -derived: - - yes D*(Host(),T) = D*(B* const, T); not allowed - = D*(D*, T); -----------------------------------------------------------------------------------> preferred as D*->D* better than D*->B* : answer yes - - no B*(Host(), int) = B*(B* const, int); - = B*(D*, int); preferred as not const and object called on is not const-----------------------> - - -not derrived: - - yes D*(Host(),T) = D*(B* const, T); not allowed - = D*(D*, T); ------------------------------> - - no B*(Host(), int) = B*(B* const, int);-----------------------> preffered as not templated : answer no - = B*(D*, int); not allowed - - - */ - - }; - #endif From baad6f3120fb25a031d34217b9a4b817075dbea8 Mon Sep 17 00:00:00 2001 From: Evgenii Zhemchugov Date: Wed, 22 Feb 2023 12:37:26 +0600 Subject: [PATCH 4/7] BinaryStream: avoid redundant tests for the mode --- src/Store/BinaryStream.h | 113 ++++++++++++++++++--------------------- 1 file changed, 51 insertions(+), 62 deletions(-) diff --git a/src/Store/BinaryStream.h b/src/Store/BinaryStream.h index b219abe..4a243fa 100644 --- a/src/Store/BinaryStream.h +++ b/src/Store/BinaryStream.h @@ -85,52 +85,64 @@ class BinaryStream : public SerialisableObject{ return *this >> rhs; } + template bool operator<<(const T& rhs) { + if (m_mode == READ) return false; + return serialise(rhs); + } + + template bool operator<<(T& rhs) { + if (m_mode == READ) return false; + return serialise(rhs); + } + + template bool operator>>(T& rhs) { + if (m_mode == NEW || m_mode == APPEND) return false; + return deserialise(rhs); + } + + private: + + int def(FILE *source, FILE *dest, int level); + int inf(FILE *source, FILE *dest); + void zerr(int ret); + template typename enable_if::value, bool>::type - operator<<(T& rhs) { - if (m_mode == READ) return false; + serialise(T& rhs) { m_write = true; return rhs.SerialiseWrapper(*this); } template typename enable_if::value, bool>::type - operator>>(T& rhs) { - if (m_mode == NEW || m_mode == APPEND) return false; + deserialise(T& rhs) { m_write = false; return rhs.SerialiseWrapper(*this); } template typename enable_if::value, bool>::type - operator<<(const T& rhs) { - if (m_mode == READ) return false; + serialise(const T& rhs) { return Bwrite(&rhs, sizeof(T)); } template typename enable_if::value, bool>::type - operator>>(T& rhs) { + deserialise(T& rhs) { if (m_mode == NEW || m_mode == APPEND) return false; return Bread(&rhs, sizeof(T)); } - bool operator<<(const std::string& rhs) { - if (m_mode == READ) return false; - + bool serialise(const std::string& rhs) { unsigned int len = rhs.length(); - if (!(*this << len)) return false; - + if (!serialise(len)) return false; if (len == 0) return true; return Bwrite(rhs.data(), len); } - bool operator>>(std::string& rhs) { - if (m_mode == NEW || m_mode == APPEND) return false; - + bool deserialise(std::string& rhs) { unsigned int len = 0; - if (!(*this >> len)) return false; - + if (!deserialise(len)) return false; rhs.resize(len); if (len == 0) return true; return Bread(rhs.data(), len); @@ -138,125 +150,102 @@ class BinaryStream : public SerialisableObject{ template typename enable_if::value, bool>::type - operator<<(std::vector& rhs) { + serialise(std::vector& rhs) { return serialise_container(rhs); } template typename enable_if::value, bool>::type - operator>>(std::vector& rhs) { + deserialise(std::vector& rhs) { return deserialise_container(rhs); } template typename enable_if::value, bool>::type - operator<<(const std::vector& rhs) { - if (m_mode == READ) return false; - + serialise(const std::vector& rhs) { unsigned int size = rhs.size(); - if (!(*this << size)) return false; - + if (!serialise(size)) return false; if (size == 0) return true; return Bwrite(rhs.data(), size * sizeof(T)); } template typename enable_if::value, bool>::type - operator>>(std::vector& rhs) { - if (m_mode == NEW || m_mode == APPEND) return false; - + deserialise(std::vector& rhs) { unsigned int size = 0; - if (!(*this >> size)) return false; - + if (!deserialise(size)) return false; rhs.resize(size); if (size == 0) return true; return Bread(rhs.data(), size * sizeof(T)); } - bool operator<<(const std::vector& rhs) { + bool serialise(const std::vector& rhs) { return serialise_container(rhs); }; - bool operator>>(std::vector& rhs) { + bool deserialise(std::vector& rhs) { return deserialise_container(rhs); }; template - bool operator<<(std::pair& rhs) { - if (m_mode == READ) return false; - return *this << rhs.first && *this << rhs.second; + bool serialise(std::pair& rhs) { + return serialise(rhs.first) && serialise(rhs.second); } template - bool operator>>(std::pair& rhs) { - if (m_mode == NEW || m_mode == APPEND) return false; - return *this >> rhs.first && *this >> rhs.second; + bool deserialise(std::pair& rhs) { + return deserialise(rhs.first) && deserialise(rhs.second); } - template bool operator<<(std::map& rhs) { + template bool serialise(std::map& rhs) { return serialise_container(rhs); } - template bool operator>>(std::map& rhs) { + template bool deserialise(std::map& rhs) { // std::map has no resize, so we cannot use deserialise_container - if (m_mode == NEW || m_mode == APPEND) return false; - unsigned int size = 0; - if (!(*this >> size)) return false; + if (!deserialise(size)) return false; for (unsigned int i = 0; i < size; ++i) { T key; U value; - if (!(*this >> key && *this >> value)) return false; + if (!(deserialise(key) && deserialise(value))) return false; rhs[key] = value; }; return true; } - template bool operator<<(std::deque& rhs) { + template bool serialise(std::deque& rhs) { return serialise_container(rhs); } - template bool operator>>(std::deque& rhs) { + template bool deserialise(std::deque& rhs) { return deserialise_container(rhs); } - private: - - int def(FILE *source, FILE *dest, int level); - int inf(FILE *source, FILE *dest); - void zerr(int ret); template bool serialise_container(const Container& container) { - if (m_mode == READ) return false; - unsigned int size = container.size(); - if (!(*this << size)) return false; - + if (!serialise(size)) return false; if (size == 0) return true; - for (typename Container::const_iterator i = container.begin(); i != container.end(); ++i) - if (!(*this << *i)) return false; + if (!serialise(*i)) return false; return true; } template bool deserialise_container(Container& container) { - if (m_mode == NEW || m_mode == APPEND) return false; - unsigned int size = 0; - if (!(*this >> size)) return false; - + if (!deserialise(size)) return false; if (size == 0) return true; - container.resize(size); for (typename Container::iterator i = container.begin(); i != container.end(); ++i) - if (!(*this >> *i)) return false; + if (!deserialise(*i)) return false; return true; } }; From bfe0f2fd46334e966580d4c58458136445234755 Mon Sep 17 00:00:00 2001 From: Evgenii Zhemchugov Date: Wed, 22 Feb 2023 13:56:56 +0600 Subject: [PATCH 5/7] src/Store/BStore.cpp: fix compiler warning on "/*" within comment Use `#if 0 ... #endif` instead. --- src/Store/BStore.cpp | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/src/Store/BStore.cpp b/src/Store/BStore.cpp index 27532dd..3a53e4a 100644 --- a/src/Store/BStore.cpp +++ b/src/Store/BStore.cpp @@ -746,7 +746,7 @@ bool BStore::Serialise(BinaryStream &bs){ // do return properly bs & output; bs & m_lookup; GetEntry(0); - /* +#if 0 save; if(bs.m_write){ @@ -761,9 +761,9 @@ bool BStore::Serialise(BinaryStream &bs){ // do return properly close open } - */ +#endif - /* +#if 0 bs & m_lookup; @@ -789,7 +789,7 @@ bool BStore::Serialise(BinaryStream &bs){ // do return properly if(!GetHeader()) return false; if(!GetEntry(0)) return false; - */ +#endif @@ -801,7 +801,7 @@ bool BStore::Serialise(BinaryStream &bs){ // do return properly ///// - /* // uncomment here to start +#if 0 // uncomment here to start //writing if(bs.m_write){ if(not ram){ @@ -856,7 +856,7 @@ bool BStore::Serialise(BinaryStream &bs){ // do return properly } - /* +#if 0 //std::cout<<"p1"< Date: Wed, 22 Feb 2023 14:01:41 +0600 Subject: [PATCH 6/7] SerialisableObject: add virtual destructor Otherwise deleting pointers to derived classes might cause memory leaks. --- src/Store/SerialisableObject.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/Store/SerialisableObject.h b/src/Store/SerialisableObject.h index 60ec016..d4853c1 100644 --- a/src/Store/SerialisableObject.h +++ b/src/Store/SerialisableObject.h @@ -20,6 +20,8 @@ class SerialisableObject{ public: + virtual ~SerialisableObject() {}; + bool SerialiseWrapper(BinaryStream &bs); virtual bool Serialise(BinaryStream &bs)=0; virtual std::string GetVersion()=0; From 3793f9eda3a73c9f902c388266d53473d47f2e49 Mon Sep 17 00:00:00 2001 From: Evgenii Zhemchugov Date: Tue, 28 Feb 2023 18:52:29 +0600 Subject: [PATCH 7/7] BinaryStream::(de-)serialise(std::string& rhs): revert to using &rhs[0] instead of rhs.data() In C++ before C++11, meddling with a string using the pointer returned by `string.data()`, `string.c_str()` or `&string[0]` are undefined behaviour. `std::string` was not required storing its contents in a continuous buffer, and GCC implemented copy-on-write strings. Nevertheless, this code apparently worked in the environments it was used, because GCC does store string contents in a continuous buffer, and calling `resize()` before deserializing probably helped with copy-on-writing. In newer standards of C++, this is no longer an undefined behaviour. --- src/Store/BinaryStream.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Store/BinaryStream.h b/src/Store/BinaryStream.h index 4a243fa..95fda6d 100644 --- a/src/Store/BinaryStream.h +++ b/src/Store/BinaryStream.h @@ -137,7 +137,7 @@ class BinaryStream : public SerialisableObject{ unsigned int len = rhs.length(); if (!serialise(len)) return false; if (len == 0) return true; - return Bwrite(rhs.data(), len); + return Bwrite(&rhs[0], len); } bool deserialise(std::string& rhs) { @@ -145,7 +145,7 @@ class BinaryStream : public SerialisableObject{ if (!deserialise(len)) return false; rhs.resize(len); if (len == 0) return true; - return Bread(rhs.data(), len); + return Bread(&rhs[0], len); } template