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

Replace fork by posix_spawn in mount_util.c #902

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

bsbernd
Copy link
Collaborator

@bsbernd bsbernd commented Mar 13, 2024

This completes commit bb9cecb
("Use posix_spawn instead of fork+exec") and replaces remaining
fork by posix_spawn.

Not modified is mount_bsd.d as I cannot test it.

Another commit adds the env FUSE_NO_MTAB_UPDATE to be able to
disable mtab updates without recompilation.

@bsbernd bsbernd added the do-not-merge WIP or needs to wait till next release label Mar 13, 2024
This completes commit bb9cecb
("Use posix_spawn instead of fork+exec") and replaces remaining
fork by posix_spawn.

Not modified is mount_bsd.d as I cannot test it.
This adds the env FUSE_NO_MTAB_UPDATE variable to be able
to disable mtab updates without recompiling libfuse.
The value needs to be set to 1.

This is an addition to commit 74b1df2, which removed
a check for mtab updates, if mtab is a symlink.
Although not related to the symlink, this caused a regression
in our code, as the symlink update was done through fork.
The env variable is a run time workaround for such cases and
to avoid the requirement to disable mtab updates at libfuse
configuration/compilation time.

int flags = POSIX_SPAWN_RESETIDS | POSIX_SPAWN_SETSIGMASK;
res = posix_spawnattr_setflags(attr, flags);
if (res) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why the difference between this check and the one on line 110? From the man pages both appear to return 0 on success and an errno on error.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks, the check below for (res < 0) needs to be fixed. Although I see it in line 103

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
do-not-merge WIP or needs to wait till next release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants