Skip to content

Commit

Permalink
Fix flexbuffer copy/assignment & clear method
Browse files Browse the repository at this point in the history 
Fixed freeing and deep copying `ddata_` from rhs, and fixed `clear` method to free `ddata_`.
  • Loading branch information
@smlu
smlu committed Dec 29, 2023
1 parent 80ad5cd commit 4cdabc0
Show file tree
Hide file tree
Showing 3 changed files with 93 additions and 33 deletions.
2 changes: 1 addition & 1 deletion include/ack/bigint.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1282,7 +1282,7 @@ namespace ack {
{
if (&rhs != this)
{
buf_ = std::move(rhs.buf_);
buf_ = std::move( rhs.buf_ );
size_ = rhs.size_;
is_neg_ = rhs.is_neg_;
}
Expand Down
122 changes: 91 additions & 31 deletions include/ack/buffer.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,13 +70,13 @@ namespace ack {
public:
using value_type = T;

constexpr fixed_buffer() = default;
constexpr fixed_buffer(const fixed_buffer& rhs) = default;
constexpr fixed_buffer(fixed_buffer&& rhs) = default;
constexpr fixed_buffer& operator=(const fixed_buffer& rhs) = default;
constexpr fixed_buffer& operator=(fixed_buffer&& rhs) = default;
constexpr fixed_buffer() noexcept = default;
constexpr fixed_buffer(const fixed_buffer& rhs) noexcept = default;
constexpr fixed_buffer(fixed_buffer&& rhs) noexcept = default;
constexpr fixed_buffer& operator=(const fixed_buffer& rhs) noexcept = default;
constexpr fixed_buffer& operator=(fixed_buffer&& rhs) noexcept = default;

constexpr bool resize(size_t n)
constexpr bool resize(size_t n) noexcept
{
if ( n > N ) {
return false;
Expand All @@ -85,12 +85,12 @@ namespace ack {
return true;
}

constexpr void clear()
constexpr void clear() noexcept
{
size_ = 0;
}

constexpr T* data()
constexpr T* data() noexcept
{
return data_.data();
}
Expand All @@ -100,17 +100,17 @@ namespace ack {
return data_.data();
}

constexpr std::size_t size() const
constexpr std::size_t size() const noexcept
{
return size_;
}

constexpr std::size_t max_size() const
constexpr std::size_t max_size() const noexcept
{
return N;
}

constexpr void swap(fixed_buffer& rhs)
constexpr void swap(fixed_buffer& rhs) noexcept
{
std::swap( data_, rhs.data_ );
std::swap( size_, rhs.size_ );
Expand Down Expand Up @@ -138,25 +138,79 @@ namespace ack {
* @warning if buffer is resized over the size of stack allocated memory (N)
* data is re-allocated on the heap, and this data is never released
* due to constexpr constrains which prohibits defining custom destructor.
* You have to manually call `clear` to free heap allocated memory.
* The flexbuffer should be used only in short lived environments like WASM.
*/
template<typename T, std::size_t N>
class flexbuffer final: public buffer_base<flexbuffer<T, N>, T> {
public:
using value_type = T;

constexpr flexbuffer() = default;
constexpr flexbuffer(const flexbuffer& rhs) = default;
constexpr flexbuffer(flexbuffer&& rhs) = default;
constexpr flexbuffer& operator=(const flexbuffer& rhs) = default;
constexpr flexbuffer& operator=(flexbuffer&& rhs) = default;
constexpr flexbuffer() noexcept = default;
constexpr flexbuffer(const flexbuffer& rhs)
{
sdata_ = rhs.sdata_;
size_ = rhs.size_;
dsize_ = rhs.dsize_;
if ( !std::is_constant_evaluated() ) {
if ( rhs.ddata_ ) {
ddata_ = new T[dsize_];
memcpy( ddata_, rhs.ddata_ , dsize_ * sizeof( T ));
}
}
}

constexpr flexbuffer(flexbuffer&& rhs) noexcept
{
sdata_ = std::move( rhs.sdata_ );
size_ = rhs.size_;
dsize_ = rhs.dsize_;
ddata_ = rhs.ddata_;

rhs.size_ = 0;
rhs.dsize_ = 0;
rhs.ddata_ = nullptr;
}

// ~flex_buffer() // destructor deleted otherwise flex_buffer can't be constructed at compile time
constexpr flexbuffer& operator=(const flexbuffer& rhs)
{
if ( &rhs != this ) {
this->clear();
sdata_ = rhs.sdata_;
size_ = rhs.size_;
dsize_ = rhs.dsize_;
if ( !std::is_constant_evaluated() ) {
if ( rhs.ddata_ ) {
ddata_ = new T[dsize_];
memcpy( ddata_, rhs.ddata_ , dsize_ * sizeof( T ));
}
}
}
return *this;
}

constexpr flexbuffer& operator=(flexbuffer&& rhs) noexcept
{
if ( &rhs != this ) {
this->clear();
sdata_ = std::move( rhs.sdata_ );
size_ = rhs.size_;
dsize_ = rhs.dsize_;
ddata_ = rhs.ddata_;

rhs.size_ = 0;
rhs.dsize_ = 0;
rhs.ddata_ = nullptr;
}
return *this;
}

// constexpr ~flex_buffer() // destructor deleted otherwise flex_buffer can't be constructed at compile time
// {
// if ( std::is_constant_evaluated() ) {
// if ( ddata_ ) {
// delete[] ddata_;
// }
// if ( ddata_ ) {
// delete[] ddata_;
// }
// }
// }

Expand All @@ -168,19 +222,18 @@ namespace ack {
}
}
else {
if ( n > N && n > dsize ) {

if ( n > N && n > dsize_ ) {
bool scpy = ( ddata_ == nullptr );
T* pold = ddata_;

dsize += std::max( N, n );
ddata_ = new T[dsize];
dsize_ += std::max( N, n );
ddata_ = new T[dsize_];

if ( scpy ) {
memcpy( ddata_, sdata_.data(), N * sizeof( T ));
}
else{
memcpy( ddata_, pold, (dsize - std::max( N, n )) * sizeof( T ));
else {
memcpy( ddata_, pold, (dsize_ - std::max( N, n )) * sizeof( T ));
delete[] pold;
pold = nullptr;
}
Expand All @@ -193,7 +246,14 @@ namespace ack {

constexpr void clear()
{
size_ = 0;
if ( !std::is_constant_evaluated() ) {
if ( ddata_ ) {
delete [] ddata_;
ddata_ = nullptr;
}
}
dsize_ = 0;
size_ = 0;
}

constexpr T* data()
Expand All @@ -203,7 +263,7 @@ namespace ack {

constexpr const T* data() const
{
return ddata_? ddata_ : sdata_.data();
return ddata_ ? ddata_ : sdata_.data();
}

constexpr std::size_t size() const
Expand All @@ -220,7 +280,7 @@ namespace ack {
{
std::swap( sdata_, rhs.sdata_ );
if ( !std::is_constant_evaluated() ) {
std::swap( dsize, rhs.dsize );
std::swap( dsize_, rhs.dsize_ );
std::swap( ddata_, rhs.ddata_ );
}
std::swap( size_, rhs.size_ );
Expand Down Expand Up @@ -251,8 +311,8 @@ namespace ack {
private:
std::array<T, N> sdata_ = {};
T* ddata_ = nullptr; // replace with std::vector<T> when C++20 constexpr ctor is supported
std::size_t size_ = 0;
std::size_t dsize = 0;
std::size_t size_ = 0;
std::size_t dsize_ = 0;
};
template<std::size_t N>
using fixed_word_buffer = fixed_buffer<word_t, N>;
Expand Down
2 changes: 1 addition & 1 deletion include/ack/ec.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1697,7 +1697,7 @@ namespace ack {
*/
[[nodiscard]]
inline field_element_type compute_y(const field_element_type& x, const bool odd) const {
auto y = (( x * x + a ) * x + b ).sqrt();
auto y = (( x.sqr() + a ) * x + b ).sqrt();
if ( odd != y.value().test_bit( 0 )) {
y = -y;
}
Expand Down

0 comments on commit 4cdabc0

Please sign in to comment.