Skip to content

Commit 5f26210

Browse files
committed
refactor: Use an enum for Autofile::seek wrapper
The `int origin` argument of the `seek` method has no meaning to those without domain specific knowledge as to how `FILE` works. The only way for a caller to understand what this is doing is to enter the method body. This enum makes it clear as to what the `origin` argument represents at the expense of the Unix-specific options, which were unused anyway.
1 parent 2210feb commit 5f26210

8 files changed

Lines changed: 31 additions & 20 deletions

File tree

src/bench/streams_findbyte.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ static void FindByte(benchmark::Bench& bench)
1919
uint8_t data[file_size] = {0};
2020
data[file_size - 1] = 1;
2121
file << data;
22-
file.seek(0, SEEK_SET);
22+
file.seek(0, SeekFrom::Set);
2323
BufferedFile bf{file, /*nBufSize=*/file_size + 1, /*nRewindIn=*/file_size};
2424

2525
bench.run([&] {

src/index/txindex.cpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@
2222

2323
#include <cassert>
2424
#include <cstdint>
25-
#include <cstdio>
2625
#include <exception>
2726
#include <iterator>
2827
#include <span>
@@ -107,7 +106,7 @@ bool TxIndex::FindTx(const Txid& tx_hash, uint256& block_hash, CTransactionRef&
107106
CBlockHeader header;
108107
try {
109108
file >> header;
110-
file.seek(postx.nTxOffset, SEEK_CUR);
109+
file.seek(postx.nTxOffset, SeekFrom::Curr);
111110
file >> TX_WITH_WITNESS(tx);
112111
} catch (const std::exception& e) {
113112
LogError("Deserialize or I/O error - %s", e.what());

src/node/blockstorage.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1086,7 +1086,7 @@ BlockManager::ReadRawBlockResult BlockManager::ReadRawBlock(const FlatFilePos& p
10861086
if (size == 0 || offset >= blk_size || size > blk_size - offset) {
10871087
return util::Unexpected{ReadRawError::BadPartRange}; // Avoid logging - offset/size come from untrusted REST input
10881088
}
1089-
filein.seek(offset, SEEK_CUR);
1089+
filein.seek(offset, SeekFrom::Curr);
10901090
blk_size = size;
10911091
}
10921092

src/node/utxo_snapshot.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ std::optional<uint256> ReadSnapshotBaseBlockhash(fs::path chaindir)
7474
afile >> base_blockhash;
7575

7676
int64_t position = afile.tell();
77-
afile.seek(0, SEEK_END);
77+
afile.seek(0, SeekFrom::End);
7878
if (position != afile.tell()) {
7979
LogWarning("[snapshot] unexpected trailing data in %s", read_from_str);
8080
}

src/streams.cpp

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -30,17 +30,17 @@ std::size_t AutoFile::detail_fread(std::span<std::byte> dst)
3030
return ret;
3131
}
3232

33-
void AutoFile::seek(int64_t offset, int origin)
33+
void AutoFile::seek(int64_t offset, SeekFrom origin)
3434
{
3535
if (IsNull()) {
3636
throw std::ios_base::failure("AutoFile::seek: file handle is nullptr");
3737
}
3838
if (std::fseek(m_file, offset, origin) != 0) {
3939
throw std::ios_base::failure(feof() ? "AutoFile::seek: end of file" : "AutoFile::seek: fseek failed");
4040
}
41-
if (origin == SEEK_SET) {
41+
if (origin == SeekFrom::Set) {
4242
m_position = offset;
43-
} else if (origin == SEEK_CUR && m_position.has_value()) {
43+
} else if (origin == SeekFrom::Curr && m_position.has_value()) {
4444
*m_position += offset;
4545
} else {
4646
int64_t r{std::ftell(m_file)};
@@ -64,10 +64,10 @@ int64_t AutoFile::size()
6464
}
6565
// Temporarily save the current position
6666
int64_t current_pos = tell();
67-
seek(0, SEEK_END);
67+
seek(0, SeekFrom::End);
6868
int64_t file_size = tell();
6969
// Restore the original position
70-
seek(current_pos, SEEK_SET);
70+
seek(current_pos, SeekFrom::Set);
7171
return file_size;
7272
}
7373

src/streams.h

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -355,6 +355,18 @@ class BitStreamWriter
355355
}
356356
};
357357

358+
/**
359+
* Relative position to move the file cursor.
360+
*/
361+
enum SeekFrom {
362+
/* Start of the file. */
363+
Set = 0,
364+
/* Current position. */
365+
Curr,
366+
/* End of the file. */
367+
End
368+
};
369+
358370
/** Non-refcounted RAII wrapper for FILE*
359371
*
360372
* Will automatically close the file when it goes out of scope if not null.
@@ -430,7 +442,7 @@ class AutoFile
430442
std::size_t detail_fread(std::span<std::byte> dst);
431443

432444
/** Wrapper around fseek(). Will throw if seeking is not possible. */
433-
void seek(int64_t offset, int origin);
445+
void seek(int64_t offset, SeekFrom origin);
434446

435447
/** Find position within the file. Will throw if unknown. */
436448
int64_t tell();

src/test/streams_tests.cpp

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -333,7 +333,7 @@ BOOST_AUTO_TEST_CASE(streams_buffered_file)
333333
for (uint8_t j = 0; j < 40; ++j) {
334334
file << j;
335335
}
336-
file.seek(0, SEEK_SET);
336+
file.seek(0, SeekFrom::Set);
337337

338338
// The buffer size (second arg) must be greater than the rewind
339339
// amount (third arg).
@@ -462,7 +462,7 @@ BOOST_AUTO_TEST_CASE(streams_buffered_file_skip)
462462
for (uint8_t j = 0; j < 40; ++j) {
463463
file << j;
464464
}
465-
file.seek(0, SEEK_SET);
465+
file.seek(0, SeekFrom::Set);
466466

467467
// The buffer is 25 bytes, allow rewinding 10 bytes.
468468
BufferedFile bf{file, 25, 10};
@@ -515,7 +515,7 @@ BOOST_AUTO_TEST_CASE(streams_buffered_file_rand)
515515
for (uint8_t i = 0; i < fileSize; ++i) {
516516
file << i;
517517
}
518-
file.seek(0, SEEK_SET);
518+
file.seek(0, SeekFrom::Set);
519519

520520
size_t bufSize = m_rng.randrange(300) + 1;
521521
size_t rewindSize = m_rng.randrange(bufSize);
@@ -797,14 +797,14 @@ BOOST_AUTO_TEST_CASE(size_preserves_position)
797797
// Test that usage of size() does not change the current position
798798
//
799799
// Case: Pos at beginning of the file
800-
f.seek(0, SEEK_SET);
800+
f.seek(0, SeekFrom::Set);
801801
(void)f.size();
802802
uint8_t first{};
803803
f >> first;
804804
BOOST_CHECK_EQUAL(first, 0);
805805

806806
// Case: Pos at middle of the file
807-
f.seek(0, SEEK_SET);
807+
f.seek(0, SeekFrom::Set);
808808
// Move pos to middle
809809
f.ignore(4);
810810
(void)f.size();
@@ -814,7 +814,7 @@ BOOST_AUTO_TEST_CASE(size_preserves_position)
814814
BOOST_CHECK_EQUAL(middle, 4);
815815

816816
// Case: Pos at EOF
817-
f.seek(0, SEEK_END);
817+
f.seek(0, SeekFrom::End);
818818
(void)f.size();
819819
uint8_t end{};
820820
BOOST_CHECK_EXCEPTION(f >> end, std::ios_base::failure, HasReason{"AutoFile::read: end of file"});

src/wallet/migrate.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -436,7 +436,7 @@ class RecordsPage
436436
}
437437

438438
// Go back to the indexes
439-
s.seek(-to_jump, SEEK_CUR);
439+
s.seek(-to_jump, SeekFrom::Curr);
440440
}
441441
}
442442
};
@@ -515,15 +515,15 @@ class InternalPage
515515
to_jump += InternalRecord::FIXED_SIZE + rec_hdr.len;
516516

517517
// Go back to the indexes
518-
s.seek(-to_jump, SEEK_CUR);
518+
s.seek(-to_jump, SeekFrom::Curr);
519519
}
520520
}
521521
};
522522

523523
static void SeekToPage(AutoFile& s, uint32_t page_num, uint32_t page_size)
524524
{
525525
int64_t pos = int64_t{page_num} * page_size;
526-
s.seek(pos, SEEK_SET);
526+
s.seek(pos, SeekFrom::Set);
527527
}
528528

529529
void BerkeleyRODatabase::Open()

0 commit comments

Comments
 (0)