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

Upgrade c-ares #1678

Merged
merged 3 commits into from
May 12, 2015
Merged

Upgrade c-ares #1678

merged 3 commits into from
May 12, 2015

Conversation

bnoordhuis
Copy link
Member

@mscdex mscdex added c++ Issues and PRs that require attention from people who are familiar with C++. dns Issues and PRs related to the dns subsystem. labels May 11, 2015
if ((status == ARES_ENODATA || status == ARES_EBADRESP) &&
hquery->want_family == AF_UNSPEC) {
if ((status == ARES_ENODATA || status == ARES_EBADRESP ||
(status == ARES_SUCCESS && host && host->h_addr_list[0] == NULL)) &&
Copy link
Contributor

Choose a reason for hiding this comment

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

Just recalling, are we going to switch io.js to use the proper error names instead of the made up one we have (ENOTFOUND)?

Copy link
Member Author

Choose a reason for hiding this comment

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

I have no love for ENOTFOUND but renaming it may result in a lot of breakage. :-/

@trevnorris
Copy link
Contributor

Change looks fine to me.

@indutny
Copy link
Member

indutny commented May 12, 2015

LGTM if CI is happy.

bnoordhuis and others added 3 commits May 12, 2015 10:46
Fixes: nodejs#1676
PR-URL: nodejs#1678
Reviewed-By: Fedor Indutny <fedor@indutny.com>
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
Provide more information in `ares_txt_reply` to coalesce chunks from the
same record into one string.

fix nodejs#7367
On Windows, when compiling with `UNICODE` defined, `LoadLibrary` becomes
`LoadLibraryW`. When an ASCII string is passed to that function it
crashes.

PR-URL: nodejs#226
Reviewed-By: Bert Belder <bertbelder@gmail.com>
@bnoordhuis bnoordhuis merged commit 08d0866 into nodejs:master May 12, 2015
@bnoordhuis bnoordhuis deleted the upgrade-cares branch May 12, 2015 08:50
@bnoordhuis
Copy link
Member Author

Landed in 7e1c0e7...08d0866. Thanks for the review, guys.

There are some CI failures but not ones introduced by this PR, AFAICT. (Child process failures on Windows, a dgram test that has been failing for some time now. There are issues about them.)

I decided to put not too much stock in the test failure on the armv7-wheezy buildbot because it always has one or two tests randomly failing (sadly.) If the failure turns out to be persistent, I'm on the hook for fixing it.

Fishrock123 pushed a commit to Fishrock123/node that referenced this pull request May 19, 2015
Fixes: nodejs#1676
PR-URL: nodejs#1678
Reviewed-By: Fedor Indutny <fedor@indutny.com>
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c++ Issues and PRs that require attention from people who are familiar with C++. dns Issues and PRs related to the dns subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants