foster/bitcoin-bitcoin-core
0
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2025-02-08 10:31:50 -05:00
Code Issues Activity

Merge bitcoin/bitcoin#24253: Remove broken and unused CDataStream methods

Browse source 
fa1b227a72 Remove broken and unused CDataStream methods (MarcoFalke)
faee5f8dc2 test: Create fresh CDataStream each time (MarcoFalke)
fa71114926 test: Inline expected_xor (MarcoFalke)

Pull request description:

  The `insert` and `erase` methods have many issues:

  * They are unused
  * They are confusing and hard to read, as they implement "special cases" for optimization, that isn't needed
  * They are broken (See https://github.com/bitcoin/bitcoin/pull/24231)
  * Fixing them leads to mingw compile errors (See https://github.com/bitcoin/bitcoin/pull/24231#issuecomment-1029286985)

  Fix all issues by removing them

ACKs for top commit:
  laanwj:
    Code review ACK fa1b227a72

Tree-SHA512: 9d9e5d42e6ffc5ae82bdb67cfb5b50b45977ae674acee6ff99092560aebf2fc7e4584ded614e190db0663226fa198e34350517cd7ee57d518de22e9568bc349a
This commit is contained in:
laanwj 2022-02-09 15:17:21 +01:00
commit 5e8e0b3d7f
No known key found for this signature in database
GPG key ID: 1E4AED62986CD25D
3 changed files with 17 additions and 138 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

67
src/streams.h
View file

@ -240,76 +240,9 @@ public:
const_reference operator[](size_type pos) const { return vch[pos + nReadPos]; }
reference operator[](size_type pos) { return vch[pos + nReadPos]; }
void clear() { vch.clear(); nReadPos = 0; }
iterator insert(iterator it, const value_type x) { return vch.insert(it, x); }
void insert(iterator it, size_type n, const value_type x) { vch.insert(it, n, x); }
value_type* data() { return vch.data() + nReadPos; }
const value_type* data() const { return vch.data() + nReadPos; }
void insert(iterator it, std::vector<value_type>::const_iterator first, std::vector<value_type>::const_iterator last)
{
if (last == first) return;
assert(last - first > 0);
if (it == vch.begin() + nReadPos && (unsigned int)(last - first) <= nReadPos)
{
// special case for inserting at the front when there's room
nReadPos -= (last - first);
memcpy(&vch[nReadPos], &first[0], last - first);
}
else
vch.insert(it, first, last);
}
void insert(iterator it, const value_type* first, const value_type* last)
{
if (last == first) return;
assert(last - first > 0);
if (it == vch.begin() + nReadPos && (unsigned int)(last - first) <= nReadPos)
{
// special case for inserting at the front when there's room
nReadPos -= (last - first);
memcpy(&vch[nReadPos], &first[0], last - first);
}
else
vch.insert(it, first, last);
}
iterator erase(iterator it)
{
if (it == vch.begin() + nReadPos)
{
// special case for erasing from the front
if (++nReadPos >= vch.size())
{
// whenever we reach the end, we take the opportunity to clear the buffer
nReadPos = 0;
return vch.erase(vch.begin(), vch.end());
}
return vch.begin() + nReadPos;
}
else
return vch.erase(it);
}
iterator erase(iterator first, iterator last)
{
if (first == vch.begin() + nReadPos)
{
// special case for erasing from the front
if (last == vch.end())
{
nReadPos = 0;
return vch.erase(vch.begin(), vch.end());
}
else
{
nReadPos = (last - vch.begin());
return last;
}
}
else
return vch.erase(first, last);
}
inline void Compact()
{
vch.erase(vch.begin(), vch.begin() + nReadPos);

45
src/test/serialize_tests.cpp
View file

@ -215,51 +215,6 @@ BOOST_AUTO_TEST_CASE(noncanonical)
BOOST_CHECK_EXCEPTION(ReadCompactSize(ss), std::ios_base::failure, isCanonicalException);
}
BOOST_AUTO_TEST_CASE(insert_delete)
{
constexpr auto B2I{[](std::byte b) { return std::to_integer<uint8_t>(b); }};
// Test inserting/deleting bytes.
CDataStream ss(SER_DISK, 0);
BOOST_CHECK_EQUAL(ss.size(), 0U);
ss.write(MakeByteSpan("\x00\x01\x02\xff").first(4));
BOOST_CHECK_EQUAL(ss.size(), 4U);
uint8_t c{11};
// Inserting at beginning/end/middle:
ss.insert(ss.begin(), std::byte{c});
BOOST_CHECK_EQUAL(ss.size(), 5U);
BOOST_CHECK_EQUAL(B2I(ss[0]), c);
BOOST_CHECK_EQUAL(B2I(ss[1]), 0);
ss.insert(ss.end(), std::byte{c});
BOOST_CHECK_EQUAL(ss.size(), 6U);
BOOST_CHECK_EQUAL(B2I(ss[4]), 0xff);
BOOST_CHECK_EQUAL(B2I(ss[5]), c);
ss.insert(ss.begin() + 2, std::byte{c});
BOOST_CHECK_EQUAL(ss.size(), 7U);
BOOST_CHECK_EQUAL(B2I(ss[2]), c);
// Delete at beginning/end/middle
ss.erase(ss.begin());
BOOST_CHECK_EQUAL(ss.size(), 6U);
BOOST_CHECK_EQUAL(B2I(ss[0]), 0);
ss.erase(ss.begin()+ss.size()-1);
BOOST_CHECK_EQUAL(ss.size(), 5U);
BOOST_CHECK_EQUAL(B2I(ss[4]), 0xff);
ss.erase(ss.begin()+1);
BOOST_CHECK_EQUAL(ss.size(), 4U);
BOOST_CHECK_EQUAL(B2I(ss[0]), 0);
BOOST_CHECK_EQUAL(B2I(ss[1]), 1);
BOOST_CHECK_EQUAL(B2I(ss[2]), 2);
BOOST_CHECK_EQUAL(B2I(ss[3]), 0xff);
}
BOOST_AUTO_TEST_CASE(class_methods)
{
int intval(100);

43
src/test/streams_tests.cpp
View file

@ -8,6 +8,8 @@
#include <boost/test/unit_test.hpp>
using namespace std::string_literals;
BOOST_FIXTURE_TEST_SUITE(streams_tests, BasicTestingSetup)
BOOST_AUTO_TEST_CASE(streams_vector_writer)
@ -162,46 +164,35 @@ BOOST_AUTO_TEST_CASE(bitstream_reader_writer)
BOOST_AUTO_TEST_CASE(streams_serializedata_xor)
{
std::vector<std::byte> in;
std::vector<char> expected_xor;
CDataStream ds(in, 0, 0);
// Degenerate case
ds.Xor({0x00, 0x00});
BOOST_CHECK_EQUAL(
std::string(expected_xor.begin(), expected_xor.end()),
ds.str());
{
CDataStream ds{in, 0, 0};
ds.Xor({0x00, 0x00});
BOOST_CHECK_EQUAL(""s, ds.str());
}
in.push_back(std::byte{0x0f});
in.push_back(std::byte{0xf0});
expected_xor.push_back('\xf0');
expected_xor.push_back('\x0f');
// Single character key
ds.clear();
ds.insert(ds.begin(), in.begin(), in.end());
ds.Xor({0xff});
BOOST_CHECK_EQUAL(
std::string(expected_xor.begin(), expected_xor.end()),
ds.str());
{
CDataStream ds{in, 0, 0};
ds.Xor({0xff});
BOOST_CHECK_EQUAL("\xf0\x0f"s, ds.str());
}
// Multi character key
in.clear();
expected_xor.clear();
in.push_back(std::byte{0xf0});
in.push_back(std::byte{0x0f});
expected_xor.push_back('\x0f');
expected_xor.push_back('\x00');
ds.clear();
ds.insert(ds.begin(), in.begin(), in.end());
ds.Xor({0xff, 0x0f});
BOOST_CHECK_EQUAL(
std::string(expected_xor.begin(), expected_xor.end()),
ds.str());
{
CDataStream ds{in, 0, 0};
ds.Xor({0xff, 0x0f});
BOOST_CHECK_EQUAL("\x0f\x00"s, ds.str());
}
}
BOOST_AUTO_TEST_CASE(streams_buffered_file)