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.

Sign up for GitHub

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

Jump to bottom

backport #7345: src: renaming ares_task struct to node_ares_task #7669

Closed
wants to merge 46 commits into from
Closed

backport #7345: src: renaming ares_task struct to node_ares_task #7669

wants to merge 46 commits into from

Conversation

addaleax
Copy link
Member

@addaleax addaleax commented Jul 12, 2016 •
edited
Loading

@addaleax addaleax added c++ Issues and PRs that require attention from people who are familiar with C++. dns Issues and PRs related to the dns subsystem. v4.x labels Jul 12, 2016
@addaleax addaleax mentioned this pull request Jul 12, 2016
Closed
2 tasks
bnoordhuis and others added 25 commits July 11, 2016 22:33
@bnoordhuis @MylesBorins
test: check types for http request and response
76e65d9 
Add a basic regression test that checks if the map for IncomingMessage
and OutgoingMessage objects is stable over time.

The test is not exhaustive in that it doesn't try to establish whether
the transition path is the same on every request, it just checks that
objects in their final states have the same map.

To be investigated why the first (and only the first) ServerRequest
object ends up with a deprecated map, regardless of the number of
iterations.

PR-URL: nodejs#7003
Refs: nodejs#6294
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@kemitchell @MylesBorins
doc: mention http request "aborted" events
774fb78 
Fixes: nodejs#6925
PR-URL: nodejs#7270
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
@Fishrock123 @MylesBorins
doc: update "who to cc in issues" chart
748702a 
Refs: nodejs#6655

PR-URL: nodejs#6694
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ingvar Stepanyan <me@rreverser.com>
@lance @MylesBorins
doc: add lance to collaborators
1d4a4b2 
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
PR-URL: nodejs#7407
@joshgav @MylesBorins
doc: add CTC meeting minutes 2016-06-15
75d21ff 
PR-URL: nodejs#7320
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Rod Vagg <rod@vagg.org>
@princejwesley @MylesBorins
repl: fix tab completion for defined commands
73cdc54 
PR-URL: nodejs#7364
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
@sartrey @MylesBorins
doc: clarify child_process stdout/stderr types
3ce574e 
Clarify how the encoding option interacts with the data
type of child process stdout and stderr.

Fixes: nodejs#6666
PR-URL: nodejs#7361
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@rus0000 @MylesBorins
doc: fix "sign.verify" typo in crypto doc.
40bd4c1 
Fix typo in example

PR-URL: nodejs#7411
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@Trott @MylesBorins
assert: remove unneeded arguments special handling
0ecdd6f 
Remove special handling when asserting on a pair of arguments objects.
The code being removed will only run if both `expected` and `actual` are
arguments objects. Given that situation, the subsequent code for
handling everything else works just fine.

Tests added to confirm expected behavior.

This came about while trying to improve test coverage. The segment of
code removed had no test coverage. I was unable to write a test that
would both exercise the code and fail if the code was removed. Further
examination indicated that this was because the special handling was not
needed.

PR-URL: nodejs#7413
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@Trott @MylesBorins
doc: improve usage of zero/0
9e0aad2 
PR-URL: nodejs#7466
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Bryan English <bryan@bryanenglish.com>
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
@Trott @MylesBorins
test: remove common.PORT from http tests
5542971 
PR-URL: nodejs#7467
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@cjihrig @MylesBorins
doc: fix detached child stdio example
efca61e 
The example changed by this commit uses ['ignore'] where
'ignore' is intended.

Fixes: nodejs#7269
PR-URL: nodejs#7540
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
@bzoz @MylesBorins
doc: add bartosz sosnowski to colaborators
54930bf 
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: João Reis <reis@janeasystems.com>
PR-URL: nodejs#7567
@Trott @MylesBorins
doc: fix minor style issues in http.md
c93579d 
PR-URL: nodejs#7528
Reviewed-By: Anna Henningsen <anna@addaleax.net>
@Trott @MylesBorins
test: fix flaky test-net-write-slow
db020b4 
Increase socket timeout so that there is enough time to reliably run the
test on FreeBSD.

Fixes: nodejs#7516
PR-URL: nodejs#7555
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
@diosney @MylesBorins
doc: added information on how to run the linter.
eb841f9 
Added clarification about the linter execution.

PR-URL: nodejs#7534
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@Trott @MylesBorins
doc: add benchmark who-to-CC info
714aee2 
Add benchmark to "Who to CC". Also, alphabetized the only
non-alphabetized subsystem.

PR-URL: nodejs#7604
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
@qheaden @MylesBorins
doc: dns.resolve fix callback argument description
8c929aa 
The dns.resolve documentation stated that an array of IP
addresses would be returned in the callback. This is true
for everything other than the SOA record which returns an object.
This fixes that documentation.

Fixes: nodejs#6506
PR-URL: nodejs#7532
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Roman Reiss <me@silverwind.io>
@Trott @MylesBorins
tools: remove unused variable
f7e1bed 
`oldDirs` is assigned but never used. Remove it.

(This was missed by the linter in previous versions of ESLint but is
flagged by the current version. Updating the linter is contingent on
this change or some similar remedy landing.)

PR-URL: nodejs#7594
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michaël Zasso <mic.besace@gmail.com>
@Trott @MylesBorins
test: remove unused var in test-tls-server-verify
c4cae1f 
`connections` is assigned but never used. Remove it.

(This was missed by the linter in previous versions of ESLint but is
flagged by the current version. Updating the linter is contingent on
this change or some similar remedy landing.)

PR-URL: nodejs#7595
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@Trott @MylesBorins
test: remove unused var from child-process-fork
700c24a 
`messageCount` is assigned, but never used. Remove it.

(This was missed by the linter in previous versions of ESLint but is
flagged by the current version. Updating the linter is contingent on
this change or some similar remedy landing.)

PR-URL: nodejs#7599
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michaël Zasso <mic.besace@gmail.com>
@Trott @MylesBorins
test: remove unused var from stream2 test
b98e20d 
`writes` is assigned but never used. Remove it.

(This was missed by the linter in previous versions of ESLint but is
flagged by the current version. Updating the linter is contingent on
this change or some similar remedy landing.)

PR-URL: nodejs#7596
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michaël Zasso <mic.besace@gmail.com>
@Trott @MylesBorins
test: remove unused var in net-server-try-ports
6d75996 
`connections` is assigned but never used. Remove it.

(This was missed by the linter in previous versions of ESLint but is
flagged by the current version. Updating the linter is contingent on
this change or some similar remedy landing.)

PR-URL: nodejs#7597
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michaël Zasso <mic.besace@gmail.com>
@Trott @MylesBorins
benchmark: remove unused variables
eaf40a0 
Remove variables that are assigned but never used.

(This was missed by the linter in previous versions of ESLint but is
flagged by the current version. Updating the linter is contingent on
this change or some similar remedy landing.)

PR-URL: nodejs#7600
Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: Michaël Zasso <mic.besace@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
@Trott @MylesBorins
test: remove unused vars from http/https tests
e3aabdb 
Remove handful of variables that are assigned but never used.

(This was missed by the linter in previous versions of ESLint but is
flagged by the current version. Updating the linter is contingent on
this change or some similar remedy landing.)

PR-URL: nodejs#7598
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michaël Zasso <mic.besace@gmail.com>
Trott and others added 18 commits July 11, 2016 22:33
@Trott @MylesBorins
tools: update ESLint, fix unused vars bug
09c8ec7 
Update ESLint to 3.0.0. This includes an enhancement to `no-unused-vars`
such that it finds a few instances in our code base that it did not find
previously (fixed in previous commits readying this for landing).

PR-URL: nodejs#7601
Reviewed-By: Michaël Zasso <mic.besace@gmail.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
@Trott @MylesBorins
test: update weak module for gc tests
a187b25 
Previous version of weak used for gc tests emitted a warning on OS X.
Updating to current version eliminates warning.

PR-URL: nodejs#7014
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
@Trott @MylesBorins
test: test isFullWidthCodePoint with invalid input
c3c26a2 
Code coverage information shows that we are only testing the happy path
for the internal readline `isFullWidthCodePoint()` function. Test it
with invalid input.

PR-URL: nodejs#7422
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
@cjihrig @MylesBorins
doc: fix cluster worker 'message' event
53f114d 
This commit adds the missing handle argument to the cluster
worker 'message' event. It also adds a link to the process
'message' event for reference.

Refs: nodejs#7297
PR-URL: nodejs#7309
Reviewed-By: Brian White <mscdex@mscdex.net>
@bnoordhuis @MylesBorins
src: remove unused #include statement
27743a9 
strcasecmp() is not used in src/node_http_parser.cc so there is no need
to include its header file.

PR-URL: nodejs#6582
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@bnoordhuis @MylesBorins
src: don't use locale-sensitive strcasecmp()
79e9d5b 
strcasecmp() is affected by the current locale as configured through
e.g. the LC_ALL environment variable and the setlocale() libc function.

It can result in unpredictable results across systems so replace it with
a function that isn't susceptible to that.

PR-URL: nodejs#6582
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@bnoordhuis @MylesBorins
tools: update certdata.txt
b5b44d8 
This is the certdata.txt[0] that ships in Firefox 47 and NSS 3.23, last
updated on 2016-02-26.

[0] https://hg.mozilla.org/mozilla-central/raw-file/1f84dea6508d/security/nss/lib/ckfw/builtins/certdata.txt

PR-URL: nodejs#7363
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
@bnoordhuis @MylesBorins
crypto: update root certificates
b168928 
Update the list of root certificates in src/node_root_certs.h with
tools/mk-ca-bundle.pl.

Certificates added:
- CA WoSign ECC Root
- Certification Authority of WoSign G2
- Certinomis - Root CA
- Certum Trusted Network CA 2
- OISTE WISeKey Global Root GB CA
- SZAFIR ROOT CA2
- TURKTRUST Elektronik Sertifika Hizmet Sa?layıcısı H5
- TURKTRUST Elektronik Sertifika Hizmet Sa?layıcısı H6

Certificates removed:
- A-Trust-nual-03
- Buypass Class 3 CA 1
- CA Disig
- ComSign Secured CA
- Equifax Secure CA
- NetLock Notary (Class A) Root
- Staat der Nederlanden Root CA
- TC TrustCenter Class 2 CA II
- TC TrustCenter Universal CA I
- TURKTRUST Certificate Services Provider Root 1
- TURKTRUST Certificate Services Provider Root 2
- UTN DATACorp SGC Root CA
- Verisign Class 4 Public Primary Certification Authority - G3

PR-URL: nodejs#7363
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
@bnoordhuis @MylesBorins
test: remove internet/test-tls-connnect-cnnic
de57ead 
Shigeki Ohtsu points out that the test is unreliable because some of
the www1.cnnnic.cn servers are misconfigured.  Remove it.

PR-URL: nodejs#7363
Refs: nodejs#7363 (comment)
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
@bnoordhuis @MylesBorins
src: check uv_async_init() return value
46c8f84 
Pointed out by Coverity.

PR-URL: nodejs#7374
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
@bnoordhuis @MylesBorins
src: guard against starting fs watcher twice
b025cae 
This commit adds a CHECK that verifies that the file event watcher is
not started twice, which would be indicative of a bug in lib/fs.js.

PR-URL: nodejs#7374
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
@bnoordhuis @MylesBorins
src: remove unused data member write_queue_size_
c4f8e2c 
Remove TLSWrap::write_queue_size_, it's not used anywhere.

PR-URL: nodejs#7374
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
@bnoordhuis @MylesBorins
src: remove unused md_ data members
5230f92 
The code assigned the result of EVP_get_digestbyname() to data members
called md_ that were not used outside the initialization functions.

PR-URL: nodejs#7374
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
@bnoordhuis @MylesBorins
src: remove duplicate HMAC_Init calls
6a8d0bb 
PR-URL: nodejs#7374
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
@bnoordhuis @MylesBorins
src: remove deprecated HMAC_Init, use HMAC_Init_ex
b3811fd 
PR-URL: nodejs#7374
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
@bnoordhuis @MylesBorins
src: fix use-after-return in zlib bindings
8be9d0a 
Pointed out by Coverity.  Introduced in commit 5b8e1da from September
2011 ("Initial pass at zlib bindings".)

The asynchronous version of Write() used a pointer to a stack-allocated
buffer on flush.  A mitigating factor is that zlib does not dereference
the pointer for zero-sized writes but it's still technically UB.

PR-URL: nodejs#7374
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
@bnoordhuis @MylesBorins
src: fix bad logic in uid/gid checks
6d2779d 
Pointed out by Coverity.  Introduced in commits 3546383 ("process_wrap:
avoid leaking memory when throwing due to invalid arguments") and
fa4eb47 ("bindings: add spawn_sync bindings").

The return statements inside the if blocks were dead code because their
guard conditions always evaluated to false.  Remove them.

PR-URL: nodejs#7374
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
@indutny @MylesBorins
tls: catch certCbDone exceptions
1e66668 
Catch and emit `certCbDone` exceptions instead of throwing them as
`uncaughtException` and crashing the whole process.

Fix: nodejs#6822
PR-URL: nodejs#6887
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
targos and others added 3 commits July 12, 2016 08:29
@targos
build: add v8 requirement to test-v8* in Makefile
b68e685 
The test targets expect that V8 is built in deps/v8/out

Ref: nodejs#7477
PR-URL: nodejs#7482
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@targos
build: use BUILDTYPE when building V8 in Makefile
110ce55 
Without this it would always compile Release and Debug builds.

Ref: nodejs#7477
PR-URL: nodejs#7482
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@danbev @addaleax
src: renaming ares_task struct to node_ares_task
ee1461f 
This commit attempts to fix one of the items in
nodejs#4641, which was to remove a TODO
comment from env.h regarding the naming of the ares_task_t struct.

Also, the struct ares_task_list was renamed to node_ares_task_list
following the same reasoning that is does not belong to the c-ares API.

PR-URL: nodejs#7345
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
@saghul
Copy link
Member

saghul commented Jul 12, 2016

LGTM, but why does this need to be backported? (other than avoiding conflicts when backporting fixes to surrounding code, that is)

@addaleax
Copy link
Member Author

other than avoiding conflicts when backporting fixes to surrounding code, that is

That’s all, yep. I don’t think anybody would mind leaving it out of v4.

@MylesBorins
Copy link
Contributor

Let's leave this as a don't land then and we can revisit if we run into problems in the future

@MylesBorins MylesBorins removed their assignment Dec 27, 2016
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.