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

fcntl DUPFD doesn't work on pipes #14640

Closed
hoodmane opened this issue Jul 13, 2021 · 1 comment
Closed

fcntl DUPFD doesn't work on pipes #14640

hoodmane opened this issue Jul 13, 2021 · 1 comment

Comments

@hoodmane
Copy link
Contributor

hoodmane commented Jul 13, 2021

There is a problem in the implementation of DUPFD for pipes. The implementation assumes that stream.path is a valid path. However, for pipes, stream.path looks like pipe[n].
The issue is here:

newStream = FS.open(stream.path, stream.flags, 0, arg);

I think if instead it said:

newStream=FS.createStream(stream, arg);

that should fix the problem? Or maybe it should say Object.assign({}, stream) instead of stream as the argument? This seems less brittle to me than the current approach.

@hoodmane
Copy link
Contributor Author

hoodmane commented Jul 18, 2021

There's another bug here which is that instead of 0, arg the line quoted above should just have arg:

newStream = FS.open(stream.path, stream.flags, arg); 

From man fcntl:

Duplicate the file descriptor fd using the lowest-numbered available file descriptor greater than or equal to arg

Existing behavior uses the lowest-numbered available file descriptor less than arg.

hoodmane pushed a commit to hoodmane/emscripten that referenced this issue Mar 20, 2022
This fixes two problems with the `dup` system calls:
1. `dup` expects that every file descriptor has a corresponding file (so pipes and (emscripten-core#14640)
   files that have been unlinked (emscripten-core#15012) cannot be duplicated ).
2. The dup'd file descriptor does not share flags and position with the original file desciptor.

This is a simplification of an upstream pull request that would fix this problem.
https://github.com/emscripten-core/emscripten/pull/9396/files
This patch is simpler than the upstream one but leaves NODERAWFS broken.
hoodmane pushed a commit to hoodmane/emscripten that referenced this issue Mar 22, 2022
This fixes two problems with the `dup` system calls:
1. `dup` expects that every file descriptor has a corresponding file (so pipes and (emscripten-core#14640)
   files that have been unlinked (emscripten-core#15012) cannot be duplicated ).
2. The dup'd file descriptor does not share flags and position with the original file desciptor.

This is a simplification of an upstream pull request that would fix this problem.
https://github.com/emscripten-core/emscripten/pull/9396/files
This patch is simpler than the upstream one but leaves NODERAWFS broken.
kripken pushed a commit that referenced this issue Apr 4, 2022
This fixes two problems with the `dup` system calls:
1. `dup` expects that every file descriptor has a corresponding file (so pipes and (#14640)
   files that have been unlinked (#15012) cannot be duplicated ).
2. The dup'd file descriptor does not share flags and position with the original file desciptor.

This is a simplification of an upstream pull request that would fix this problem.
https://github.com/emscripten-core/emscripten/pull/9396/files
This patch is simpler than the upstream one but leaves NODERAWFS broken.
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

No branches or pull requests

1 participant