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.

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

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

Closed
wants to merge 1 commit into from

Conversation

bnoordhuis
Copy link
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 nodejs-github-bot added the c++ Issues and PRs that require attention from people who are familiar with C++. label Jun 21, 2019
@Trott
Copy link
Member

Trott commented Jun 21, 2019

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?

@bnoordhuis
Copy link
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.

@nodejs-github-bot

This comment has been minimized.

@nodejs-github-bot

This comment has been minimized.

@BridgeAR BridgeAR added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jul 4, 2019
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@Trott
Copy link
Member

Trott commented Jul 30, 2019

Landed in 54ae530

@Trott 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 <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
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 <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
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 <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
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 pending requests for changes, 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