Skip to content

Commit

Permalink
Refactor LfsWrapper.[ch]pp
Browse files Browse the repository at this point in the history
- Always pass paths as `Path` aka `etl::string` and use `c_str()` to
  ensure that the littlefs functions get null-terminated strings
- Move member variables which are only used in `CreateLockFile()` to
  that function
  • Loading branch information
PatrickKa committed Oct 5, 2024
1 parent bd1bffc commit 45988ae
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 22 deletions.
33 changes: 17 additions & 16 deletions Sts1CobcSw/FileSystem/LfsWrapper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -43,22 +43,23 @@ auto Unmount() -> Result<void>
}


auto Open(std::string_view path, unsigned int flags) -> Result<File>
auto Open(Path const & path, unsigned int flags) -> Result<File>
{
auto file = File();
file.lockFilePath_ = Path(path.data(), path.size()).append(".lock");

auto errorLockFile = file.CreateLockFile();
if(errorLockFile.has_error())
// We need to create a temporary with Path(path) here since append() modifies the object and we
// don't want to change the original path
file.lockFilePath_ = Path(path).append(".lock");
auto createLockFileResult = file.CreateLockFile();
if(createLockFileResult.has_error())
{
return errorLockFile.error();
return createLockFileResult.error();
}

auto error = lfs_file_opencfg(
&lfs, &file.lfsFile_, path.data(), static_cast<int>(flags), &file.lfsFileConfig_);
&lfs, &file.lfsFile_, path.c_str(), static_cast<int>(flags), &file.lfsFileConfig_);
if(error == 0)
{
file.path_ = Path(path.data(), path.size());
file.path_ = path;
file.openFlags_ = flags;
file.isOpen_ = true;
return file;
Expand Down Expand Up @@ -115,7 +116,7 @@ auto File::Close() -> Result<void>
return outcome_v2::success();
}

auto error = lfs_remove(&lfs, lockFilePath_.data());
auto error = lfs_remove(&lfs, lockFilePath_.c_str());
if(error != 0)
{
return static_cast<ErrorCode>(error);
Expand Down Expand Up @@ -161,16 +162,16 @@ auto File::MoveConstructFrom(File * other) noexcept -> void
}


auto File::CreateLockFile() noexcept -> Result<int>
auto File::CreateLockFile() noexcept -> Result<void>
{
lfs_file_t lfsLockFile;

auto lfsLockFile = lfs_file_t{};
auto lockFileBuffer = std::array<Byte, lfsCacheSize>{};
auto lfsLockFileConfig = lfs_file_config{.buffer = lockFileBuffer.data()};
auto error = lfs_file_opencfg(&lfs,
&lfsLockFile,
lockFilePath_.data(),
static_cast<unsigned int>(LFS_O_RDWR | LFS_O_CREAT) | LFS_O_EXCL,
&lfsLockFileConfig_);

lockFilePath_.c_str(),
static_cast<unsigned int>(LFS_O_RDWR) | LFS_O_CREAT | LFS_O_EXCL,
&lfsLockFileConfig);
if(error == 0)
{
error = lfs_file_close(&lfs, &lfsLockFile);
Expand Down
9 changes: 3 additions & 6 deletions Sts1CobcSw/FileSystem/LfsWrapper.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,9 @@ class File;

[[nodiscard]] auto Mount() -> Result<void>;
[[nodiscard]] auto Unmount() -> Result<void>;
[[nodiscard]] auto Open(std::string_view path, unsigned int flags) -> Result<File>;
[[nodiscard]] auto Open(Path const & path, unsigned int flags) -> Result<File>;


// FIXME: Make File const-correct (only Write() should be non-const)
// TODO: Consider moving this class to a separate file
class File
{
Expand All @@ -38,7 +37,7 @@ class File
auto operator=(File && other) noexcept -> File &;
~File();

friend auto Open(std::string_view path, unsigned int flags) -> Result<File>;
friend auto Open(Path const & path, unsigned int flags) -> Result<File>;

template<std::size_t extent>
[[nodiscard]] auto Read(std::span<Byte, extent> data) const -> Result<int>;
Expand All @@ -52,7 +51,7 @@ class File
// Only allow creation of File class through friend function Open()
File() = default;
auto MoveConstructFrom(File * other) noexcept -> void;
[[nodiscard]] auto CreateLockFile() noexcept -> Result<int>;
[[nodiscard]] auto CreateLockFile() noexcept -> Result<void>;
[[nodiscard]] auto Read(void * buffer, std::size_t size) const -> Result<int>;
[[nodiscard]] auto Write(void const * buffer, std::size_t size) -> Result<int>;

Expand All @@ -62,9 +61,7 @@ class File
mutable lfs_file_t lfsFile_ = {};
bool isOpen_ = false;
std::array<Byte, lfsCacheSize> buffer_ = {};
std::array<Byte, lfsCacheSize> bufferLockFile_ = {};
lfs_file_config lfsFileConfig_ = {.buffer = buffer_.data()};
lfs_file_config lfsLockFileConfig_ = {.buffer = bufferLockFile_.data()};
};
}

Expand Down

0 comments on commit 45988ae

Please sign in to comment.