-
Notifications
You must be signed in to change notification settings - Fork 806
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support get file path #976
base: master
Are you sure you want to change the base?
Conversation
Signed-off-by: crafcat7 <[email protected]>
Tests passed ✓, Code: 17300 B (+1.4%), Stack: ∞ B (-100.0%), Structs: 812 B (+0.0%)
|
Hi @crafcat7, thanks for creating a PR for this. This is a clever implementation, unfortunately I don't think it makes sense to add this API to littlefs. I need to default to rejecting features without a strong motivation in order to combat feature-creep/code cost, so sorry if this comes across a bit harsh. The first issue is this seems relatively easy to emulate outside of the littlefs core. When a user opens a file, they must know the file path, so if they want access to the file path later, they can store the file path next to the The second issue is the use of There is also work in progress to move away from I think when PC-scale OSs provide this feature, they are just returning the path stored in RAM inside the kernel. |
some OSes including linux, yes. some others implement it by performing readdir on ".." as it would be done for a naive getcwd() implementation. |
Oh interesting, that is clever. It would also not work because littlefs doesn't really have ".." entries. They are just faked in the path parser. I wonder if the lack of real ".." entries will become problematic when adding |
yes. actually, it's problematic even for traditional chdir(). btw, this get-path functionality "requirement" came from an attempt to implement F_SETLK style file lock. apache/nuttx#11724 |
Unfortunately, no. This is an area where littlefs diverges greatly from POSIX filesystems in that littlefs doesn't have inodes, so no unique ino, no hard-linking, etc... This wasn't really an intentional design decision, just a side-effect of not having inodes as a requirement, unlike filesystem that start off in Linux, etc. It's in theory possible to extend littlefs to use inodes, but it would add a layer of indirection and complexity (and ino reclamation, bleh). littlefs does map each file to an id internally, but this id can change if neighboring files are created/deleted. This is one reason we keep all open files in a linked-list, so we can keeps files up to date with any id changes. This id hasn't been exposed in lfs_stat because it could be confused for an ino without actually being useful...
This is an interesting puzzle. I'm not really sure what the best solution is. Some thoughts: You could keep the file path around, though as you mention handling renames would be tricky. You'd also need to worry about messy path comparisons (a/b/c == /a/b/./c?)... You could use a custom attribute to assign each file a fake ino when needed. Though making sure each ino is unique would be difficult (ino reclamation, bleh). This solution would be a bit easier if we had a way to iterate over all files in the filesystem, which isn't an unreasonable API... If you keep track of each locked file handle, and we add some sort of comparison API (either lfs_file_handlecmp-like or by exposing the metadata-id), you could compare against all locked files to find conflicting locks. This would be less efficient than a hashtable, but littlefs's opened-linked-list already scales There is some planned work to make custom attributes with multiple open file handles more intuitive, by "broadcasting" custom attribute updates on lfs_file_sync. This almost sounds like what you need, except you probably don't want the locked flag to actually be written to disk... Maybe we should allow custom attributes to not be written to disk? Maybe this is the wrong tool for this...
This is probably something more familiar to OS-level developers, but outside of shell emulators and backwards compatibility, does chdir() really make sense for applications? It seems like you'd want to prefer openat/*at relative to an open directory handle these days. |
how do you plan to find the broadcast destinations?
in an ideal world, maybe. |
This is a trend off, file name consume memory, saving file path for each open handle will increase the memory consumption if many files are opened. |
There is an invasive linked-list going through all lfs_file_t and lfs_dir_t structs so we can keep these handles up-to-date. I suppose we could expose this to the user, though the API would be complicated (we can't just say use the internals of lfs_file_t, switching to type-specific linked-lists was considered at one point for example). Thinking about it, if you control the lfs_file_t structs, could you add an additional invasive linked-list outside of littlefs for the purpose of updating locks, etc?
Fair enough, guess adding openat will be a learning experience.
Fair point. But if Maybe an option is to 1. add an API to iterate over all files, and 2. add file handle comparison (metadata-id), so the |
are you talking about "lfs->mlist"?
is there a way to determine if given two lfs_flie_t objects are referencing to the same on-disk file or not? |
Yep. The exact logic to update ids/pairs, though messy, is here: Extending this to also update custom attributes would not be too difficult. The only blocker is that WRONLY attributes are currently allowed to be const, which is a bit of a problem. So I've been waiting to address this until an eventual major version bump.
Not currently, but this isn't an unreasonable API to add. The Right now creating an API it is a bit tricky, since you need to both compare the block addresses with |
ok. |
Yes, if this API is added to littlefs. But it's not currently exposed. That's interesting that WASI did not just expose ino, I wonder what other filesystems motivated this. |
iirc it's motivated by some windows filesystems (ReFS?) which have 128-bit file ID. |
I added two new interfaces to obtain the path of the file/folder.
Mainly rely on a new internal interface, to a recursive search target