Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

src: check return value of ftell() #46495

Merged

Conversation

tniessen
Copy link
Member

@tniessen tniessen commented Feb 4, 2023

If ftell() returns -1L, abort instead of passing static_cast<size_t>(-1) to the vector allocator and fread().

Refs: #46463

If ftell() returns -1L, abort instead of passing static_cast<size_t>(-1)
to the vector allocator and fread().

Refs: nodejs#46463
@tniessen tniessen requested a review from joyeecheung February 4, 2023 13:21
@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/startup

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run. labels Feb 4, 2023
@tniessen tniessen added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. request-ci Add this label to start a Jenkins CI on a PR. labels Feb 4, 2023
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Feb 4, 2023
@nodejs-github-bot

This comment was marked as outdated.

Copy link
Member

@addaleax addaleax left a comment

Choose a reason for hiding this comment

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

Will suggest fast-track because this looks like a very obviously correct change & so that I can rebase #46491 against this :)

@addaleax addaleax added the fast-track PRs that do not need to wait for 48 hours to land. label Feb 4, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Feb 4, 2023

Fast-track has been requested by @addaleax. Please 👍 to approve.

@nodejs-github-bot

This comment was marked as outdated.

@nodejs-github-bot

This comment was marked as outdated.

@nodejs-github-bot

This comment was marked as outdated.

@nodejs-github-bot

This comment was marked as outdated.

@nodejs-github-bot

This comment was marked as outdated.

@tniessen
Copy link
Member Author

tniessen commented Feb 4, 2023

Something is wrong with Windows CI but I have no idea what it is.

@nodejs-github-bot

This comment was marked as outdated.

@RaisinTen
Copy link
Contributor

I think it's a flaky one, opened an issue about it - #46517.

@nodejs-github-bot

This comment was marked as outdated.

@nodejs-github-bot

This comment was marked as outdated.

@nodejs-github-bot
Copy link
Collaborator

@addaleax addaleax added the commit-queue Add this label to land a pull request using GitHub Actions. label Feb 6, 2023
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Feb 6, 2023
@nodejs-github-bot nodejs-github-bot merged commit 1e9ca45 into nodejs:main Feb 6, 2023
@nodejs-github-bot
Copy link
Collaborator

Landed in 1e9ca45

MylesBorins pushed a commit that referenced this pull request Feb 18, 2023
If ftell() returns -1L, abort instead of passing static_cast<size_t>(-1)
to the vector allocator and fread().

Refs: #46463
PR-URL: #46495
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
@MylesBorins MylesBorins mentioned this pull request Feb 19, 2023
danielleadams pushed a commit that referenced this pull request Apr 11, 2023
If ftell() returns -1L, abort instead of passing static_cast<size_t>(-1)
to the vector allocator and fread().

Refs: #46463
PR-URL: #46495
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
# for free to join this conversation on GitHub. Already have an account? # 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++. fast-track PRs that do not need to wait for 48 hours to land. needs-ci PRs that need a full CI run.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants