Skip to content

Improve auto unmount #293

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

Merged
merged 1 commit into from
Jun 10, 2017
Merged

Improve auto unmount #293

merged 1 commit into from
Jun 10, 2017

Conversation

benrubson
Copy link
Contributor

@benrubson benrubson commented Mar 9, 2017

Hello,

Here is a PR to improve auto unmount.

It does not use fuse_unmount_compat22 anymore, which :

  • has been removed from libfuse3 ;
  • has some issues under FreeBSD.

Thank you 👍

Ben

This was referenced Mar 9, 2017
@benrubson benrubson mentioned this pull request Apr 4, 2017
encfs/main.cpp Outdated
if (access("/bin/umount", F_OK) != -1)
rc = system(("/bin/umount "+std::string(arg->opts->mountPoint)).c_str());
else
rc = system(("/sbin/umount "+std::string(arg->opts->mountPoint)).c_str());
Copy link
Collaborator

Choose a reason for hiding this comment

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

I wonder if it's worth it to search $PATH if mount is not found in the "normal" locations.

Also, how would this port to Windows, Mac OS, or FreeBSD?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for your review Sam !

The risk of searching through $PATH is to run a malicious umount, this is why I used only hard-coded paths.

FreeBSD :

# which umount
/sbin/umount

Mac OS :

# which umount
/sbin/umount

Do you have some systems in mind with umount elsewhere ?

Regarding Windows, good question.
Seems that encfs4win calls fuse_unmount but disables fuse_unmount_compat22, as this fuse_unmount call goes to Dokany library.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  • we can't call fuse_unmount as we don't have a fuse handle (fuse_main does not return it) ;

Well, according to encfs4win call to fuse_unmount (second parameter to NULL), and according to libfuse source code, we could clearly set this second parameter to NULL.

Let me do some tests, however I think I will still face the drawbacks listed in the first post.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I notice encfs4win is a complete fork. Is encfs not designed to run on Windows as well? I guess not since it requires a whole fork.

@benrubson
Copy link
Contributor Author

So here is a fuse-only method, which is then Windows compatible.

@samrocketman
Copy link
Collaborator

Cool :-) looks good to me. I'll let someone with more seasoned eyes than mine do the final approval and merge.

@benrubson
Copy link
Contributor Author

Thank you very much Sam 👍

@benrubson
Copy link
Contributor Author

Hi @rfjakob !
What about merging this PR ?
Many thx 👍

@rfjakob rfjakob merged commit d216cc4 into vgough:master Jun 10, 2017
@rfjakob
Copy link
Collaborator

rfjakob commented Jun 10, 2017

Let's go.

@benrubson
Copy link
Contributor Author

Thank you !

@benrubson benrubson deleted the autounmount branch June 10, 2017 20:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants