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

Fix a bug in Select() on *nix. #4652

Merged
merged 1 commit into from Nov 25, 2015

Conversation

@pgavlin
Copy link
Contributor

pgavlin commented Nov 24, 2015

The translations between PAL <-> platform file descriptor sets were inverted,
causing incorrect behavior in the case that a Select() completed with any
signalled file descriptors.

The translations between PAL <-> platform file descriptor sets were inverted,
causing incorrect behavior in the case that a Select() completed with any
signalled file descriptors.
@pgavlin

This comment has been minimized.

Copy link
Contributor Author

pgavlin commented Nov 24, 2015

@pgavlin

This comment has been minimized.

Copy link
Contributor Author

pgavlin commented Nov 24, 2015

This fixes #4631.

@davidsh davidsh added this to the 1.0.0-rc2 milestone Nov 24, 2015
@pgavlin

This comment has been minimized.

Copy link
Contributor Author

pgavlin commented Nov 24, 2015

Ping. I'd like to get this merged as soon as possible.

size_t bytesToCopy = static_cast<size_t>((fdCount / 8) + ((fdCount % 8) == 0 ? 1 : 0));

uint8_t* dest;
uint8_t* source;

This comment has been minimized.

Copy link
@stephentoub

stephentoub Nov 25, 2015

Member

Can you remind me why we have this whole else section? Is it purely a performance thing to be able to use memcpy, and we're sure that's valid?

This comment has been minimized.

Copy link
@pgavlin

pgavlin Nov 25, 2015

Author Contributor

Yes, it's just a performance thing. I am certain that the memcpy is valid: both the source and target are bit vectors with the same size, polarity, and indexing semantics.

}

[Fact]
public void SelectError_Multiple_Timeout()

This comment has been minimized.

Copy link
@stephentoub

stephentoub Nov 25, 2015

Member

Can we somehow add a test for selecting with an error list and having something remain in the list (i.e. there is an error)?

This comment has been minimized.

Copy link
@pgavlin

pgavlin Nov 25, 2015

Author Contributor

I tried: unfortunately I couldn't find something that worked reliably.

@stephentoub

This comment has been minimized.

Copy link
Member

stephentoub commented Nov 25, 2015

Two nit questions, but LGTM. Thanks for the fix.

@CIPop

This comment has been minimized.

Copy link
Member

CIPop commented Nov 25, 2015

LGTM (Looked mainly at the tests.)

pgavlin added a commit that referenced this pull request Nov 25, 2015
Fix a bug in Select() on *nix.
@pgavlin pgavlin merged commit 339dcf2 into dotnet:master Nov 25, 2015
1 check passed
1 check passed
default Build finished. No test results found.
Details
@pgavlin pgavlin deleted the pgavlin:gh4631 branch Nov 25, 2015
@karelz karelz added the os-linux label Mar 8, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.