FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Coverity fixes, round one by bnoordhuis · Pull Request #7374 · nodejs/node · GitHub

/ node Public

Coverity fixes, round one - #7374

Merged
bnoordhuis merged 10 commits into
nodejs:masterfrom
bnoordhuis:coverity-fixes-round-one
Jun 29, 2016
ConversationCommits (10)ChecksFiles changed
Merged

Coverity fixes, round one#7374
bnoordhuis merged 10 commits into
nodejs:masterfrom
bnoordhuis:coverity-fixes-round-one

Conversation

bnoordhuis commented Jun 22, 2016 •
edited
Loading

Copy link
Copy Markdown
Member

Issues pointed out by Coverity.

CI: https://ci.nodejs.org/job/node-test-pull-request/3056/

bnoordhuis added c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs related to general changes in the lib or src directory. labels Jun 22, 2016

addaleax commented Jun 22, 2016

Copy link
Copy Markdown
Member

LGTM, CI failure seems to be infrastructure-related

mhdawson commented Jun 23, 2016

Copy link
Copy Markdown
Member

LGTM

1 similar comment

cjihrig commented Jun 23, 2016

Copy link
Copy Markdown
Contributor

LGTM

santigimeno reviewed Jun 23, 2016
View reviewed changes
Comment thread src/fs_event_wrap.cc Outdated

santigimeno Jun 23, 2016

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

just wondering, could these have been initialized in the initializer list of FSEventWrap::FSEventWrap?

bnoordhuis Jun 23, 2016

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Could have, yes, but I think the C++11 way of assigning at definition is clearer; no need to look up the constructor to see if and how it's assigned.

santigimeno Jun 24, 2016

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Understood. Thanks

jasnell commented Jun 27, 2016

Copy link
Copy Markdown
Member

LGTM

bnoordhuis force-pushed the coverity-fixes-round-one branch from 5ebe1ba to 23d31b4 Compare June 29, 2016 09:09

bnoordhuis commented Jun 29, 2016

Copy link
Copy Markdown
Member Author

Rebased with no structural changes. New CI: https://ci.nodejs.org/job/node-test-pull-request/3117/

bnoordhuis added 10 commits June 29, 2016 12:19
src: check uv_async_init() return value
5e60ded 
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>
src: initialize encoding_ data member
88a8748 
Pointed out by Coverity.  Not really a bug because it's assigned before
use but explicit assignment in the constructor is more obviously
correct.

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>
src: guard against starting fs watcher twice
57387fa 
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>
src: remove unused data member write_queue_size_
664ba51 
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>
src: remove unused md_ data members
0e82069 
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>
src: remove duplicate HMAC_Init calls
d404566 
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>
src: remove deprecated HMAC_Init, use HMAC_Init_ex
26a918f 
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>
src: fix use-after-return in zlib bindings
da4c1c3 
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>
src: fix memory leak in WriteBuffers() error path
c50e192 
Pointed out by Coverity.  Introduced in commit 05d30d5 from July 2015
("fs: implemented WriteStream#writev").

WriteBuffers() leaked memory in the synchronous uv_fs_write() error path
when trying to write > 1024 buffers.

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>
src: fix bad logic in uid/gid checks
ed3d8b1 
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>
bnoordhuis force-pushed the coverity-fixes-round-one branch from 23d31b4 to ed3d8b1 Compare June 29, 2016 10:23
bnoordhuis closed this Jun 29, 2016
bnoordhuis deleted the coverity-fixes-round-one branch June 29, 2016 10:23
bnoordhuis merged commit ed3d8b1 into nodejs:master Jun 29, 2016
Fishrock123 pushed a commit that referenced this pull request Jul 5, 2016
src: check uv_async_init() return value
c795d1e 
Pointed out by Coverity.

PR-URL: #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>
Fishrock123 pushed a commit that referenced this pull request Jul 5, 2016
src: initialize encoding_ data member
3b1c19f 
Pointed out by Coverity.  Not really a bug because it's assigned before
use but explicit assignment in the constructor is more obviously
correct.

PR-URL: #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>
Fishrock123 pushed a commit that referenced this pull request Jul 5, 2016
src: guard against starting fs watcher twice
9945b4e 
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: #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>
Fishrock123 pushed a commit that referenced this pull request Jul 5, 2016
src: remove unused data member write_queue_size_
2228a65 
Remove TLSWrap::write_queue_size_, it's not used anywhere.

PR-URL: #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>
Fishrock123 pushed a commit that referenced this pull request Jul 5, 2016
src: remove unused md_ data members
25bc7fe 
The code assigned the result of EVP_get_digestbyname() to data members
called md_ that were not used outside the initialization functions.

PR-URL: #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>
Fishrock123 pushed a commit that referenced this pull request Jul 5, 2016
src: remove duplicate HMAC_Init calls
b7e661b 
PR-URL: #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>
Fishrock123 pushed a commit that referenced this pull request Jul 5, 2016
src: remove deprecated HMAC_Init, use HMAC_Init_ex
2816418 
PR-URL: #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>
Fishrock123 pushed a commit that referenced this pull request Jul 5, 2016
src: fix use-after-return in zlib bindings
ce039c3 
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: #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>
Fishrock123 pushed a commit that referenced this pull request Jul 5, 2016
src: fix memory leak in WriteBuffers() error path
6fa560d 
Pointed out by Coverity.  Introduced in commit 05d30d5 from July 2015
("fs: implemented WriteStream#writev").

WriteBuffers() leaked memory in the synchronous uv_fs_write() error path
when trying to write > 1024 buffers.

PR-URL: #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>
Fishrock123 pushed a commit that referenced this pull request Jul 5, 2016
src: fix bad logic in uid/gid checks
8f7baff 
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: #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>
Fishrock123 mentioned this pull request Jul 5, 2016
Merged

MylesBorins commented Jul 11, 2016

Copy link
Copy Markdown
Contributor

@bnoordhuis this is not landing cleanly on v4.x would you be able to backport?

bnoordhuis added a commit to bnoordhuis/io.js that referenced this pull request Jul 11, 2016
src: check uv_async_init() return value
785a100 
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 added a commit to bnoordhuis/io.js that referenced this pull request Jul 11, 2016
src: guard against starting fs watcher twice
c8e8f79 
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 added a commit to bnoordhuis/io.js that referenced this pull request Jul 11, 2016
src: remove unused data member write_queue_size_
a3ba357 
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 added a commit to bnoordhuis/io.js that referenced this pull request Jul 11, 2016
src: remove unused md_ data members
31a1016 
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 added a commit to bnoordhuis/io.js that referenced this pull request Jul 11, 2016
src: remove duplicate HMAC_Init calls
d378b8f 
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 added a commit to bnoordhuis/io.js that referenced this pull request Jul 11, 2016
src: remove deprecated HMAC_Init, use HMAC_Init_ex
2ec9307 
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 added a commit to bnoordhuis/io.js that referenced this pull request Jul 11, 2016
src: fix use-after-return in zlib bindings
b9c7d59 
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 added a commit to bnoordhuis/io.js that referenced this pull request Jul 11, 2016
src: fix bad logic in uid/gid checks
ea053f6 
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>
addaleax mentioned this pull request Jul 11, 2016
Closed
MylesBorins mentioned this pull request Jul 12, 2016
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
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++. lib / src Issues and PRs related to general changes in the lib or src directory.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

Back | FazBrowse Home | New Git URL