8000 do not permit mounting over a directory or file with the same name as the mount point by dhalbert · Pull Request #935 · adafruit/circuitpython · GitHub
[go: up one dir, main page]
Skip to content

do not permit mounting over a directory or file with the same name as the mount point #935

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jun 16, 2018
Merged

do not permit mounting over a directory or file with the same name as the mount point #935

merged 1 commit into from
Jun 16, 2018

Conversation

dhalbert
Copy link
Collaborator
@dhalbert dhalbert commented Jun 15, 2018 •
edited
Loading

stat() for a file or directory with the same name.

Also I did all the error checking before doing any mount operations. For some reason the original code did part of the mount before checking for errors.

Fixes #933.

@dhalbert dhalbert requested a review from tannewt June 15, 2018 17:59
@dhalbert dhalbert closed this Jun 15, 2018
@dhalbert dhalbert deleted the mount_name_clash_check branch June 15, 2018 18:04
@dhalbert dhalbert restored the mount_name_clash_check branch June 15, 2018 18:06
@dhalbert
Copy link
Collaborator Author

deleted branch by mistake while cleaning up github repo

@dhalbert dhalbert reopened this Jun 15, 2018
tannewt
tannewt requested changes Jun 15, 2018
View reviewed changes
Copy link
Member
@tannewt tannewt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This mismatches UNIX right? Isn't a folder necessary as a mount point? I think we should match it if so.

@dhalbert
Copy link
Collaborator Author
dhalbert commented Jun 16, 2018 •
edited
Loading

In UNIX(-like) systems, you have to have an existing dir as a mountpoint. But storage.mount() does not require or mean to allow this now, and neither does MicroPython. In fact, right now, it breaks things to have a dir with the same name: you end up with two identical names in the dir listing and it wreaks havoc when you try to umount or use the mount point. So this checks for duplicates and complains. It's just enforcing something that should have been tested previously. In MicroPython it wasn't really an issue because MPy doesn't mount the base filesystem on /, it's on /flash instead.

The UNIX thing is an OS artifact, and not really applicable to FAT. Windows doesn't do this (it's completely different). I'd rather not introduce this new arbitrary requirement now, "well, yeah, you have to create a folder with the same name".

Long discussion of why UNIX does this: https://unix.stackexchange.com/questions/251090/why-does-mount-happen-over-an-existing-directory

@tannewt
Copy link
Member
tannewt commented Jun 16, 2018

Ok, sounds good.

@tannewt tannewt merged commit 35ead7b into adafruit:master Jun 16, 2018
@dhalbert dhalbert mentioned this pull request Jun 21, 2018
Merged
@dhalbert dhalbert deleted the mount_name_clash_check branch May 28, 2019 13:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tannewt tannewt tannewt approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@dhalbert @tannewt
0