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

src: readlink("/proc/self/exe") -> uv_exename() by bnoordhuis · Pull Request #28333 · nodejs/node · GitHub

/ node Public

src: readlink("/proc/self/exe") -> uv_exename() - #28333

Closed
bnoordhuis wants to merge 1 commit into
nodejs:masterfrom
bnoordhuis:linux-thp-exepath
Closed

src: readlink("/proc/self/exe") -> uv_exename()#28333
bnoordhuis wants to merge 1 commit into
nodejs:masterfrom
bnoordhuis:linux-thp-exepath

Conversation

Copy link
Copy Markdown
Member

This commit also adds error handling. A THP-enabled build terminated
with an out-of-memory error on a system without /proc because it cast
the -1 from readlink() to size_t (i.e. ULONG_MAX) and then tried to
allocate a string of that size.

Caveat emptor: this code path isn't exercised by the CI.

This commit also adds error handling. A THP-enabled build terminated
with an out-of-memory error on a system without /proc because it cast
the -1 from readlink() to size_t (i.e. ULONG_MAX) and then tried to
allocate a string of that size.
nodejs-github-bot added the c++ Issues and PRs that require attention from people who are familiar with C++. label Jun 21, 2019

Trott commented Jun 21, 2019

Copy link
Copy Markdown
Member

Would it be overkill to add a comment explaining the situation where this arises so that no one removes this later as unreachable code when reviewing coverage or anything like that? Might even be an OK TODO item for adding a host to the CI that would cause the branch to run?

Copy link
Copy Markdown
Member Author

It isn't exercised because there's no CI bot that builds Node.js in this configuration and I don't expect that to happen anytime soon. This feature is only relevant for a tiny subset of the small subset of people that build Node.js from source.

This comment has been minimized.

This comment has been minimized.

BridgeAR added the author ready PRs that have at least one approval, no outstanding review comments, and a CI started. label Jul 4, 2019

Copy link
Copy Markdown
Collaborator

Copy link
Copy Markdown
Collaborator

Copy link
Copy Markdown
Collaborator

Trott commented Jul 30, 2019

Copy link
Copy Markdown
Member

Landed in 54ae530

Trott closed this Jul 30, 2019
Trott pushed a commit to Trott/io.js that referenced this pull request Jul 30, 2019
This commit also adds error handling. A THP-enabled build terminated
with an out-of-memory error on a system without /proc because it cast
the -1 from readlink() to size_t (i.e. ULONG_MAX) and then tried to
allocate a string of that size.

PR-URL: nodejs#28333
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
targos pushed a commit that referenced this pull request Aug 2, 2019
This commit also adds error handling. A THP-enabled build terminated
with an out-of-memory error on a system without /proc because it cast
the -1 from readlink() to size_t (i.e. ULONG_MAX) and then tried to
allocate a string of that size.

PR-URL: #28333
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
gabrielschulhof pushed a commit to gabrielschulhof/node that referenced this pull request Mar 6, 2020
This commit also adds error handling. A THP-enabled build terminated
with an out-of-memory error on a system without /proc because it cast
the -1 from readlink() to size_t (i.e. ULONG_MAX) and then tried to
allocate a string of that size.

PR-URL: nodejs#28333
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author ready PRs that have at least one approval, no outstanding review comments, and a CI started. c++ Issues and PRs that require attention from people who are familiar with C++.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants


Back | FazBrowse Home | New Git URL