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

stream: use SO_OOBINLINE on OS X #222

Closed
wants to merge 2 commits into from
Closed

Conversation

indutny
Copy link
Member

@indutny indutny commented Feb 24, 2015

In the collaboration with Ben Noordhuis info@bnoordhuis.nl and
Saúl Ibarra Corretgé saghul@gmail.com.

@saghul please process this urgently

In the collaboration with Ben Noordhuis <info@bnoordhuis.nl> and
Saúl Ibarra Corretgé <saghul@gmail.com>.

/* Send some OOB data */
ASSERT(0 == uv_fileno((uv_handle_t*) &client_handle, &fd));

Copy link
Member

Choose a reason for hiding this comment

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

The problem triggered with more than one message, right? If so, can you please add a comment clarifying that here?

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.

@bnoordhuis
Copy link
Member

LGTM. Great test case!


ASSERT(0 == uv_stream_set_blocking((uv_stream_t*) &client_handle, 1));

do {
Copy link
Member

Choose a reason for hiding this comment

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

The problem triggered with more than one message, right? If so, can you please add a comment clarifying that here?

@indutny
Copy link
Member Author

indutny commented Feb 24, 2015

@bnoordhuis ahaha! :) With your help, as cited.

@saghul
Copy link
Member

saghul commented Feb 24, 2015

LGTM. If you squash and land I can make a v1.x release. Also we'll need a backport for 0.10.

@indutny
Copy link
Member Author

indutny commented Feb 24, 2015

@saghul thanks, landed in e19089f . Please release (and maybe backport it ;) ? )

@indutny indutny closed this Feb 24, 2015
@indutny indutny deleted the fix/oob branch February 24, 2015 19:34
gagern pushed a commit to gagern/libuv that referenced this pull request Apr 7, 2015
io.js ships a newer V8 version where String::ExternalAsciiStringResource
has been replaced with String::ExternalOneByteStringResource.

This change makes nan compile again with V8 3.29 and up.

Fixes libuv#222.
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.

3 participants