Skip to content

Conversation

vtikoo
Copy link
Contributor

@vtikoo vtikoo commented Nov 2, 2021

Signed-off-by: Vikas Tikoo vikasamar@gmail.com

Signed-off-by: Vikas Tikoo <vikasamar@gmail.com>
@ghost ghost added area-PAL-coreclr community-contribution Indicates that the PR has been added by a community member labels Nov 2, 2021
Copy link
Member

@adamsitnik adamsitnik left a comment

Choose a reason for hiding this comment

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

@vtikoo have you hit any particular problem or just found a possible issue when analyzing the source code?

LGTM, thank you!

Comment on lines +753 to +754
flags = fcntl(fds[0], F_GETFL, 0);
fcntl(fds[0], F_SETFL, flags | O_NONBLOCK);
Copy link
Member

Choose a reason for hiding this comment

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

an alternative would be using pipe2 but this would require an #ifdef

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do some targets not support pipe2?

Copy link
Member

Choose a reason for hiding this comment

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

pipe2 is Linux-specific and we support macOS and few other Unix-like OSese. The current solution is good, I am going to merge the PR.

@adamsitnik adamsitnik requested a review from trylek November 24, 2021 10:33
@vtikoo
Copy link
Contributor Author

vtikoo commented Nov 25, 2021

@vtikoo have you hit any particular problem or just found a possible issue when analyzing the source code?

I hit this when running against a library OS our team is working on - Mystikos.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-PAL-coreclr community-contribution Indicates that the PR has been added by a community member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants