Skip to content

Upgrade c-ares#1678

Merged
bnoordhuis merged 3 commits intonodejs:masterfrom
bnoordhuis:upgrade-cares
May 12, 2015
Merged

Upgrade c-ares#1678
bnoordhuis merged 3 commits intonodejs:masterfrom
bnoordhuis:upgrade-cares

Conversation

@bnoordhuis
Copy link
Member

@bnoordhuis bnoordhuis commented May 11, 2015

@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
trevnorris
trevnorris reviewed May 12, 2015
View reviewed changes
deps/cares/src/ares_gethostbyname.c
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

@trevnorris trevnorris May 12, 2015

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

@bnoordhuis bnoordhuis May 12, 2015

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

trevnorris commented May 12, 2015

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
@bnoordhuis
deps: sync with upstream c-ares/c-ares@bba4dc5
7e1c0e7 
Fixes: nodejs#1676
PR-URL: nodejs#1678
Reviewed-By: Fedor Indutny <fedor@indutny.com>
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
@indutny @bnoordhuis
deps: provide TXT chunk info in c-ares
0f850f7 
Provide more information in `ares_txt_reply` to coalesce chunks from the
same record into one string.

fix nodejs#7367
@zcbenz @bnoordhuis
src,deps: replace LoadLibrary by LoadLibraryW
08d0866 
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

bnoordhuis commented May 12, 2015

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.

@bnoordhuis bnoordhuis mentioned this pull request May 12, 2015
Closed
@Fishrock123 Fishrock123 mentioned this pull request May 13, 2015
Merged
Fishrock123 pushed a commit to Fishrock123/node that referenced this pull request May 19, 2015
@bnoordhuis @Fishrock123
deps: sync with upstream c-ares/c-ares@bba4dc5
8ebdb29 
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

Reviewers

No reviews

Assignees

No one assigned

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

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@bnoordhuis @trevnorris @indutny @mscdex @zcbenz

Comments