Skip to content

Commit

Permalink
test, build: Enable -Werror=sign-compare
Browse files Browse the repository at this point in the history 
Summary:
test: Fix outstanding -Wsign-compare errors
refactor: Rework asmap Interpret to avoid ptrdiff_t

This is a backport of Core [[bitcoin/bitcoin#18216 | PR18216]]

It also includes a change in asmap.cpp that was missed in D9055 ([[bitcoin/bitcoin#18512 | PR18512]])

Test Plan: ninja all check-all

Reviewers: #bitcoin_abc, Fabien

Reviewed By: #bitcoin_abc, Fabien

Subscribers: Fabien

Differential Revision: https://reviews.bitcoinabc.org/D9197
  • Loading branch information
@Empact @PiRK
Empact authored and PiRK committed Feb 10, 2021
1 parent 21862c3 commit 942d919
Show file tree
Hide file tree
Showing 15 changed files with 88 additions and 78 deletions.
1 change: 1 addition & 0 deletions src/CMakeLists.txt
View file
Expand Up @@ -227,6 +227,7 @@ add_compiler_flags(
-Wrange-loop-analysis
-Wredundant-decls
-Wunreachable-code-loop-increment
-Wsign-compare
)
add_compiler_flag_group(-Wformat -Wformat-security)
add_cxx_compiler_flags(
Expand Down
9 changes: 5 additions & 4 deletions src/test/blockfilter_index_tests.cpp
View file
Expand Up @@ -49,8 +49,8 @@ static bool CheckFilterLookups(BlockFilterIndex &filter_index,
BOOST_CHECK(filter_index.LookupFilterHashRange(block_index->nHeight,
block_index, filter_hashes));

BOOST_CHECK_EQUAL(filters.size(), 1);
BOOST_CHECK_EQUAL(filter_hashes.size(), 1);
BOOST_CHECK_EQUAL(filters.size(), 1U);
BOOST_CHECK_EQUAL(filter_hashes.size(), 1U);

BOOST_CHECK_EQUAL(filter.GetHash(), expected_filter.GetHash());
BOOST_CHECK_EQUAL(filter_header,
Expand Down Expand Up @@ -278,8 +278,9 @@ BOOST_FIXTURE_TEST_CASE(blockfilter_index_initial_sync,
BOOST_CHECK(filter_index.LookupFilterRange(0, tip, filters));
BOOST_CHECK(filter_index.LookupFilterHashRange(0, tip, filter_hashes));

BOOST_CHECK_EQUAL(filters.size(), tip->nHeight + 1);
BOOST_CHECK_EQUAL(filter_hashes.size(), tip->nHeight + 1);
BOOST_CHECK(tip->nHeight >= 0);
BOOST_CHECK_EQUAL(filters.size(), tip->nHeight + 1U);
BOOST_CHECK_EQUAL(filter_hashes.size(), tip->nHeight + 1U);

filters.clear();
filter_hashes.clear();
Expand Down
10 changes: 5 additions & 5 deletions src/test/blockfilter_tests.cpp
View file
Expand Up @@ -42,14 +42,14 @@ BOOST_AUTO_TEST_CASE(gcsfilter_test) {

BOOST_AUTO_TEST_CASE(gcsfilter_default_constructor) {
GCSFilter filter;
BOOST_CHECK_EQUAL(filter.GetN(), 0);
BOOST_CHECK_EQUAL(filter.GetEncoded().size(), 1);
BOOST_CHECK_EQUAL(filter.GetN(), 0U);
BOOST_CHECK_EQUAL(filter.GetEncoded().size(), 1U);

const GCSFilter::Params &params = filter.GetParams();
BOOST_CHECK_EQUAL(params.m_siphash_k0, 0);
BOOST_CHECK_EQUAL(params.m_siphash_k1, 0);
BOOST_CHECK_EQUAL(params.m_siphash_k0, 0U);
BOOST_CHECK_EQUAL(params.m_siphash_k1, 0U);
BOOST_CHECK_EQUAL(params.m_P, 0);
BOOST_CHECK_EQUAL(params.m_M, 1);
BOOST_CHECK_EQUAL(params.m_M, 1U);
}

BOOST_AUTO_TEST_CASE(blockfilter_basic_test) {
Expand Down
4 changes: 2 additions & 2 deletions src/test/bloom_tests.cpp
View file
Expand Up @@ -1124,7 +1124,7 @@ BOOST_AUTO_TEST_CASE(rolling_bloom) {
}
}
// Expect about 100 hits
BOOST_CHECK_EQUAL(nHits, 75);
BOOST_CHECK_EQUAL(nHits, 75U);

BOOST_CHECK(rb1.contains(data[DATASIZE - 1]));
rb1.reset();
Expand Down Expand Up @@ -1154,7 +1154,7 @@ BOOST_AUTO_TEST_CASE(rolling_bloom) {
}
}
// Expect about 5 false positives
BOOST_CHECK_EQUAL(nHits, 6);
BOOST_CHECK_EQUAL(nHits, 6U);

// last-1000-entry, 0.01% false positive:
CRollingBloomFilter rb2(1000, 0.001);
Expand Down
16 changes: 8 additions & 8 deletions src/test/compress_tests.cpp
View file
Expand Up @@ -75,14 +75,14 @@ BOOST_AUTO_TEST_CASE(compress_script_to_ckey_id) {
CScript script = CScript()
<< OP_DUP << OP_HASH160 << ToByteVector(pubkey.GetID())
<< OP_EQUALVERIFY << OP_CHECKSIG;
BOOST_CHECK_EQUAL(script.size(), 25);
BOOST_CHECK_EQUAL(script.size(), 25U);

std::vector<uint8_t> out;
bool done = CompressScript(script, out);
BOOST_CHECK_EQUAL(done, true);

// Check compressed script
BOOST_CHECK_EQUAL(out.size(), 21);
BOOST_CHECK_EQUAL(out.size(), 21U);
BOOST_CHECK_EQUAL(out[0], 0x00);
// compare the 20 relevant chars of the CKeyId in the script
BOOST_CHECK_EQUAL(memcmp(&out[1], &script[3], 20), 0);
Expand All @@ -92,14 +92,14 @@ BOOST_AUTO_TEST_CASE(compress_script_to_cscript_id) {
// case CScriptID
CScript script, redeemScript;
script << OP_HASH160 << ToByteVector(CScriptID(redeemScript)) << OP_EQUAL;
BOOST_CHECK_EQUAL(script.size(), 23);
BOOST_CHECK_EQUAL(script.size(), 23U);

std::vector<uint8_t> out;
bool done = CompressScript(script, out);
BOOST_CHECK_EQUAL(done, true);

// Check compressed script
BOOST_CHECK_EQUAL(out.size(), 21);
BOOST_CHECK_EQUAL(out.size(), 21U);
BOOST_CHECK_EQUAL(out[0], 0x01);
// compare the 20 relevant chars of the CScriptId in the script
BOOST_CHECK_EQUAL(memcmp(&out[1], &script[2], 20), 0);
Expand All @@ -112,14 +112,14 @@ BOOST_AUTO_TEST_CASE(compress_script_to_compressed_pubkey_id) {

// COMPRESSED_PUBLIC_KEY_SIZE (33)
CScript script = CScript() << ToByteVector(key.GetPubKey()) << OP_CHECKSIG;
BOOST_CHECK_EQUAL(script.size(), 35);
BOOST_CHECK_EQUAL(script.size(), 35U);

std::vector<uint8_t> out;
bool done = CompressScript(script, out);
BOOST_CHECK_EQUAL(done, true);

// Check compressed script
BOOST_CHECK_EQUAL(out.size(), 33);
BOOST_CHECK_EQUAL(out.size(), 33U);
BOOST_CHECK_EQUAL(memcmp(&out[0], &script[1], 1), 0);
// compare the 32 chars of the compressed CPubKey
BOOST_CHECK_EQUAL(memcmp(&out[1], &script[2], 32), 0);
Expand All @@ -132,14 +132,14 @@ BOOST_AUTO_TEST_CASE(compress_script_to_uncompressed_pubkey_id) {
// PUBLIC_KEY_SIZE (65)
CScript script = CScript() << ToByteVector(key.GetPubKey()) << OP_CHECKSIG;
// 1 char code + 65 char pubkey + OP_CHECKSIG
BOOST_CHECK_EQUAL(script.size(), 67);
BOOST_CHECK_EQUAL(script.size(), 67U);

std::vector<uint8_t> out;
bool done = CompressScript(script, out);
BOOST_CHECK_EQUAL(done, true);

// Check compressed script
BOOST_CHECK_EQUAL(out.size(), 33);
BOOST_CHECK_EQUAL(out.size(), 33U);
// first 32 chars of CPubKey are copied into out[1:]
BOOST_CHECK_EQUAL(memcmp(&out[1], &script[2], 32), 0);
// least significant bit (lsb) of last char of pubkey is mapped into out[0]
Expand Down
2 changes: 1 addition & 1 deletion src/test/descriptor_tests.cpp
View file
Expand Up @@ -293,7 +293,7 @@ void DoCheck(const std::string &prv, const std::string &pub, int flags,
FlatSigningProvider provider_inferred;
BOOST_CHECK(inferred->Expand(0, provider_inferred,
spks_inferred, provider_inferred));
BOOST_CHECK_EQUAL(spks_inferred.size(), 1);
BOOST_CHECK_EQUAL(spks_inferred.size(), 1U);
BOOST_CHECK(spks_inferred[0] == spks[n]);
BOOST_CHECK_EQUAL(
IsSolvable(provider_inferred, spks_inferred[0]),
Expand Down
16 changes: 8 additions & 8 deletions src/test/flatfile_tests.cpp
View file
Expand Up @@ -102,16 +102,16 @@ BOOST_AUTO_TEST_CASE(flatfile_allocate) {

bool out_of_space;

BOOST_CHECK_EQUAL(seq.Allocate(FlatFilePos(0, 0), 1, out_of_space), 100);
BOOST_CHECK_EQUAL(fs::file_size(seq.FileName(FlatFilePos(0, 0))), 100);
BOOST_CHECK_EQUAL(seq.Allocate(FlatFilePos(0, 0), 1, out_of_space), 100U);
BOOST_CHECK_EQUAL(fs::file_size(seq.FileName(FlatFilePos(0, 0))), 100U);
BOOST_CHECK(!out_of_space);

BOOST_CHECK_EQUAL(seq.Allocate(FlatFilePos(0, 99), 1, out_of_space), 0);
BOOST_CHECK_EQUAL(fs::file_size(seq.FileName(FlatFilePos(0, 99))), 100);
BOOST_CHECK_EQUAL(seq.Allocate(FlatFilePos(0, 99), 1, out_of_space), 0U);
BOOST_CHECK_EQUAL(fs::file_size(seq.FileName(FlatFilePos(0, 99))), 100U);
BOOST_CHECK(!out_of_space);

BOOST_CHECK_EQUAL(seq.Allocate(FlatFilePos(0, 99), 2, out_of_space), 101);
BOOST_CHECK_EQUAL(fs::file_size(seq.FileName(FlatFilePos(0, 99))), 200);
BOOST_CHECK_EQUAL(seq.Allocate(FlatFilePos(0, 99), 2, out_of_space), 101U);
BOOST_CHECK_EQUAL(fs::file_size(seq.FileName(FlatFilePos(0, 99))), 200U);
BOOST_CHECK(!out_of_space);
}

Expand All @@ -124,11 +124,11 @@ BOOST_AUTO_TEST_CASE(flatfile_flush) {

// Flush without finalize should not truncate file.
seq.Flush(FlatFilePos(0, 1));
BOOST_CHECK_EQUAL(fs::file_size(seq.FileName(FlatFilePos(0, 1))), 100);
BOOST_CHECK_EQUAL(fs::file_size(seq.FileName(FlatFilePos(0, 1))), 100U);

// Flush with finalize should truncate file.
seq.Flush(FlatFilePos(0, 1), true);
BOOST_CHECK_EQUAL(fs::file_size(seq.FileName(FlatFilePos(0, 1))), 1);
BOOST_CHECK_EQUAL(fs::file_size(seq.FileName(FlatFilePos(0, 1))), 1U);
}

BOOST_AUTO_TEST_SUITE_END()
2 changes: 1 addition & 1 deletion src/test/netbase_tests.cpp
View file
Expand Up @@ -483,7 +483,7 @@ BOOST_AUTO_TEST_CASE(netpermissions_test) {
error));

const auto strings = NetPermissions::ToStrings(PF_ALL);
BOOST_CHECK_EQUAL(strings.size(), 5);
BOOST_CHECK_EQUAL(strings.size(), 5U);
BOOST_CHECK(std::find(strings.begin(), strings.end(), "bloomfilter") !=
strings.end());
BOOST_CHECK(std::find(strings.begin(), strings.end(), "forcerelay") !=
Expand Down
2 changes: 1 addition & 1 deletion src/test/random_tests.cpp
View file
Expand Up @@ -137,7 +137,7 @@ BOOST_AUTO_TEST_CASE(shuffle_stat_test) {
}
BOOST_CHECK(chi_score > 58.1411); // 99.9999% confidence interval
BOOST_CHECK(chi_score < 210.275);
BOOST_CHECK_EQUAL(sum, 12000);
BOOST_CHECK_EQUAL(sum, 12000U);
}

BOOST_AUTO_TEST_SUITE_END()
44 changes: 22 additions & 22 deletions src/test/streams_tests.cpp
View file
Expand Up @@ -72,29 +72,29 @@ BOOST_AUTO_TEST_CASE(streams_vector_reader) {
std::vector<uint8_t> vch = {1, 255, 3, 4, 5, 6};

VectorReader reader(SER_NETWORK, INIT_PROTO_VERSION, vch, 0);
BOOST_CHECK_EQUAL(reader.size(), 6);
BOOST_CHECK_EQUAL(reader.size(), 6U);
BOOST_CHECK(!reader.empty());

// Read a single byte as an uint8_t.
uint8_t a;
reader >> a;
BOOST_CHECK_EQUAL(a, 1);
BOOST_CHECK_EQUAL(reader.size(), 5);
BOOST_CHECK_EQUAL(reader.size(), 5U);
BOOST_CHECK(!reader.empty());

// Read a single byte as a (signed) int8_t.
int8_t b;
reader >> b;
BOOST_CHECK_EQUAL(b, -1);
BOOST_CHECK_EQUAL(reader.size(), 4);
BOOST_CHECK_EQUAL(reader.size(), 4U);
BOOST_CHECK(!reader.empty());

// Read a 4 bytes as an unsigned uint32_t.
uint32_t c;
reader >> c;
// 100992003 = 3,4,5,6 in little-endian base-256
BOOST_CHECK_EQUAL(c, 100992003);
BOOST_CHECK_EQUAL(reader.size(), 0);
BOOST_CHECK_EQUAL(reader.size(), 0U);
BOOST_CHECK(reader.empty());

// Reading after end of byte vector throws an error.
Expand All @@ -106,7 +106,7 @@ BOOST_AUTO_TEST_CASE(streams_vector_reader) {
new_reader >> d;
// 67370753 = 1,255,3,4 in little-endian base-256
BOOST_CHECK_EQUAL(d, 67370753);
BOOST_CHECK_EQUAL(new_reader.size(), 2);
BOOST_CHECK_EQUAL(new_reader.size(), 2U);
BOOST_CHECK(!new_reader.empty());

// Reading after end of byte vector throws an error even if the reader is
Expand Down Expand Up @@ -139,14 +139,14 @@ BOOST_AUTO_TEST_CASE(bitstream_reader_writer) {
BOOST_CHECK_EQUAL(serialized_int2, (uint16_t)0x1072);

BitStreamReader<CDataStream> bit_reader(data_copy);
BOOST_CHECK_EQUAL(bit_reader.Read(1), 0);
BOOST_CHECK_EQUAL(bit_reader.Read(2), 2);
BOOST_CHECK_EQUAL(bit_reader.Read(3), 6);
BOOST_CHECK_EQUAL(bit_reader.Read(4), 11);
BOOST_CHECK_EQUAL(bit_reader.Read(5), 1);
BOOST_CHECK_EQUAL(bit_reader.Read(6), 32);
BOOST_CHECK_EQUAL(bit_reader.Read(7), 7);
BOOST_CHECK_EQUAL(bit_reader.Read(16), 30497);
BOOST_CHECK_EQUAL(bit_reader.Read(1), 0U);
BOOST_CHECK_EQUAL(bit_reader.Read(2), 2U);
BOOST_CHECK_EQUAL(bit_reader.Read(3), 6U);
BOOST_CHECK_EQUAL(bit_reader.Read(4), 11U);
BOOST_CHECK_EQUAL(bit_reader.Read(5), 1U);
BOOST_CHECK_EQUAL(bit_reader.Read(6), 32U);
BOOST_CHECK_EQUAL(bit_reader.Read(7), 7U);
BOOST_CHECK_EQUAL(bit_reader.Read(16), 30497U);
BOOST_CHECK_THROW(bit_reader.Read(8), std::ios_base::failure);
}

Expand Down Expand Up @@ -253,7 +253,7 @@ BOOST_AUTO_TEST_CASE(streams_buffered_file) {
BOOST_CHECK_EQUAL(i, 1);

// After reading bytes 0 and 1, we're positioned at 2.
BOOST_CHECK_EQUAL(bf.GetPos(), 2);
BOOST_CHECK_EQUAL(bf.GetPos(), 2U);

// Rewind to offset 0, ok (within the 10 byte window).
BOOST_CHECK(bf.SetPos(0));
Expand All @@ -280,18 +280,18 @@ BOOST_AUTO_TEST_CASE(streams_buffered_file) {
// The default argument removes the limit completely.
BOOST_CHECK(bf.SetLimit());
// The read position should still be at 3 (no change).
BOOST_CHECK_EQUAL(bf.GetPos(), 3);
BOOST_CHECK_EQUAL(bf.GetPos(), 3U);

// Read from current offset, 3, forward until position 10.
for (uint8_t j = 3; j < 10; ++j) {
bf >> i;
BOOST_CHECK_EQUAL(i, j);
}
BOOST_CHECK_EQUAL(bf.GetPos(), 10);
BOOST_CHECK_EQUAL(bf.GetPos(), 10U);

// We're guaranteed (just barely) to be able to rewind to zero.
BOOST_CHECK(bf.SetPos(0));
BOOST_CHECK_EQUAL(bf.GetPos(), 0);
BOOST_CHECK_EQUAL(bf.GetPos(), 0U);
bf >> i;
BOOST_CHECK_EQUAL(i, 0);

Expand All @@ -301,12 +301,12 @@ BOOST_AUTO_TEST_CASE(streams_buffered_file) {
BOOST_CHECK(bf.SetPos(10));
bf >> i;
BOOST_CHECK_EQUAL(i, 10);
BOOST_CHECK_EQUAL(bf.GetPos(), 11);
BOOST_CHECK_EQUAL(bf.GetPos(), 11U);

// Now it's only guaranteed that we can rewind to offset 1
// (current read position, 11, minus rewind amount, 10).
BOOST_CHECK(bf.SetPos(1));
BOOST_CHECK_EQUAL(bf.GetPos(), 1);
BOOST_CHECK_EQUAL(bf.GetPos(), 1U);
bf >> i;
BOOST_CHECK_EQUAL(i, 1);

Expand All @@ -320,7 +320,7 @@ BOOST_AUTO_TEST_CASE(streams_buffered_file) {
BOOST_CHECK_EQUAL(a[j], 11 + j);
}
}
BOOST_CHECK_EQUAL(bf.GetPos(), 40);
BOOST_CHECK_EQUAL(bf.GetPos(), 40U);

// We've read the entire file, the next read should throw.
try {
Expand All @@ -334,11 +334,11 @@ BOOST_AUTO_TEST_CASE(streams_buffered_file) {
BOOST_CHECK(bf.eof());

// Still at offset 40, we can go back 10, to 30.
BOOST_CHECK_EQUAL(bf.GetPos(), 40);
BOOST_CHECK_EQUAL(bf.GetPos(), 40U);
BOOST_CHECK(bf.SetPos(30));
bf >> i;
BOOST_CHECK_EQUAL(i, 30);
BOOST_CHECK_EQUAL(bf.GetPos(), 31);
BOOST_CHECK_EQUAL(bf.GetPos(), 31U);

// We're too far to rewind to position zero.
BOOST_CHECK(!bf.SetPos(0));
Expand Down

0 comments on commit 942d919

Please sign in to comment.