fix overflow in calc_bytes(), add more tests, fix bug in piece picker accounting of filtered pad blocks.

This commit is contained in:
arvidn 2018-09-06 19:09:09 -07:00 committed by Arvid Norberg
parent fc7b61a6f3
commit 16249b8135
4 changed files with 115 additions and 8 deletions

View File

@ -3526,6 +3526,12 @@ get_out:
++m_pads_in_piece[block.piece_index]; ++m_pads_in_piece[block.piece_index];
piece_pos& p = m_piece_map[block.piece_index];
if (p.filtered())
{
++m_filtered_pad_blocks;
}
// if we mark and entire piece as a pad file, we need to also // if we mark and entire piece as a pad file, we need to also
// consder that piece as "had" and increment some counters // consder that piece as "had" and increment some counters
int const blocks = blocks_in_piece(block.piece_index); int const blocks = blocks_in_piece(block.piece_index);
@ -3664,33 +3670,49 @@ get_out:
piece_count piece_picker::want() const piece_count piece_picker::want() const
{ {
bool const want_last = piece_priority(piece_index_t(num_pieces() - 1)) != dont_download; bool const want_last = piece_priority(piece_index_t(num_pieces() - 1)) != dont_download;
return { num_pieces() - m_num_filtered - m_num_have_filtered piece_count ret{ num_pieces() - m_num_filtered - m_num_have_filtered
, num_pad_blocks() - m_filtered_pad_blocks - m_have_filtered_pad_blocks , num_pad_blocks() - m_filtered_pad_blocks - m_have_filtered_pad_blocks
, want_last }; , want_last };
TORRENT_ASSERT(!(ret.num_pieces == 0 && ret.last_piece == true));
TORRENT_ASSERT(!(ret.num_pieces == 0 && ret.pad_blocks > 0));
TORRENT_ASSERT(!(ret.num_pieces == num_pieces() && ret.last_piece == false));
return ret;
} }
piece_count piece_picker::have_want() const piece_count piece_picker::have_want() const
{ {
bool const have_last = have_piece(piece_index_t(num_pieces() - 1)); bool const have_last = have_piece(piece_index_t(num_pieces() - 1));
bool const want_last = piece_priority(piece_index_t(num_pieces() - 1)) != dont_download; bool const want_last = piece_priority(piece_index_t(num_pieces() - 1)) != dont_download;
return { m_num_have - m_num_have_filtered piece_count ret{ m_num_have - m_num_have_filtered
, m_have_pad_blocks - m_have_filtered_pad_blocks , m_have_pad_blocks - m_have_filtered_pad_blocks
, have_last && want_last }; , have_last && want_last };
TORRENT_ASSERT(!(ret.num_pieces == 0 && ret.last_piece == true));
TORRENT_ASSERT(!(ret.num_pieces == 0 && ret.pad_blocks > 0));
TORRENT_ASSERT(!(ret.num_pieces == num_pieces() && ret.last_piece == false));
return ret;
} }
piece_count piece_picker::have() const piece_count piece_picker::have() const
{ {
bool const have_last = have_piece(piece_index_t(num_pieces() - 1)); bool const have_last = have_piece(piece_index_t(num_pieces() - 1));
return { m_num_have piece_count ret{ m_num_have
, m_have_pad_blocks , m_have_pad_blocks
, have_last }; , have_last };
TORRENT_ASSERT(!(ret.num_pieces == 0 && ret.last_piece == true));
TORRENT_ASSERT(!(ret.num_pieces == 0 && ret.pad_blocks > 0));
TORRENT_ASSERT(!(ret.num_pieces == num_pieces() && ret.last_piece == false));
return ret;
} }
piece_count piece_picker::all_pieces() const piece_count piece_picker::all_pieces() const
{ {
return { num_pieces() piece_count ret{ num_pieces()
, num_pad_blocks() , num_pad_blocks()
, true}; , true};
TORRENT_ASSERT(!(ret.num_pieces == 0 && ret.last_piece == true));
TORRENT_ASSERT(!(ret.num_pieces == 0 && ret.pad_blocks > 0));
TORRENT_ASSERT(!(ret.num_pieces == num_pieces() && ret.last_piece == false));
return ret;
} }
} }

View File

@ -3478,10 +3478,10 @@ bool is_downloading_state(int const st)
int const block_size = std::min(default_block_size, fs.piece_length()); int const block_size = std::min(default_block_size, fs.piece_length());
// every block should not be a pad block // every block should not be a pad block
TORRENT_ASSERT(pc.pad_blocks <= pc.num_pieces * fs.piece_length() / block_size); TORRENT_ASSERT(pc.pad_blocks <= std::int64_t(pc.num_pieces) * fs.piece_length() / block_size);
return std::int64_t(pc.num_pieces) * fs.piece_length() return std::int64_t(pc.num_pieces) * fs.piece_length()
- (pc.last_piece ? 1 : 0) * (fs.piece_length() - fs.piece_size(fs.last_piece())) - (pc.last_piece ? fs.piece_length() - fs.piece_size(fs.last_piece()) : 0)
- std::int64_t(pc.pad_blocks) * block_size; - std::int64_t(pc.pad_blocks) * block_size;
} }

View File

@ -2061,5 +2061,90 @@ TORRENT_TEST(pad_blocks_in_last_piece)
TEST_EQUAL(p->pad_blocks_in_piece(piece_index_t{0}), 0); TEST_EQUAL(p->pad_blocks_in_piece(piece_index_t{0}), 0);
} }
namespace {
void validate_piece_count(piece_count const& c)
{
// it's an impossible combination to have 0 pieces, but still have one of them be the last piece
TEST_CHECK(!(c.num_pieces == 0 && c.last_piece == true));
// if we have 0 pieces, we can't have any pad blocks either
TEST_CHECK(!(c.num_pieces == 0 && c.pad_blocks > 0));
// if we have all pieces, we must also have the last one
TEST_CHECK(!(c.num_pieces == 4 && c.last_piece == false));
}
void validate_all_pieces(piece_count const& c)
{
TEST_EQUAL(c.last_piece, true);
TEST_EQUAL(c.num_pieces, 4);
TEST_EQUAL(c.pad_blocks, 3);
}
void validate_no_pieces(piece_count const& c)
{
TEST_EQUAL(c.last_piece, false);
TEST_EQUAL(c.num_pieces, 0);
TEST_EQUAL(c.pad_blocks, 0);
}
}
TORRENT_TEST(pad_blocks_all_filtered)
{
auto p = setup_picker("1111", " ", "0000", "");
p->mark_as_pad({piece_index_t{1}, 0});
p->mark_as_pad({piece_index_t{1}, 1});
p->mark_as_pad({piece_index_t{2}, 0});
validate_piece_count(p->all_pieces());
validate_piece_count(p->have());
validate_piece_count(p->have_want());
validate_piece_count(p->want());
validate_all_pieces(p->all_pieces());
validate_no_pieces(p->have());
validate_no_pieces(p->have_want());
validate_no_pieces(p->want());
}
TORRENT_TEST(pad_blocks_all_wanted)
{
auto p = setup_picker("1111", " ", "4444", "");
p->mark_as_pad({piece_index_t{1}, 0});
p->mark_as_pad({piece_index_t{1}, 1});
p->mark_as_pad({piece_index_t{2}, 0});
validate_piece_count(p->all_pieces());
validate_piece_count(p->have());
validate_piece_count(p->have_want());
validate_piece_count(p->want());
validate_all_pieces(p->all_pieces());
validate_all_pieces(p->want());
validate_no_pieces(p->have());
validate_no_pieces(p->have_want());
}
TORRENT_TEST(pad_blocks_some_wanted)
{
auto p = setup_picker("1111", " ", "0404", "");
p->mark_as_pad({piece_index_t{1}, 0});
p->mark_as_pad({piece_index_t{1}, 1});
p->mark_as_pad({piece_index_t{2}, 0});
validate_piece_count(p->all_pieces());
validate_piece_count(p->have());
validate_piece_count(p->have_want());
validate_piece_count(p->want());
validate_all_pieces(p->all_pieces());
validate_no_pieces(p->have());
validate_no_pieces(p->have_want());
TEST_EQUAL(p->want().num_pieces, 2);
TEST_EQUAL(p->want().last_piece, true);
TEST_EQUAL(p->want().pad_blocks, 2);
}
//TODO: 2 test picking with partial pieces and other peers present so that both //TODO: 2 test picking with partial pieces and other peers present so that both
// backup_pieces and backup_pieces2 are used // backup_pieces and backup_pieces2 are used

View File

@ -694,13 +694,13 @@ TORRENT_TEST(test_read_piece_out_of_range)
} }
namespace { namespace {
int const piece_size = 0x4000 * 2; int const piece_size = 0x4000 * 128;
file_storage test_fs() file_storage test_fs()
{ {
file_storage fs; file_storage fs;
fs.set_piece_length(piece_size); fs.set_piece_length(piece_size);
fs.add_file("temp", 999999); fs.add_file("temp", 99999999999);
fs.set_num_pieces(int((fs.total_size() + piece_size - 1) / piece_size)); fs.set_num_pieces(int((fs.total_size() + piece_size - 1) / piece_size));
return fs; return fs;
} }