Skip to content

Conversation

kyllingstad
Copy link
Contributor

https://issues.dlang.org/show_bug.cgi?id=13100

There are two cases in which the fcntl() calls may fail:

  1. the file descriptor is invalid, in which case errno == EBADF,
  2. they receive invalid arguments, in which case errno == EINVAL.

We just want to ignore the error in the former case, as evidenced by the bug report, and the latter means that we have an error in std.process, which is best handled by an assertion. (Note that it was never possible to catch the exception anyway, since it was thrown in the child process.)

There are two cases in which the fcntl() calls may fail:

  1) the file descriptor is invalid, in which case errno == EBADF,
  2) they receive invalid arguments, in which case errno == EINVAL.

We just want to ignore the error in the former case, as evidenced by
the bug report, and the latter means that we have an error in
std.process, which is best handled by an assertion. (Note that it was
never possible to catch the exception anyway, since it was thrown in
the child process.)
@Hackerpilot
Copy link
Contributor

Looks good to me.

@mihails-strasuns
Copy link

and the latter means that we have an error in std.process, which is best handled by an assertion

Just to make sure I understand it correctly - fcntl argument validity is 100% defined by internal std.process implementation and there is no way user input can affect this?

@kyllingstad
Copy link
Contributor Author

fcntl argument validity is 100% defined by internal std.process implementation and there is no way user input can affect this?

That's right. It is private and only called from spawnProcess() with the standard input/output/error stream file descriptors as arguments. (In fact, it should probably be nested inside spawnProcess(), but that is for a different pull request.)

@mihails-strasuns
Copy link

Thanks, that nails it for me.

@mihails-strasuns
Copy link

Auto-merge toggled on

mihails-strasuns pushed a commit that referenced this pull request Jul 12, 2014
Fix issue #13100 - std.process.setCLOEXEC() throws on invalid file descriptor
@mihails-strasuns mihails-strasuns merged commit 357b627 into dlang:master Jul 12, 2014
@kyllingstad
Copy link
Contributor Author

@akushner: You're welcome!

@kyllingstad kyllingstad deleted the no-setcloexec-fail branch August 5, 2014 15:10
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.

4 participants