diff --git a/src/storage/v2/durability.cpp b/src/storage/v2/durability.cpp
index aa69e7707..8744371e6 100644
--- a/src/storage/v2/durability.cpp
+++ b/src/storage/v2/durability.cpp
@@ -132,7 +132,7 @@ void Encoder::Finalize() {
std::optional<uint64_t> Decoder::Initialize(const std::filesystem::path &path,
const std::string &magic) {
- file_.Open(path);
+ if (!file_.Open(path)) return std::nullopt;
std::string file_magic(magic.size(), '\0');
if (!Read(reinterpret_cast<uint8_t *>(file_magic.data()), file_magic.size()))
return std::nullopt;
@@ -366,12 +366,12 @@ bool Decoder::SkipPropertyValue() {
}
}
-uint64_t Decoder::GetSize() { return file_.GetSize(); }
+std::optional<uint64_t> Decoder::GetSize() { return file_.GetSize(); }
-uint64_t Decoder::GetPosition() { return file_.GetPosition(); }
+std::optional<uint64_t> Decoder::GetPosition() { return file_.GetPosition(); }
-void Decoder::SetPosition(uint64_t position) {
- file_.SetPosition(utils::InputFile::Position::SET, position);
+bool Decoder::SetPosition(uint64_t position) {
+ return !!file_.SetPosition(utils::InputFile::Position::SET, position);
}
} // namespace storage::durability
diff --git a/src/storage/v2/durability.hpp b/src/storage/v2/durability.hpp
index 6b65c39e5..1d27eb4d6 100644
--- a/src/storage/v2/durability.hpp
+++ b/src/storage/v2/durability.hpp
@@ -102,9 +102,9 @@ class Decoder final {
bool SkipString();
bool SkipPropertyValue();
- uint64_t GetSize();
- uint64_t GetPosition();
- void SetPosition(uint64_t position);
+ std::optional<uint64_t> GetSize();
+ std::optional<uint64_t> GetPosition();
+ bool SetPosition(uint64_t position);
private:
utils::InputFile file_;
diff --git a/src/utils/file.cpp b/src/utils/file.cpp
index 86892f3fa..1eb72cf01 100644
--- a/src/utils/file.cpp
+++ b/src/utils/file.cpp
@@ -63,9 +63,7 @@ bool CopyFile(const std::filesystem::path &src,
static_assert(std::is_same_v<off_t, ssize_t>, "off_t must fit into ssize_t!");
-InputFile::~InputFile() {
- if (IsOpen()) Close();
-}
+InputFile::~InputFile() { Close(); }
InputFile::InputFile(InputFile &&other) noexcept
: fd_(other.fd_), path_(std::move(other.path_)) {
@@ -73,7 +71,7 @@ InputFile::InputFile(InputFile &&other) noexcept
}
InputFile &InputFile::operator=(InputFile &&other) noexcept {
- if (IsOpen()) Close();
+ Close();
fd_ = other.fd_;
path_ = std::move(other.path_);
@@ -83,11 +81,8 @@ InputFile &InputFile::operator=(InputFile &&other) noexcept {
return *this;
}
-void InputFile::Open(const std::filesystem::path &path) {
- CHECK(!IsOpen())
- << "While trying to open " << path
- << " for writing the database used a handle that already has " << path_
- << " opened in it!";
+bool InputFile::Open(const std::filesystem::path &path) {
+ if (IsOpen()) return false;
path_ = path;
@@ -97,15 +92,13 @@ void InputFile::Open(const std::filesystem::path &path) {
// The call was interrupted, try again...
continue;
} else {
- // All other possible errors are fatal errors and are handled in the CHECK
- // below.
+ // All other possible errors are fatal errors and are handled with the
+ // return value.
break;
}
}
- CHECK(fd_ != -1) << "While trying to open " << path_
- << " for reading an error occurred: " << strerror(errno)
- << " (" << errno << ").";
+ return fd_ != -1;
}
bool InputFile::IsOpen() const { return fd_ != -1; }
@@ -154,18 +147,21 @@ bool InputFile::Peek(uint8_t *data, size_t size) {
return true;
}
-size_t InputFile::GetSize() {
- size_t current = GetPosition();
- size_t size = SetPosition(Position::RELATIVE_TO_END, 0);
- SetPosition(Position::SET, current);
+std::optional<size_t> InputFile::GetSize() {
+ auto current = GetPosition();
+ if (!current) return std::nullopt;
+ auto size = SetPosition(Position::RELATIVE_TO_END, 0);
+ if (!size) return std::nullopt;
+ if (!SetPosition(Position::SET, *current)) return std::nullopt;
return size;
}
-size_t InputFile::GetPosition() {
+std::optional<size_t> InputFile::GetPosition() {
return SetPosition(Position::RELATIVE_TO_CURRENT, 0);
}
-size_t InputFile::SetPosition(Position position, ssize_t offset) {
+std::optional<size_t> InputFile::SetPosition(Position position,
+ ssize_t offset) {
int whence;
switch (position) {
case Position::SET:
@@ -183,14 +179,14 @@ size_t InputFile::SetPosition(Position position, ssize_t offset) {
if (pos == -1 && errno == EINTR) {
continue;
}
- CHECK(pos >= 0) << "While trying to set the position in " << path_
- << " an error occurred: " << strerror(errno) << " ("
- << errno << ").";
+ if (pos < 0) return std::nullopt;
return pos;
}
}
void InputFile::Close() noexcept {
+ if (!IsOpen()) return;
+
int ret = 0;
while (true) {
ret = close(fd_);
@@ -204,9 +200,11 @@ void InputFile::Close() noexcept {
}
}
- CHECK(ret == 0) << "While trying to close " << path_
- << " an error occurred: " << strerror(errno) << " (" << errno
- << ").";
+ if (ret != 0) {
+ LOG(ERROR) << "While trying to close " << path_
+ << " an error occurred: " << strerror(errno) << " (" << errno
+ << ").";
+ }
fd_ = -1;
}
diff --git a/src/utils/file.hpp b/src/utils/file.hpp
index a6fed5131..a6b1b743e 100644
--- a/src/utils/file.hpp
+++ b/src/utils/file.hpp
@@ -7,6 +7,7 @@
#pragma once
#include <filesystem>
+#include <optional>
#include <string>
#include <string_view>
#include <vector>
@@ -63,8 +64,8 @@ class InputFile {
InputFile &operator=(InputFile &&other) noexcept;
/// This method opens the file used for reading. If the file can't be opened
- /// or doesn't exist it crashes the program.
- void Open(const std::filesystem::path &path);
+ /// or doesn't exist it returns `false`.
+ bool Open(const std::filesystem::path &path);
/// Returns a boolean indicating whether a file is opened.
bool IsOpen() const;
@@ -83,22 +84,21 @@ class InputFile {
/// doesn't change the current position in the file.
bool Peek(uint8_t *data, size_t size);
- /// This method gets the size of the file. On failure and misuse it crashes
- /// the program.
- size_t GetSize();
+ /// This method gets the size of the file. On failure it returns
+ /// `std::nullopt`.
+ std::optional<size_t> GetSize();
- /// This method gets the current absolute position in the file. On failure and
- /// misuse it crashes the program.
- size_t GetPosition();
+ /// This method gets the current absolute position in the file. On failure it
+ /// returns `std::nullopt`.
+ std::optional<size_t> GetPosition();
/// This method sets the current position in the file and returns the absolute
/// set position in the file. The position is set to `offset` with the
- /// starting point taken from `position`. On failure and misuse it crashes the
- /// program.
- size_t SetPosition(Position position, ssize_t offset);
+ /// starting point taken from `position`. On failure it returns
+ /// `std::nullopt`.
+ std::optional<size_t> SetPosition(Position position, ssize_t offset);
- /// Closes the currently opened file. On failure and misuse it crashes the
- /// program.
+ /// Closes the currently opened file. On failure it crashes the program.
void Close() noexcept;
private:
diff --git a/tests/unit/storage_v2_decoder_encoder.cpp b/tests/unit/storage_v2_decoder_encoder.cpp
index 26362e765..b73fb5c59 100644
--- a/tests/unit/storage_v2_decoder_encoder.cpp
+++ b/tests/unit/storage_v2_decoder_encoder.cpp
@@ -59,7 +59,9 @@ TEST_F(DecoderEncoderTest, ReadMarker) {
}
ASSERT_FALSE(decoder.ReadMarker());
ASSERT_FALSE(decoder.ReadMarker());
- ASSERT_EQ(decoder.GetPosition(), decoder.GetSize());
+ auto pos = decoder.GetPosition();
+ ASSERT_TRUE(pos);
+ ASSERT_EQ(pos, decoder.GetSize());
}
}
@@ -91,7 +93,9 @@ TEST_F(DecoderEncoderTest, ReadMarker) {
} \
ASSERT_FALSE(decoder.Read##name()); \
ASSERT_FALSE(decoder.Read##name()); \
- ASSERT_EQ(decoder.GetPosition(), decoder.GetSize()); \
+ auto pos = decoder.GetPosition(); \
+ ASSERT_TRUE(pos); \
+ ASSERT_EQ(pos, decoder.GetSize()); \
} \
}
@@ -148,7 +152,9 @@ GENERATE_READ_TEST(
} \
ASSERT_FALSE(decoder.Skip##name()); \
ASSERT_FALSE(decoder.Skip##name()); \
- ASSERT_EQ(decoder.GetPosition(), decoder.GetSize()); \
+ auto pos = decoder.GetPosition(); \
+ ASSERT_TRUE(pos); \
+ ASSERT_EQ(pos, decoder.GetSize()); \
} \
}
@@ -179,7 +185,7 @@ GENERATE_SKIP_TEST(
{ \
utils::InputFile ifile; \
utils::OutputFile ofile; \
- ifile.Open(storage_file); \
+ ASSERT_TRUE(ifile.Open(storage_file)); \
ofile.Open(alternate_file, utils::OutputFile::Mode::OVERWRITE_EXISTING); \
auto size = ifile.GetSize(); \
for (size_t i = 0; i <= size; ++i) { \
@@ -245,7 +251,7 @@ GENERATE_PARTIAL_READ_TEST(
{ \
utils::InputFile ifile; \
utils::OutputFile ofile; \
- ifile.Open(storage_file); \
+ ASSERT_TRUE(ifile.Open(storage_file)); \
ofile.Open(alternate_file, utils::OutputFile::Mode::OVERWRITE_EXISTING); \
auto size = ifile.GetSize(); \
for (size_t i = 0; i <= size; ++i) { \
@@ -388,11 +394,14 @@ TEST_F(DecoderEncoderTest, DecoderPosition) {
ASSERT_TRUE(version);
ASSERT_EQ(*version, kTestVersion);
for (int i = 0; i < 10; ++i) {
- decoder.SetPosition(kTestMagic.size() + sizeof(kTestVersion));
+ ASSERT_TRUE(
+ decoder.SetPosition(kTestMagic.size() + sizeof(kTestVersion)));
auto decoded = decoder.ReadBool();
ASSERT_TRUE(decoded);
ASSERT_TRUE(*decoded);
- ASSERT_EQ(decoder.GetPosition(), decoder.GetSize());
+ auto pos = decoder.GetPosition();
+ ASSERT_TRUE(pos);
+ ASSERT_EQ(pos, decoder.GetSize());
}
}
}
@@ -416,6 +425,8 @@ TEST_F(DecoderEncoderTest, EncoderPosition) {
auto decoded = decoder.ReadBool();
ASSERT_TRUE(decoded);
ASSERT_TRUE(*decoded);
- ASSERT_EQ(decoder.GetPosition(), decoder.GetSize());
+ auto pos = decoder.GetPosition();
+ ASSERT_TRUE(pos);
+ ASSERT_EQ(pos, decoder.GetSize());
}
}