Skip to content

Commit

Permalink
Little FS improvements and IFS revisions to support user metadata (#2308
Browse files Browse the repository at this point in the history
)

**Main changes**

This PR adds the following methods to `IFileSystem`:

* setProfiler

This allows applications to hook into the low-level read/write/erase calls for debugging and profiling.

* fsetxattr
* fgetxattr
* setxattr
* getxattr

The `setacl`, `settime`, `setattr` and `setcompression` methods have been moved to `FileSystem` and
call the above new methods to do their work.

Additional overloads have been provided in FileSystem (and via methods of File objects):

* setAttribute
* getAttribute

These work with file handles or paths and have various convenient overloads.

* setUserAttribute, getUserAttribute

These are to access user-defined attributes.

For LFS volumes, each item is limited to a maximum size of 1023 bytes.

SPIFFS is less flexible (by design). Metadata is stored in the object headers
(after filename, etc.) and is restricted by the setting of `SPIFFS_OBJ_META_LEN`.
The first 16 bytes are used for system attributes (e.g. modified time), with user attributes
occupying any remaining space.

The current default setting (16) is unchanged, so if user metadata for SPIFFS volumes
is required then this must be increased accordingly.
Each user attribute requires a 2-byte header (tag + size).


* fopen
* fopendir

Removed - see below.


**Fixes**

- Revise HYFS to track file path

SPIFFS getFilePath() call sometimes returns weird results.

- Fix SPIFFS  FileSystem::getFilePath return code

- Fix FileDir weirdness

The generic `DirHandle` type points to an `IFS::FileDir` struct, which each filesystem defines internally as required.
It looks like the compiler sometimes mixes these up and causes problems,
though this has only been seen when compiling in debug mode for Host.

So instead, redefine `DirHandle` as an abstract type, move the `FileDir` definition inside the appropriate filesystem namespace
and add the `GET_FILEDIR()` macro to deal with checking and casting.

- Fix memory leak in test module (thanks valgrind)

- Set lasterror in `File::readContent()`

- Fix `FileSystem::makedirs()`

- Fix LFS reported volumesize


**Improvements**

- Increase LFS_CACHE_SIZE from 16 to 32 bytes

Results in signficant reduction in read counts

- Switch to littlefs fork, manage file attributes manually

Only changed attributes (and those with non-default values) get written out
File time only gets updated if file is changed, opening in write mode is insufficient

- LFS mkdir sets mtime on success

- LFS `mkdir` succeeds if directory already exists

- Add LFS inspect sample

- Implement Host `flush`, `rename` and `remove` methods

- Helpers return FileSystem* instead of IFileSystem* for easier use

- Add optional `IFileSystem::setProfiler` method to enable flexible debugging and performance evaluation

- Revise hybrid filesystem to allow use of alternative writeable filesystem (e.g. Little FS)

- Remove `IFileSystem.fopendir(Stat&)` and `IFileSystem.fopen(Stat&)`

The goal of these methods was to provide more efficient means to open files/directories discovered during enumeration.
However, they are difficult to implement effectively and the additional complexity is hard to justify.

POSIX defines a whole raft of such methods such as `openat`, `mkdirat`, etc. which add an additional `dirfd` parameter
to specific the working directory. See https://man7.org/linux/man-pages/man3/open.3p.html for some background.

This may be re-visited in the future if the need arises.

- Add generic get/set attribute methods

These support LFS file/directory attributes as well as standard attribute types via `AttributeTag`.

There is no POSIX specification for this but the linux convention (setxattr, etc.) are appropriate.
  • Loading branch information
mikee47 authored and slaff committed Sep 27, 2021
1 parent 21437bc commit 2651856
Show file tree
Hide file tree
Showing 16 changed files with 131 additions and 171 deletions.
13 changes: 10 additions & 3 deletions Sming/Components/spiffs/spiffs.patch
Original file line number Diff line number Diff line change
Expand Up @@ -71,18 +71,25 @@ index 235aaaa..4df4b4e 100644
spiffs_page_object_ix_header objix_hdr;

diff --git a/src/spiffs_nucleus.c b/src/spiffs_nucleus.c
index f811d93..ff9db29 100644
index f811d93..781c52f 100644
--- a/src/spiffs_nucleus.c
+++ b/src/spiffs_nucleus.c
@@ -945,6 +945,7 @@ s32_t spiffs_object_create(
@@ -939,12 +939,14 @@ s32_t spiffs_object_create(
fs->stats_p_allocated++;

// write empty object index page
+ memset(&oix_hdr, 0xff, sizeof(oix_hdr));
oix_hdr.p_hdr.obj_id = obj_id;
oix_hdr.p_hdr.span_ix = 0;
oix_hdr.p_hdr.flags = 0xff & ~(SPIFFS_PH_FLAG_FINAL | SPIFFS_PH_FLAG_INDEX | SPIFFS_PH_FLAG_USED);
oix_hdr.type = type;
oix_hdr.size = SPIFFS_UNDEFINED_LEN; // keep ones so we can update later without wasting this page
strncpy((char*)oix_hdr.name, (const char*)name, SPIFFS_OBJ_NAME_LEN);
+ oix_hdr.name[SPIFFS_OBJ_NAME_LEN - 1] = '\0';
#if SPIFFS_OBJ_META_LEN
if (meta) {
_SPIFFS_MEMCPY(oix_hdr.meta, meta, SPIFFS_OBJ_META_LEN);
@@ -1008,6 +1009,7 @@ s32_t spiffs_object_update_index_hdr(
@@ -1008,6 +1010,7 @@ s32_t spiffs_object_update_index_hdr(
// change name
if (name) {
strncpy((char*)objix_hdr->name, (const char*)name, SPIFFS_OBJ_NAME_LEN);
Expand Down
4 changes: 2 additions & 2 deletions Sming/Core/Data/Stream/FileStream.h
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,9 @@ class FileStream : public IFS::FileStream
open(fileName, openFlags);
}

FileStream(const FileStat& stat, FileOpenFlags openFlags = File::ReadOnly) : FileStream()
FileStream(DirHandle dir, const String& name, FileOpenFlags openFlags = File::ReadOnly) : FileStream()
{
open(stat, openFlags);
open(dir, name, openFlags);
}

using IFS::FileStream::attach;
Expand Down
49 changes: 7 additions & 42 deletions Sming/Core/Data/Stream/IFS/FileStream.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,8 @@ void FileStream::attach(FileHandle file, size_t size)
return;
}

auto fs = getFileSystem();
if(fs == nullptr) {
return;
}
GET_FS()

handle = file;
this->size = size;
fs->lseek(handle, 0, SeekOrigin::Start);
Expand All @@ -33,30 +31,9 @@ void FileStream::attach(FileHandle file, size_t size)
debug_d("attached file: '%s' (%u bytes) #0x%08X", fileName().c_str(), size, this);
}

bool FileStream::open(const Stat& stat, OpenFlags openFlags)
{
auto fs = getFileSystem();
if(fs == nullptr) {
return false;
}

lastError = FS_OK;

FileHandle file = fs->fopen(stat, openFlags);
if(!check(file)) {
return false;
}

attach(file, stat.size);
return true;
}

bool FileStream::open(const String& fileName, OpenFlags openFlags)
{
auto fs = getFileSystem();
if(fs == nullptr) {
return false;
}
GET_FS(false)

lastError = FS_OK;

Expand Down Expand Up @@ -96,10 +73,7 @@ size_t FileStream::readBytes(char* buffer, size_t length)
return 0;
}

auto fs = getFileSystem();
if(fs == nullptr) {
return 0;
}
GET_FS(0)

int available = fs->read(handle, buffer, std::min(size - pos, length));
if(!check(available)) {
Expand All @@ -113,10 +87,7 @@ size_t FileStream::readBytes(char* buffer, size_t length)

uint16_t FileStream::readMemoryBlock(char* data, int bufSize)
{
auto fs = getFileSystem();
if(fs == nullptr) {
return 0;
}
GET_FS(0)

assert(bufSize >= 0);
size_t startPos = pos;
Expand All @@ -131,10 +102,7 @@ uint16_t FileStream::readMemoryBlock(char* data, int bufSize)

size_t FileStream::write(const uint8_t* buffer, size_t size)
{
auto fs = getFileSystem();
if(fs == nullptr) {
return 0;
}
GET_FS(0)

if(pos != this->size) {
int writePos = fs->lseek(handle, 0, SeekOrigin::End);
Expand All @@ -156,10 +124,7 @@ size_t FileStream::write(const uint8_t* buffer, size_t size)

int FileStream::seekFrom(int offset, SeekOrigin origin)
{
auto fs = getFileSystem();
if(fs == nullptr) {
return 0;
}
GET_FS(lastError)

// Cannot rely on return value from fileSeek - failure does not mean position hasn't changed
fs->lseek(handle, offset, origin);
Expand Down
20 changes: 17 additions & 3 deletions Sming/Core/Data/Stream/IFS/FileStream.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,15 +35,22 @@ class FileStream : public FsBase, public ReadWriteStream
*/
void attach(FileHandle file, size_t size);

bool open(const Stat& stat, OpenFlags openFlags = OpenFlag::Read);
/** @brief Open a file by path, and attach this stream object to it
* @param fileName Full path to file
* @param openFlags
* @retval bool true on success, false on error
* @note call getLastError() to determine cause of failure
*/
bool open(const String& fileName, IFS::OpenFlags openFlags = OpenFlag::Read);

/** @brief Open a file and attach this stream object to it
* @param fileName
* @param dir Location of file
* @param fileName Name of file
* @param openFlags
* @retval bool true on success, false on error
* @note call getLastError() to determine cause of failure
*/
bool open(const String& fileName, IFS::OpenFlags openFlags = OpenFlag::Read);
bool open(DirHandle dir, const String& name, OpenFlags openFlags = OpenFlag::Read);

/** @brief Close file
*/
Expand Down Expand Up @@ -139,6 +146,13 @@ class FileStream : public FsBase, public ReadWriteStream
return truncate(pos);
}

bool stat(Stat& s)
{
GET_FS(false)

return check(fs->fstat(handle, &s));
}

private:
FileHandle handle{-1};
size_t pos{0};
Expand Down
7 changes: 4 additions & 3 deletions Sming/Core/FileSystem.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,14 @@

namespace SmingInternal
{
IFS::IFileSystem* activeFileSystem;
IFS::FileSystem* activeFileSystem;
}

void fileSetFileSystem(IFS::IFileSystem* fileSystem)
{
if(SmingInternal::activeFileSystem != fileSystem) {
delete SmingInternal::activeFileSystem;
SmingInternal::activeFileSystem = fileSystem;
SmingInternal::activeFileSystem = IFS::FileSystem::cast(fileSystem);
}
}

Expand Down Expand Up @@ -79,7 +79,8 @@ bool hyfs_mount()

bool hyfs_mount(Storage::Partition fwfsPartition, Storage::Partition spiffsPartition)
{
auto fs = IFS::createHybridFilesystem(fwfsPartition, spiffsPartition);
auto ffs = IFS::createSpiffsFilesystem(spiffsPartition);
auto fs = IFS::createHybridFilesystem(fwfsPartition, ffs);
return fileMountFileSystem(fs);
}

Expand Down
Loading

0 comments on commit 2651856

Please sign in to comment.