Handle special case in trim_blocklog_front #10370
Conversation
| uint64_t complete_pos_content; | ||
| bool re_write = false; | ||
| uint64_t re_write_offset; | ||
|
|
||
| // walk this memory section to adjust block position to match the adjusted location | ||
| // of the block start and store in the new index file | ||
| while(original_pos >= start_of_blk_buffer_pos) { |
There was a problem hiding this comment.
The loop can be simplified to work within block boundaries only, for those doesn't fit into block boundaries, it should be re-read from the outer for-loop.
There was a problem hiding this comment.
Simplified the while loop. Will make another update if it's not the way you expected. Thanks!
libraries/chain/block_log.cpp
Outdated
| index.write(pos_content); | ||
| original_pos = start_of_this_block - pos_size; | ||
| if (buffer_index + pos_size <= detail::reverse_iterator::_buf_len){ | ||
| uint64_t& pos_content = *(uint64_t*)(buf + buffer_index); |
There was a problem hiding this comment.
please use memcpy() instead
There was a problem hiding this comment.
Since reference var pos_content is defined for both reading and writing, I replaced this line with two methods read_buffer and write_buffer.
| @@ -81,6 +81,9 @@ namespace eosio { namespace chain { | |||
|
|
|||
| static bool trim_blocklog_front(const fc::path& block_dir, const fc::path& temp_dir, uint32_t truncate_at_block); | |||
|
|
|||
| // used only for unit test to adjust the buffer length | |||
| static void set_buff_len(uint64_t len); | |||
There was a problem hiding this comment.
This shouldn't be in the public interface. Just implement it in block_log.cpp as a regular function and forward declare it inunittests/restart_chain_tests.cpp.
| } | ||
|
|
||
| BOOST_CHECK( block_log::trim_blocklog_front(temp1.path, temp2.path, truncate_at_block) == true); | ||
| } |
There was a problem hiding this comment.
Could you follow the example in
eos/unittests/restart_chain_tests.cpp
Lines 651 to 658 in ce50319
libraries/chain/block_log.cpp
Outdated
| @@ -1185,3 +1211,8 @@ namespace eosio { namespace chain { | |||
| } | |||
|
|
|||
| } } /// eosio::chain | |||
|
|
|||
| // used only for unit test to adjust the buffer length | |||
| void set_buff_len(uint64_t len){ | |||
There was a problem hiding this comment.
The name of the function is too generic without any namespace. Consider to use block_log_set_buff_len.
Change Description
EPE-895 - GitHub Issue]( #8948)
This line
uint64_t& pos_content = *(uint64_t*)(buf + buffer_index);in methodtrim_blocklog_front() can cause bad memory access exception when there are less than 8 bytes right after addressbuf + buffer_indexin thebuffer, which is defined asauto buffer = make_unique<char[]>(detail::reverse_iterator::_buf_len);Change Type
Select ONE:
Testing Changes
Select ANY that apply:
Consensus Changes
API Changes
Documentation Additions