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

node-api: make napi_get_buffer_info check if passed buffer is valid by Janrupf · Pull Request #51571 · nodejs/node · GitHub

/ node Public

node-api: make napi_get_buffer_info check if passed buffer is valid - #51571

Merged
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
Janrupf:main
Feb 23, 2024
ConversationCommits (1)ChecksFiles changed
Merged

node-api: make napi_get_buffer_info check if passed buffer is valid#51571
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
Janrupf:main

Conversation

Janrupf commented Jan 26, 2024

Copy link
Copy Markdown
Contributor

Fixes #51570

nodejs-github-bot commented Jan 26, 2024

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/node-api

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. node-api Issues and PRs related to the Node-API. labels Jan 26, 2024
mhdawson reviewed Jan 26, 2024
View reviewed changes
Comment thread src/node_api.cc Outdated
Janrupf marked this pull request as ready for review January 26, 2024 19:17
Janrupf requested a review from mhdawson January 26, 2024 19:18
mhdawson approved these changes Jan 29, 2024
View reviewed changes

mhdawson left a comment

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

LGTM

mhdawson added the request-ci Add this label to start a Jenkins CI on a PR. label Jan 29, 2024
github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jan 29, 2024

nodejs-github-bot commented Jan 29, 2024

Copy link
Copy Markdown
Collaborator

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

toyobayashi mentioned this pull request Jan 30, 2024
Merged
Janrupf requested a review from mhdawson January 30, 2024 20:21

Janrupf commented Jan 30, 2024

Copy link
Copy Markdown
Contributor Author

Fixed the linting errors, however, I have no idea why the test suddenly blew up on macOS. Possibly flaky?

vmoroz approved these changes Jan 30, 2024
View reviewed changes

vmoroz left a comment

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

LGTM

mhdawson added the request-ci Add this label to start a Jenkins CI on a PR. label Jan 31, 2024
github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jan 31, 2024

mhdawson commented Jan 31, 2024

Copy link
Copy Markdown
Member

@Janrupf kicked off another CI, I should have waited until the local ones had run before doing that earlier, we'll see what the results look like now.

nodejs-github-bot commented Jan 31, 2024

Copy link
Copy Markdown
Collaborator

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

Janrupf commented Feb 1, 2024

Copy link
Copy Markdown
Contributor Author

Formatting should be good now, though the coverage test failed. I'm not sure if this is related to my changes (seems a bit unlikely)

mhdawson approved these changes Feb 2, 2024
View reviewed changes

mhdawson left a comment

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

LGTM

mhdawson added the request-ci Add this label to start a Jenkins CI on a PR. label Feb 2, 2024
github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Feb 2, 2024

nodejs-github-bot commented Feb 2, 2024

Copy link
Copy Markdown
Collaborator

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

nodejs-github-bot commented Feb 4, 2024

Copy link
Copy Markdown
Collaborator

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

mertcanaltin approved these changes Feb 9, 2024
View reviewed changes

mertcanaltin left a comment

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

LGTM

Janrupf commented Feb 10, 2024

Copy link
Copy Markdown
Contributor Author

Jenkins still says its under security embargo, so I have no idea why the tests fail (they do pass locally...), any chance someone could take a look at this/make the logs available?

nodejs-github-bot commented Feb 15, 2024

Copy link
Copy Markdown
Collaborator

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

Janrupf commented Feb 15, 2024

Copy link
Copy Markdown
Contributor Author

CI seems to be failing due to something unrelated again, @mhdawson mind taking a look?

mhdawson added the request-ci Add this label to start a Jenkins CI on a PR. label Feb 15, 2024

mhdawson commented Feb 15, 2024

Copy link
Copy Markdown
Member

I think it's been too long to resume the existing ci, kicked off another one, expect it to need to be resumed a few times.

github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Feb 15, 2024

nodejs-github-bot commented Feb 15, 2024

Copy link
Copy Markdown
Collaborator

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

Janrupf commented Feb 16, 2024

Copy link
Copy Markdown
Contributor Author

@mhdawson CI failed again (sorry for the pings...), this time another random check. I don't think this is related to my changes, but neither can I fully confirm it is not. Any idea if thats a flaky test or something really is broken?

nodejs-github-bot commented Feb 16, 2024

Copy link
Copy Markdown
Collaborator

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

nodejs-github-bot commented Feb 20, 2024

Copy link
Copy Markdown
Collaborator

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

nodejs-github-bot commented Feb 20, 2024

Copy link
Copy Markdown
Collaborator

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

nodejs-github-bot commented Feb 20, 2024

Copy link
Copy Markdown
Collaborator

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

mhdawson commented Feb 20, 2024

Copy link
Copy Markdown
Member

Opened - #51813 for latest flaky test failure.

nodejs-github-bot commented Feb 22, 2024

Copy link
Copy Markdown
Collaborator

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

legendecas added the commit-queue Add this label to land a pull request using GitHub Actions. label Feb 23, 2024
nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Feb 23, 2024
nodejs-github-bot merged commit 281c342 into nodejs:main Feb 23, 2024

nodejs-github-bot commented Feb 23, 2024

Copy link
Copy Markdown
Collaborator

Landed in 281c342

marco-ippolito pushed a commit that referenced this pull request Feb 26, 2024
node-api: make napi_get_buffer_info check if passed buffer is valid
bab27c1 
PR-URL: #51571
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Vladimir Morozov <vmorozov@microsoft.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
marco-ippolito pushed a commit that referenced this pull request Feb 27, 2024
node-api: make napi_get_buffer_info check if passed buffer is valid
bc6f33d 
PR-URL: #51571
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Vladimir Morozov <vmorozov@microsoft.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
marco-ippolito mentioned this pull request Mar 1, 2024
Merged
richardlau pushed a commit that referenced this pull request Mar 25, 2024
node-api: make napi_get_buffer_info check if passed buffer is valid
06a29f8 
PR-URL: #51571
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Vladimir Morozov <vmorozov@microsoft.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
richardlau mentioned this pull request Mar 25, 2024
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

vmoroz vmoroz approved these changes
legendecas legendecas approved these changes
+2 more reviewers
mhdawson mhdawson approved these changes
mertcanaltin mertcanaltin approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run. node-api Issues and PRs related to the Node-API.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

napi_get_buffer_info crashes with an assertion error

6 participants

Back | FazBrowse Home | New Git URL