Skip to content
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

trailing '/' on file confuses stat but not open #1040

Open
rob-zeno opened this issue Nov 4, 2024 · 4 comments
Open

trailing '/' on file confuses stat but not open #1040

rob-zeno opened this issue Nov 4, 2024 · 4 comments
Labels
needs fix we know what is wrong needs minor version new functionality only allowed in minor versions

Comments

@rob-zeno
Copy link

rob-zeno commented Nov 4, 2024

Recently integrated littlefs support into my Zephyr-based application and was doing some crude tests.

I noticed that if I create a file:

   `/mountpoint/file`

And then I pass

   `/mountpoint/file/`

(note the trailing '/') to Zephyr's fs_stat (which calls lfs_stat), it will return information about the file. That seems incorrect -- the trailing '/' ought to require that the item be a directory.

The fs_open (which calls lfs_open) call DOES fail -- correctly in my opinion.

@geky geky added needs minor version new functionality only allowed in minor versions needs fix we know what is wrong labels Nov 18, 2024
@geky
Copy link
Member

geky commented Nov 18, 2024

Hi @rob-zeno, thanks for creating an issue.

This is a good find, and it does look like a problem on littlefs's side.

There is a similar issue in lfs_mkdir, though humorously reversed, where lfs_mkdir rejects trailing slashes (#679 and #428).

These both stem from a lack of testing around trailing slashes.


It would be good to fix this, though one hiccup is does a change to path behavior constitute a breaking API change. I think you can argue no, since the current behavior is inconsistent both with POSIX and, as you noted, with itself.

@geky
Copy link
Member

geky commented Nov 24, 2024

Hi @rob-zeno, I've gone ahead and put together a PR that should fix this: #1046

With any luck it should land in the next minor release.

@rob-zeno
Copy link
Author

Thanks! I'm following along in 1046, and looking forward to the fixes!

BTW, I found some other issues with Zephyr and large flash parts which turned out to not be LittleFS related at all, although it was LittleFS failues that confused me. For a very large flash part (64MB), unless I used JEDEC runtime discovery (which is NOT the default in Zephyr), it would only use 3-byte addressing, and that caused LittleFS file systems in the lower part of the flash part to get actually written in the upper part. Very scary when I first saw it, but when I finally got to the bottom of it, it made sense, and after enabling the JEDEC runtime support (which allowed 4-byte addressing to work), LittleFS is behaving rock-solid in my testing so far. There's probably something to take up with the Zephyr community -- if large flash parts won't work with 3-byte addressing, then they should probably fail (it's actually detectable at build-time and run-time). But LittleFS isn't involved.

@geky
Copy link
Member

geky commented Nov 25, 2024

That's good to know. I don't really have any connections to Zephyr, but it's useful if someone else has a similar problem.

I sort of understand why they wouldn't include JEDEC by default. I've poked around it before and it's an ugly paywalled monster of a spec. It's jarring after the 3-byte address protocol is so simple. But you would hope Zephyr would at least error instead of silently corrupting things...

This sort of problem -- I've been calling it address truncation -- is very common. Or at least I get a lot of bug reports about it since it usually looks like a filesystem failure. Eventually I want to add a sort of lfs_testbd(&lfs, &cfg) that rules out problems like these, but it's on the backburner.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs fix we know what is wrong needs minor version new functionality only allowed in minor versions
Projects
None yet
Development

No branches or pull requests

2 participants