search

littlefs-project / littlefs

A little fail-safe filesystem designed for microcontrollers
BSD 3-Clause "New" or "Revised" License
5.23k stars 801 forks source link

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

Open rob-zeno opened 3 weeks ago

rob-zeno commented 3 weeks ago

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 commented 1 week ago

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 (https://github.com/littlefs-project/littlefs/pull/679 and https://github.com/littlefs-project/littlefs/issues/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 commented 2 days ago

Hi @rob-zeno, I've gone ahead and put together a PR that should fix this: https://github.com/littlefs-project/littlefs/pull/1046

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

rob-zeno commented 1 day ago

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 commented 20 hours ago

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.