Skip to content

ffi: accept pointer BigInts in multi-argument fast calls - #64964

Open
trivikr wants to merge 1 commit into
nodejs:mainfrom
trivikr:ffi-buffer-args-bigint-pointers
Open

ffi: accept pointer BigInts in multi-argument fast calls#64964
trivikr wants to merge 1 commit into
nodejs:mainfrom
trivikr:ffi-buffer-args-bigint-pointers

Conversation

@trivikr

@trivikr trivikr commented Aug 2, 2026

Copy link
Copy Markdown
Member

Fixes: #64963

convertPointerArg ended with an unconditional getRawPointer call for buffer and arraybuffer types, rejecting BigInt addresses that the single-argument fast path and ToFFIArgument both accept. Drop the fallback; hasPointerMemoryArg already converts memory-backed values, and null, undefined, and strings are handled earlier.


Assisted-by: claude:opus-5

convertPointerArg ended with an unconditional getRawPointer call for
buffer and arraybuffer types, rejecting BigInt addresses that the
single-argument fast path and ToFFIArgument both accept. Drop the
fallback; hasPointerMemoryArg already converts memory-backed values,
and null, undefined, and strings are handled earlier.

Signed-off-by: Trivikram Kamat <16024985+trivikr@users.noreply.github.com>
Assisted-by: claude:opus-5
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/ffi

@nodejs-github-bot nodejs-github-bot added ffi Issues and PRs related to experimental Foreign Function Interface support. needs-ci PRs that need a full CI run. labels Aug 2, 2026
@trivikr trivikr added the request-ci Add this label to start a Jenkins CI on a PR. label Aug 2, 2026
@codecov

codecov Bot commented Aug 2, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.30%. Comparing base (c55eb4a) to head (f8a058a).
⚠️ Report is 5 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main   #64964   +/-   ##
=======================================
  Coverage   90.30%   90.30%           
=======================================
  Files         760      760           
  Lines      247238   247237    -1     
  Branches    46623    46616    -7     
=======================================
+ Hits       223259   223266    +7     
  Misses      15448    15448           
+ Partials     8531     8523    -8     
Files with missing lines Coverage Δ
lib/internal/ffi/fast-api.js 93.95% <100.00%> (+1.62%) ⬆️

... and 31 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Aug 2, 2026
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@mcollina mcollina 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.

lgtm

@mcollina mcollina added the commit-queue Add this label to land a pull request using GitHub Actions. label Aug 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

commit-queue Add this label to land a pull request using GitHub Actions. ffi Issues and PRs related to experimental Foreign Function Interface support. needs-ci PRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ffi: buffer/arraybuffer args reject BigInt pointers in multi-argument fast signatures

4 participants