src: clean up uv_fs_t's in module_wrap.cc by cjihrig · Pull Request #16722 · nodejs/node · GitHub
Skip to content

src: clean up uv_fs_t's in module_wrap.cc#16722

Merged
cjihrig merged 1 commit into
nodejs:masterfrom
cjihrig:cleanup
Nov 6, 2017
Merged

src: clean up uv_fs_t's in module_wrap.cc#16722
cjihrig merged 1 commit into
nodejs:masterfrom
cjihrig:cleanup

Conversation

@cjihrig

@cjihrig cjihrig commented Nov 3, 2017

Copy link
Copy Markdown
Contributor

This commit adds uv_fs_req_cleanup() calls to all uses of uv_fs_t's in src/module_wrap.cc.

Note, this originally started as a response to a Coverity scan:

>>>     CID 178567:  Error handling issues  (CHECKED_RETURN)
>>>     Calling "uv_fs_read" without checking return value (as is done elsewhere 4 out of 5 times).
314         uv_fs_read(uv_default_loop(),

I fixed the Coverity issue by using the return value from uv_fs_read() instead of req.result.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)

src

@nodejs-github-bot nodejs-github-bot added the c++ Issues and PRs that require attention from people who are familiar with C++. label Nov 3, 2017
@addaleax addaleax added the esm Issues and PRs related to the ECMAScript Modules implementation. label Nov 3, 2017
@cjihrig

cjihrig commented Nov 6, 2017

Copy link
Copy Markdown
Contributor Author

This commit adds uv_fs_req_cleanup() calls to all uses of
uv_fs_t's in src/module_wrap.cc.

PR-URL: nodejs#16722
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
@cjihrig cjihrig merged commit 4023702 into nodejs:master Nov 6, 2017
@cjihrig cjihrig deleted the cleanup branch November 6, 2017 15:36
cjihrig added a commit to cjihrig/node that referenced this pull request Nov 6, 2017
This commit adds uv_fs_req_cleanup() calls to all uses of
uv_fs_t's in src/module_wrap.cc.

PR-URL: nodejs#16722
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
@cjihrig cjihrig mentioned this pull request Nov 6, 2017
gibfahn pushed a commit that referenced this pull request Dec 19, 2017
This commit adds uv_fs_req_cleanup() calls to all uses of
uv_fs_t's in src/module_wrap.cc.

PR-URL: #16722
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
@gibfahn

gibfahn commented Dec 19, 2017

Copy link
Copy Markdown
Member

This was referenced Dec 20, 2017
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++. esm Issues and PRs related to the ECMAScript Modules implementation.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants