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

support slash-ended mkdir, such as "mkdir dir/". #679

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

XinStellaris
Copy link
Contributor

"mkdir dir/" will not work in littlefs, littlefs will return LFS_ERR_NOENT in such case. However, linux supports such commands. This patch adds support for such behavior.

Signed-off-by: 田昕 tianxin7@xiaomi.com

@XinStellaris XinStellaris force-pushed the support_slash_ended_mkdir branch from 51cdbb1 to 3f82e2f Compare May 16, 2022 13:00
@XinStellaris XinStellaris marked this pull request as draft May 16, 2022 14:13
Signed-off-by: 田昕 <tianxin7@xiaomi.com>
@XinStellaris XinStellaris force-pushed the support_slash_ended_mkdir branch from 3f82e2f to 4e51a1f Compare May 16, 2022 14:26
@XinStellaris XinStellaris marked this pull request as ready for review May 16, 2022 14:32
@XinStellaris XinStellaris marked this pull request as draft May 17, 2022 02:39
@XinStellaris
Copy link
Contributor Author

Oh no, this patch causes other problems. If we input following commands:

  1. echo 1 > testfile
  2. echo 2 > testfile/
    the result is testfile has content of 2. However, step2 should fail because testfile/ is a dir.

I am confused now, how to support slash-ended mkdir command? Or, is this really necessary?
@geky

@geky
Copy link
Member

geky commented Sep 7, 2022

Hi @XinStellaris sorry about the late response. Thanks for creating the PR, it would certainly be good to match Linux/POSIX here.

Hmm, this does look tricky to add. The simplest thing would be include a flag indicating that lfs_dir_find should ignore the trailing slash, this should only be set for lfs_mkdir. Sometimes special cases need to be special cases.

Then the test for the last name could be written:

const char *tail = name + strcspn(name, "/");
if (ignore_trailing_slashes) {
    tail += strspn(tail, "/");
}
// are we last name?
(*tail == '\0' ? id : NULL)

@geky geky added enhancement needs fix we know what is wrong needs minor version new functionality only allowed in minor versions and removed needs fix we know what is wrong labels Sep 7, 2022
@geky geky added the api tweak label Nov 23, 2024
@geky
Copy link
Member

geky commented Nov 24, 2024

I've gone ahead and reimplemented this in: #1046

The solution was a bit involved, I decided to solve the trailing slash issue in the calling functions by checking the modified path. It turns out after lfs_dir_find, since the modified path is no longer NULL-terminated (slash-terminated?), we can determine whether or not the path points to a directory or not based on what follows the last file name.

Let me know if you see any issues with #1046, and thanks for the original PR!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api tweak enhancement needs minor version new functionality only allowed in minor versions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants